[PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Tue May 2 21:36:33 UTC 2023
On 03/05/2023 00:33, Abhinav Kumar wrote:
>
>
> On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote:
>> Move common DPU physical encoder initialization code to the new function
>> dpu_encoder_phys_init().
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 31 +++++++++++++++++--
>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 ++
>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 19 +++---------
>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 20 +++---------
>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 19 +++---------
>> 5 files changed, 46 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 8c45c949ec39..c60dce5861e2 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct
>> dpu_encoder_virt *dpu_enc,
>> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>> - atomic_set(&phys->vsync_cnt, 0);
>> - atomic_set(&phys->underrun_cnt, 0);
>> if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
>> phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm,
>> phys->intf_idx);
>> @@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct
>> dpu_encoder_phys *phys_enc)
>> return dpu_enc->dsc_mask;
>> }
>> +
>> +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
>> + struct dpu_enc_phys_init_params *p)
>> +{
>> + int i;
>> +
>> + phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>> + phys_enc->intf_idx = p->intf_idx;
>> + phys_enc->wb_idx = p->wb_idx;
>> + phys_enc->parent = p->parent;
>> + phys_enc->dpu_kms = p->dpu_kms;
>> + phys_enc->split_role = p->split_role;
>> + phys_enc->enc_spinlock = p->enc_spinlock;
>> + phys_enc->enable_state = DPU_ENC_DISABLED;
>> +
>> + for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
>> + phys_enc->irq[i] = -EINVAL;
>> +
>> + atomic_set(&phys_enc->vblank_refcount, 0);
>> + atomic_set(&phys_enc->pending_kickoff_cnt, 0);
>> + atomic_set(&phys_enc->pending_ctlstart_cnt, 0);
>> +
>> + atomic_set(&phys_enc->vsync_cnt, 0);
>> + atomic_set(&phys_enc->underrun_cnt, 0);
>> +
>> + init_waitqueue_head(&phys_enc->pending_kickoff_wq);
>> +
>> + return 0;
>> +}
>> 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 1d434b22180d..7019870215c0 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> @@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback(
>> struct drm_encoder *drm_enc,
>> struct dpu_encoder_phys *ready_phys, u32 event);
>> +int dpu_encoder_phys_init(struct dpu_encoder_phys *phys,
>> + struct dpu_enc_phys_init_params *p);
>> +
>> #endif /* __dpu_encoder_phys_H__ */
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> index 74470d068622..ce86b9ef6bf1 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> @@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
>> {
>> struct dpu_encoder_phys *phys_enc = NULL;
>> struct dpu_encoder_phys_cmd *cmd_enc = NULL;
>> - int i, ret = 0;
>> + int ret = 0;
>> DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
>> @@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
>> return ERR_PTR(ret);
>> }
>> phys_enc = &cmd_enc->base;
>> - phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>> - phys_enc->intf_idx = p->intf_idx;
>> +
>> + ret = dpu_encoder_phys_init(phys_enc, p);
>> + if (ret)
>> + return ERR_PTR(ret);
>
> dpu_encoder_phys_init() seems to always return 0, so we can make that
> void and drop ret and return here?
I had in mind a possible error from INTF_n/WB_n -> hw_intf/hw_wb lookup,
but at the end I got rid of that. So, yes, why not.
>
>> dpu_encoder_phys_cmd_init_ops(&phys_enc->ops);
>> - phys_enc->parent = p->parent;
>> - phys_enc->dpu_kms = p->dpu_kms;
>> - phys_enc->split_role = p->split_role;
>> phys_enc->intf_mode = INTF_MODE_CMD;
>> - phys_enc->enc_spinlock = p->enc_spinlock;
>> cmd_enc->stream_sel = 0;
>> - phys_enc->enable_state = DPU_ENC_DISABLED;
>> - for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
>> - phys_enc->irq[i] = -EINVAL;
>> - atomic_set(&phys_enc->vblank_refcount, 0);
>> - atomic_set(&phys_enc->pending_kickoff_cnt, 0);
>> - atomic_set(&phys_enc->pending_ctlstart_cnt, 0);
>> atomic_set(&cmd_enc->pending_vblank_cnt, 0);
>> - init_waitqueue_head(&phys_enc->pending_kickoff_wq);
>> init_waitqueue_head(&cmd_enc->pending_vblank_wq);
>> DPU_DEBUG_CMDENC(cmd_enc, "created\n");
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> index 3a374292f311..aca3849621e2 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> @@ -699,7 +699,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>> struct dpu_enc_phys_init_params *p)
>> {
>> struct dpu_encoder_phys *phys_enc = NULL;
>> - int i;
>> + int ret;
>> if (!p) {
>> DPU_ERROR("failed to create encoder due to invalid
>> parameter\n");
>> @@ -712,24 +712,14 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>> return ERR_PTR(-ENOMEM);
>> }
>> - phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>> - phys_enc->intf_idx = p->intf_idx;
>> -
>> DPU_DEBUG_VIDENC(phys_enc, "\n");
>> + ret = dpu_encoder_phys_init(phys_enc, p);
>> + if (ret)
>> + return ERR_PTR(ret);
>
> same here.
>
>> +
>> dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
>> - phys_enc->parent = p->parent;
>> - phys_enc->dpu_kms = p->dpu_kms;
>> - phys_enc->split_role = p->split_role;
>> phys_enc->intf_mode = INTF_MODE_VIDEO;
>> - phys_enc->enc_spinlock = p->enc_spinlock;
>> - for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
>> - phys_enc->irq[i] = -EINVAL;
>> -
>> - atomic_set(&phys_enc->vblank_refcount, 0);
>> - atomic_set(&phys_enc->pending_kickoff_cnt, 0);
>> - init_waitqueue_head(&phys_enc->pending_kickoff_wq);
>> - phys_enc->enable_state = DPU_ENC_DISABLED;
>> DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->intf_idx);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> index f879d006de21..c252127552c6 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> @@ -683,7 +683,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
>> struct dpu_encoder_phys *phys_enc = NULL;
>> struct dpu_encoder_phys_wb *wb_enc = NULL;
>> int ret = 0;
>> - int i;
>> DPU_DEBUG("\n");
>> @@ -701,28 +700,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
>> }
>> phys_enc = &wb_enc->base;
>> - phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>> - phys_enc->wb_idx = p->wb_idx;
>> +
>> + ret = dpu_encoder_phys_init(phys_enc, p);
>> + if (ret)
>> + return ERR_PTR(ret);
>
> same here
>
>> dpu_encoder_phys_wb_init_ops(&phys_enc->ops);
>> - phys_enc->parent = p->parent;
>> - phys_enc->dpu_kms = p->dpu_kms;
>> - phys_enc->split_role = p->split_role;
>> phys_enc->intf_mode = INTF_MODE_WB_LINE;
>> - phys_enc->wb_idx = p->wb_idx;
>> - phys_enc->enc_spinlock = p->enc_spinlock;
>> atomic_set(&wb_enc->wbirq_refcount, 0);
>> - for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
>> - phys_enc->irq[i] = -EINVAL;
>> -
>> - atomic_set(&phys_enc->pending_kickoff_cnt, 0);
>> - atomic_set(&phys_enc->vblank_refcount, 0);
>> wb_enc->wb_done_timeout_cnt = 0;
>> - init_waitqueue_head(&phys_enc->pending_kickoff_wq);
>> - phys_enc->enable_state = DPU_ENC_DISABLED;
>> DPU_DEBUG("Created dpu_encoder_phys for wb %d\n",
>> phys_enc->wb_idx);
--
With best wishes
Dmitry
More information about the dri-devel
mailing list