Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
On 08/26/2014 07:53 AM, Joonyoung Shim wrote: > Hi Andrzej, > > On 08/22/2014 04:52 PM, Andrzej Hajda wrote: >> The patch removes redundant checks, redundant HW reads >> and simplifies code. >> >> Signed-off-by: Andrzej Hajda >> --- >> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 >> >> 1 file changed, 15 insertions(+), 49 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> index bd6628d..b20078e 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c >> @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int >> swap, >> return 0; >> } >> >> -static int fimc_dst_get_buf_count(struct fimc_context *ctx) >> -{ >> -u32 cfg, buf_num; >> - >> -cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); >> - >> -buf_num = hweight32(cfg); >> - >> -DRM_DEBUG_KMS("buf_num[%d]\n", buf_num); >> - >> -return buf_num; >> -} >> - >> -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, >> +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, >> enum drm_exynos_ipp_buf_type buf_type) >> { >> -struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv; >> -bool enable; >> -u32 cfg; >> -u32 mask = 0x0001 << buf_id; >> -int ret = 0; >> unsigned long flags; >> +u32 buf_num; >> +u32 cfg; >> >> DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type); >> >> spin_lock_irqsave(&ctx->lock, flags); >> >> -/* mask register set */ >> cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); >> >> -switch (buf_type) { >> -case IPP_BUF_ENQUEUE: >> -enable = true; >> -break; >> -case IPP_BUF_DEQUEUE: >> -enable = false; >> -break; >> -default: >> -dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n"); >> -ret = -EINVAL; >> -goto err_unlock; >> -} >> +if (buf_type == IPP_BUF_ENQUEUE) >> +cfg |= (1 << buf_id); >> +else >> +cfg &= (1 << buf_id); > ~ Missing? Yes, thanks for pointing it out. Regards Andrzej > >> >> -/* sequence id */ >> -cfg &= ~mask; >> -cfg |= (enable << buf_id); >> fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ); >> >> -/* interrupt enable */ >> -if (buf_type == IPP_BUF_ENQUEUE && >> -fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START) >> -fimc_mask_irq(ctx, true); >> +buf_num = hweight32(cfg); >> >> -/* interrupt disable */ >> -if (buf_type == IPP_BUF_DEQUEUE && >> -fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP) >> +if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START) >> +fimc_mask_irq(ctx, true); >> +else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP) >> fimc_mask_irq(ctx, false); >> >> -err_unlock: >> spin_unlock_irqrestore(&ctx->lock, flags); >> -return ret; >> } >> >> static int fimc_dst_set_addr(struct device *dev, >> @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev, >> break; >> } >> >> -return fimc_dst_set_buf_seq(ctx, buf_id, buf_type); >> +fimc_dst_set_buf_seq(ctx, buf_id, buf_type); >> + >> +return 0; >> } >> >> static struct exynos_drm_ipp_ops fimc_dst_ops = { >> @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void >> *dev_id) >> >> DRM_DEBUG_KMS("buf_id[%d]\n", buf_id); >> >> -if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) { >> -DRM_ERROR("failed to dequeue.\n"); >> -return IRQ_HANDLED; >> -} >> +fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE); >> >> event_work->ippdrv = ippdrv; >> event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id; >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
Hi Andrzej, On 08/22/2014 04:52 PM, Andrzej Hajda wrote: > The patch removes redundant checks, redundant HW reads > and simplifies code. > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 > > 1 file changed, 15 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > index bd6628d..b20078e 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c > @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int > swap, > return 0; > } > > -static int fimc_dst_get_buf_count(struct fimc_context *ctx) > -{ > - u32 cfg, buf_num; > - > - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); > - > - buf_num = hweight32(cfg); > - > - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num); > - > - return buf_num; > -} > - > -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, > +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, > enum drm_exynos_ipp_buf_type buf_type) > { > - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv; > - bool enable; > - u32 cfg; > - u32 mask = 0x0001 << buf_id; > - int ret = 0; > unsigned long flags; > + u32 buf_num; > + u32 cfg; > > DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type); > > spin_lock_irqsave(&ctx->lock, flags); > > - /* mask register set */ > cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); > > - switch (buf_type) { > - case IPP_BUF_ENQUEUE: > - enable = true; > - break; > - case IPP_BUF_DEQUEUE: > - enable = false; > - break; > - default: > - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n"); > - ret = -EINVAL; > - goto err_unlock; > - } > + if (buf_type == IPP_BUF_ENQUEUE) > + cfg |= (1 << buf_id); > + else > + cfg &= (1 << buf_id); ~ Missing? > > - /* sequence id */ > - cfg &= ~mask; > - cfg |= (enable << buf_id); > fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ); > > - /* interrupt enable */ > - if (buf_type == IPP_BUF_ENQUEUE && > - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START) > - fimc_mask_irq(ctx, true); > + buf_num = hweight32(cfg); > > - /* interrupt disable */ > - if (buf_type == IPP_BUF_DEQUEUE && > - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP) > + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START) > + fimc_mask_irq(ctx, true); > + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP) > fimc_mask_irq(ctx, false); > > -err_unlock: > spin_unlock_irqrestore(&ctx->lock, flags); > - return ret; > } > > static int fimc_dst_set_addr(struct device *dev, > @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev, > break; > } > > - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type); > + fimc_dst_set_buf_seq(ctx, buf_id, buf_type); > + > + return 0; > } > > static struct exynos_drm_ipp_ops fimc_dst_ops = { > @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void > *dev_id) > > DRM_DEBUG_KMS("buf_id[%d]\n", buf_id); > > - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) { > - DRM_ERROR("failed to dequeue.\n"); > - return IRQ_HANDLED; > - } > + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE); > > event_work->ippdrv = ippdrv; > event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
The patch removes redundant checks, redundant HW reads and simplifies code. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 1 file changed, 15 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index bd6628d..b20078e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap, return 0; } -static int fimc_dst_get_buf_count(struct fimc_context *ctx) -{ - u32 cfg, buf_num; - - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); - - buf_num = hweight32(cfg); - - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num); - - return buf_num; -} - -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id, enum drm_exynos_ipp_buf_type buf_type) { - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv; - bool enable; - u32 cfg; - u32 mask = 0x0001 << buf_id; - int ret = 0; unsigned long flags; + u32 buf_num; + u32 cfg; DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type); spin_lock_irqsave(&ctx->lock, flags); - /* mask register set */ cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ); - switch (buf_type) { - case IPP_BUF_ENQUEUE: - enable = true; - break; - case IPP_BUF_DEQUEUE: - enable = false; - break; - default: - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n"); - ret = -EINVAL; - goto err_unlock; - } + if (buf_type == IPP_BUF_ENQUEUE) + cfg |= (1 << buf_id); + else + cfg &= (1 << buf_id); - /* sequence id */ - cfg &= ~mask; - cfg |= (enable << buf_id); fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ); - /* interrupt enable */ - if (buf_type == IPP_BUF_ENQUEUE && - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START) - fimc_mask_irq(ctx, true); + buf_num = hweight32(cfg); - /* interrupt disable */ - if (buf_type == IPP_BUF_DEQUEUE && - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP) + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START) + fimc_mask_irq(ctx, true); + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP) fimc_mask_irq(ctx, false); -err_unlock: spin_unlock_irqrestore(&ctx->lock, flags); - return ret; } static int fimc_dst_set_addr(struct device *dev, @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev, break; } - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type); + fimc_dst_set_buf_seq(ctx, buf_id, buf_type); + + return 0; } static struct exynos_drm_ipp_ops fimc_dst_ops = { @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id) DRM_DEBUG_KMS("buf_id[%d]\n", buf_id); - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) { - DRM_ERROR("failed to dequeue.\n"); - return IRQ_HANDLED; - } + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE); event_work->ippdrv = ippdrv; event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/