Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: split irq_control into irq_enable and _disable

2023-08-29 Thread Abhinav Kumar




On 6/4/2023 7:45 AM, Dmitry Baryshkov wrote:

The single helper for both enable and disable cases is too complicated,
especially if we start adding more code to these helpers. Split it into
irq_enable and irq_disable cases.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 36 ---
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  6 +-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 63 ++-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 48 +++---
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 29 ++---
  5 files changed, 112 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2e1873d29c4b..7c131c5cbe71 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -717,7 +717,7 @@ static void _dpu_encoder_update_vsync_source(struct 
dpu_encoder_virt *dpu_enc,
}
  }
  
-static void _dpu_encoder_irq_control(struct drm_encoder *drm_enc, bool enable)

+static void _dpu_encoder_irq_enable(struct drm_encoder *drm_enc)
  {
struct dpu_encoder_virt *dpu_enc;
int i;
@@ -729,14 +729,32 @@ static void _dpu_encoder_irq_control(struct drm_encoder 
*drm_enc, bool enable)
  
  	dpu_enc = to_dpu_encoder_virt(drm_enc);
  
-	DPU_DEBUG_ENC(dpu_enc, "enable:%d\n", enable);

+   DPU_DEBUG_ENC(dpu_enc, "\n");
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
  
-		if (phys->ops.irq_control)

-   phys->ops.irq_control(phys, enable);
+   phys->ops.irq_enable(phys);
+   }
+}
+
+static void _dpu_encoder_irq_disable(struct drm_encoder *drm_enc)
+{
+   struct dpu_encoder_virt *dpu_enc;
+   int i;
+
+   if (!drm_enc) {
+   DPU_ERROR("invalid encoder\n");
+   return;
}
  
+	dpu_enc = to_dpu_encoder_virt(drm_enc);

+
+   DPU_DEBUG_ENC(dpu_enc, "\n");
+   for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+   struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+
+   phys->ops.irq_disable(phys);
+   }
  }
  
  static void _dpu_encoder_resource_control_helper(struct drm_encoder *drm_enc,

@@ -762,11 +780,11 @@ static void _dpu_encoder_resource_control_helper(struct 
drm_encoder *drm_enc,
pm_runtime_get_sync(_kms->pdev->dev);
  
  		/* enable all the irq */

-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
  
  	} else {

/* disable all the irq */
-   _dpu_encoder_irq_control(drm_enc, false);
+   _dpu_encoder_irq_disable(drm_enc);
  
  		/* disable DPU core clks */

pm_runtime_put_sync(_kms->pdev->dev);
@@ -827,7 +845,7 @@ static int dpu_encoder_resource_control(struct drm_encoder 
*drm_enc,
}
  
  		if (is_vid_mode && dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE)

-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, true);
  
@@ -882,7 +900,7 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
  
  		if (is_vid_mode &&

  dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE) {
-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
}
/* skip if is already OFF or IDLE, resources are off already */
else if (dpu_enc->rc_state == DPU_ENC_RC_STATE_OFF ||
@@ -957,7 +975,7 @@ static int dpu_encoder_resource_control(struct drm_encoder 
*drm_enc,
}
  
  		if (is_vid_mode)

-   _dpu_encoder_irq_control(drm_enc, false);
+   _dpu_encoder_irq_disable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, false);
  
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h

index d48558ede488..faf033cd086e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -84,7 +84,8 @@ struct dpu_encoder_phys;
   * @handle_post_kickoff:  Do any work necessary post-kickoff work
   * @trigger_start:Process start event on physical encoder
   * @needs_single_flush:   Whether encoder slaves need to be 
flushed
- * @irq_control:   Handler to enable/disable all the encoder IRQs
+ * @irq_enable:Handler to enable all the encoder IRQs
+ * @irq_disable:   Handler to disable all the encoder IRQs
   * @prepare_idle_pc:  phys encoder can update the vsync_enable status
   *  

[Freedreno] [PATCH 1/3] drm/msm/dpu: split irq_control into irq_enable and _disable

2023-06-04 Thread Dmitry Baryshkov
The single helper for both enable and disable cases is too complicated,
especially if we start adding more code to these helpers. Split it into
irq_enable and irq_disable cases.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 36 ---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  6 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 63 ++-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 48 +++---
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 29 ++---
 5 files changed, 112 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2e1873d29c4b..7c131c5cbe71 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -717,7 +717,7 @@ static void _dpu_encoder_update_vsync_source(struct 
dpu_encoder_virt *dpu_enc,
}
 }
 
-static void _dpu_encoder_irq_control(struct drm_encoder *drm_enc, bool enable)
+static void _dpu_encoder_irq_enable(struct drm_encoder *drm_enc)
 {
struct dpu_encoder_virt *dpu_enc;
int i;
@@ -729,14 +729,32 @@ static void _dpu_encoder_irq_control(struct drm_encoder 
*drm_enc, bool enable)
 
dpu_enc = to_dpu_encoder_virt(drm_enc);
 
-   DPU_DEBUG_ENC(dpu_enc, "enable:%d\n", enable);
+   DPU_DEBUG_ENC(dpu_enc, "\n");
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
 
-   if (phys->ops.irq_control)
-   phys->ops.irq_control(phys, enable);
+   phys->ops.irq_enable(phys);
+   }
+}
+
+static void _dpu_encoder_irq_disable(struct drm_encoder *drm_enc)
+{
+   struct dpu_encoder_virt *dpu_enc;
+   int i;
+
+   if (!drm_enc) {
+   DPU_ERROR("invalid encoder\n");
+   return;
}
 
+   dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+   DPU_DEBUG_ENC(dpu_enc, "\n");
+   for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+   struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+
+   phys->ops.irq_disable(phys);
+   }
 }
 
 static void _dpu_encoder_resource_control_helper(struct drm_encoder *drm_enc,
@@ -762,11 +780,11 @@ static void _dpu_encoder_resource_control_helper(struct 
drm_encoder *drm_enc,
pm_runtime_get_sync(_kms->pdev->dev);
 
/* enable all the irq */
-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
 
} else {
/* disable all the irq */
-   _dpu_encoder_irq_control(drm_enc, false);
+   _dpu_encoder_irq_disable(drm_enc);
 
/* disable DPU core clks */
pm_runtime_put_sync(_kms->pdev->dev);
@@ -827,7 +845,7 @@ static int dpu_encoder_resource_control(struct drm_encoder 
*drm_enc,
}
 
if (is_vid_mode && dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE)
-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, true);
 
@@ -882,7 +900,7 @@ static int dpu_encoder_resource_control(struct drm_encoder 
*drm_enc,
 
if (is_vid_mode &&
  dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE) {
-   _dpu_encoder_irq_control(drm_enc, true);
+   _dpu_encoder_irq_enable(drm_enc);
}
/* skip if is already OFF or IDLE, resources are off already */
else if (dpu_enc->rc_state == DPU_ENC_RC_STATE_OFF ||
@@ -957,7 +975,7 @@ static int dpu_encoder_resource_control(struct drm_encoder 
*drm_enc,
}
 
if (is_vid_mode)
-   _dpu_encoder_irq_control(drm_enc, false);
+   _dpu_encoder_irq_disable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, false);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index d48558ede488..faf033cd086e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -84,7 +84,8 @@ struct dpu_encoder_phys;
  * @handle_post_kickoff:   Do any work necessary post-kickoff work
  * @trigger_start: Process start event on physical encoder
  * @needs_single_flush:Whether encoder slaves need to be 
flushed
- * @irq_control:   Handler to enable/disable all the encoder IRQs
+ * @irq_enable:Handler to enable all the encoder IRQs
+ * @irq_disable:   Handler to disable all the encoder IRQs
  * @prepare_idle_pc:   phys encoder can update the vsync_enable status
  *