Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: split irq_control into irq_enable and _disable
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
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 *