[PATCH 14/15] drm/exynos/fimc: simplify buffer queuing

2014-08-26 Thread Joonyoung Shim 
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 = >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(>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(>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;
>

[PATCH 14/15] drm/exynos/fimc: simplify buffer queuing

2014-08-26 Thread Andrzej Hajda 
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 = >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(>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(>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;
>>
>

[PATCH 14/15] drm/exynos/fimc: simplify buffer queuing

2014-08-22 Thread Andrzej Hajda 
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 = >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(>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(>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