[v2,08/19] media: ti-vpe: cal: simplify irq handling

Message ID 20200319075023.22151-9-tomi.valkeinen@ti.com (mailing list archive)
State Superseded, archived
Delegated to: Hans Verkuil
Headers
Series CAL fixes and improvements |

Commit Message

Tomi Valkeinen March 19, 2020, 7:50 a.m. UTC
  Instead of having identical code block to handle irqs for the two CAL
ports, we can have a for loop and a single code block.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/media/platform/ti-vpe/cal.c | 68 +++++++++++------------------
 1 file changed, 25 insertions(+), 43 deletions(-)
  

Comments

Benoit Parrot March 19, 2020, 10:39 p.m. UTC | #1
Tomi,

Thanks for the patch.

On 3/19/20 2:50 AM, Tomi Valkeinen wrote:
> Instead of having identical code block to handle irqs for the two CAL
> ports, we can have a for loop and a single code block.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Tested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/media/platform/ti-vpe/cal.c | 68 +++++++++++------------------
>  1 file changed, 25 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index b070c56f8d80..ba82f0887875 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -1228,64 +1228,46 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
>  	/* Check which DMA just finished */
>  	irqst2 = reg_read(dev, CAL_HL_IRQSTATUS(2));
>  	if (irqst2) {
> +		int i;
> +
>  		/* Clear Interrupt status */
>  		reg_write(dev, CAL_HL_IRQSTATUS(2), irqst2);
>  
> -		/* Need to check both port */
> -		if (isportirqset(irqst2, 1)) {
> -			ctx = dev->ctx[0];
> -
> -			spin_lock(&ctx->slock);
> -			ctx->dma_act = false;
> -
> -			if (ctx->cur_frm != ctx->next_frm)
> -				cal_process_buffer_complete(ctx);
> -
> -			spin_unlock(&ctx->slock);
> -		}
> -
> -		if (isportirqset(irqst2, 2)) {
> -			ctx = dev->ctx[1];
> +		for (i = 1; i <= 2; ++i) {
> +			if (isportirqset(irqst2, i)) {
> +				ctx = dev->ctx[i - 1];
>  
> -			spin_lock(&ctx->slock);
> -			ctx->dma_act = false;
> +				spin_lock(&ctx->slock);
> +				ctx->dma_act = false;
>  
> -			if (ctx->cur_frm != ctx->next_frm)
> -				cal_process_buffer_complete(ctx);
> +				if (ctx->cur_frm != ctx->next_frm)
> +					cal_process_buffer_complete(ctx);
>  
> -			spin_unlock(&ctx->slock);
> +				spin_unlock(&ctx->slock);
> +			}
>  		}
>  	}
>  
>  	/* Check which DMA just started */
>  	irqst3 = reg_read(dev, CAL_HL_IRQSTATUS(3));
>  	if (irqst3) {
> +		int i;
> +
>  		/* Clear Interrupt status */
>  		reg_write(dev, CAL_HL_IRQSTATUS(3), irqst3);
>  
> -		/* Need to check both port */
> -		if (isportirqset(irqst3, 1)) {
> -			ctx = dev->ctx[0];
> -			dma_q = &ctx->vidq;
> -
> -			spin_lock(&ctx->slock);
> -			ctx->dma_act = true;
> -			if (!list_empty(&dma_q->active) &&
> -			    ctx->cur_frm == ctx->next_frm)
> -				cal_schedule_next_buffer(ctx);
> -			spin_unlock(&ctx->slock);
> -		}
> -
> -		if (isportirqset(irqst3, 2)) {
> -			ctx = dev->ctx[1];
> -			dma_q = &ctx->vidq;
> -
> -			spin_lock(&ctx->slock);
> -			ctx->dma_act = true;
> -			if (!list_empty(&dma_q->active) &&
> -			    ctx->cur_frm == ctx->next_frm)
> -				cal_schedule_next_buffer(ctx);
> -			spin_unlock(&ctx->slock);
> +		for (i = 1; i <= 2; ++i) {
> +			if (isportirqset(irqst3, i)) {
> +				ctx = dev->ctx[i - 1];
> +				dma_q = &ctx->vidq;
> +
> +				spin_lock(&ctx->slock);
> +				ctx->dma_act = true;
> +				if (!list_empty(&dma_q->active) &&
> +				    ctx->cur_frm == ctx->next_frm)
> +					cal_schedule_next_buffer(ctx);
> +				spin_unlock(&ctx->slock);
> +			}
>  		}
>  	}
>  
> 

Reviewed-by: Benoit Parrot <bparrot@ti.com>
  

Patch

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index b070c56f8d80..ba82f0887875 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1228,64 +1228,46 @@  static irqreturn_t cal_irq(int irq_cal, void *data)
 	/* Check which DMA just finished */
 	irqst2 = reg_read(dev, CAL_HL_IRQSTATUS(2));
 	if (irqst2) {
+		int i;
+
 		/* Clear Interrupt status */
 		reg_write(dev, CAL_HL_IRQSTATUS(2), irqst2);
 
-		/* Need to check both port */
-		if (isportirqset(irqst2, 1)) {
-			ctx = dev->ctx[0];
-
-			spin_lock(&ctx->slock);
-			ctx->dma_act = false;
-
-			if (ctx->cur_frm != ctx->next_frm)
-				cal_process_buffer_complete(ctx);
-
-			spin_unlock(&ctx->slock);
-		}
-
-		if (isportirqset(irqst2, 2)) {
-			ctx = dev->ctx[1];
+		for (i = 1; i <= 2; ++i) {
+			if (isportirqset(irqst2, i)) {
+				ctx = dev->ctx[i - 1];
 
-			spin_lock(&ctx->slock);
-			ctx->dma_act = false;
+				spin_lock(&ctx->slock);
+				ctx->dma_act = false;
 
-			if (ctx->cur_frm != ctx->next_frm)
-				cal_process_buffer_complete(ctx);
+				if (ctx->cur_frm != ctx->next_frm)
+					cal_process_buffer_complete(ctx);
 
-			spin_unlock(&ctx->slock);
+				spin_unlock(&ctx->slock);
+			}
 		}
 	}
 
 	/* Check which DMA just started */
 	irqst3 = reg_read(dev, CAL_HL_IRQSTATUS(3));
 	if (irqst3) {
+		int i;
+
 		/* Clear Interrupt status */
 		reg_write(dev, CAL_HL_IRQSTATUS(3), irqst3);
 
-		/* Need to check both port */
-		if (isportirqset(irqst3, 1)) {
-			ctx = dev->ctx[0];
-			dma_q = &ctx->vidq;
-
-			spin_lock(&ctx->slock);
-			ctx->dma_act = true;
-			if (!list_empty(&dma_q->active) &&
-			    ctx->cur_frm == ctx->next_frm)
-				cal_schedule_next_buffer(ctx);
-			spin_unlock(&ctx->slock);
-		}
-
-		if (isportirqset(irqst3, 2)) {
-			ctx = dev->ctx[1];
-			dma_q = &ctx->vidq;
-
-			spin_lock(&ctx->slock);
-			ctx->dma_act = true;
-			if (!list_empty(&dma_q->active) &&
-			    ctx->cur_frm == ctx->next_frm)
-				cal_schedule_next_buffer(ctx);
-			spin_unlock(&ctx->slock);
+		for (i = 1; i <= 2; ++i) {
+			if (isportirqset(irqst3, i)) {
+				ctx = dev->ctx[i - 1];
+				dma_q = &ctx->vidq;
+
+				spin_lock(&ctx->slock);
+				ctx->dma_act = true;
+				if (!list_empty(&dma_q->active) &&
+				    ctx->cur_frm == ctx->next_frm)
+					cal_schedule_next_buffer(ctx);
+				spin_unlock(&ctx->slock);
+			}
 		}
 	}