| Message ID | 20241216-concurrent-wb-v4-8-fe220297a7f0@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+ | expand |
On 12/16/2024 4:43 PM, Jessica Zhang wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources > from dpu_crtc_assign_resources(). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > [quic_abhinavk@quicinc.com: cleaned up formatting] > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 29 +++++++++++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > <snip> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 2b999a0558b2a016644ed5d25bf54ab45c38d1d9..a895d48fe81ccc71d265e089992786e8b6268b1b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1138,7 +1138,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC]; > struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC]; > struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC]; > - int num_ctl, num_pp, num_dsc; > + int num_pp, num_dsc, num_ctl; > unsigned int dsc_mask = 0; > int i; > > @@ -1166,7 +1166,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp, > ARRAY_SIZE(hw_pp)); > num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, > - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) > dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i]) > This chunk of diff is unnecessary. You are just changing the order of defines and fixing alignment. Does not have to be in this change.
On Mon, Dec 16, 2024 at 05:39:15PM -0800, Abhinav Kumar wrote: > > > On 12/16/2024 4:43 PM, Jessica Zhang wrote: > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources > > from dpu_crtc_assign_resources(). > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > [quic_abhinavk@quicinc.com: cleaned up formatting] > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 29 +++++++++++++++++++++++++++++ > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- > > 2 files changed, 31 insertions(+), 2 deletions(-) > > > > <snip> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > index 2b999a0558b2a016644ed5d25bf54ab45c38d1d9..a895d48fe81ccc71d265e089992786e8b6268b1b 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > @@ -1138,7 +1138,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > > struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC]; > > struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC]; > > struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC]; > > - int num_ctl, num_pp, num_dsc; > > + int num_pp, num_dsc, num_ctl; > > unsigned int dsc_mask = 0; > > int i; > > @@ -1166,7 +1166,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > > drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp, > > ARRAY_SIZE(hw_pp)); > > num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, > > - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > > + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) > > dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i]) > > > > This chunk of diff is unnecessary. You are just changing the order of > defines and fixing alignment. Does not have to be in this change. I can drop it while applying.
On Fri, Dec 20, 2024 at 04:40:52AM +0200, Dmitry Baryshkov wrote: > On Mon, Dec 16, 2024 at 05:39:15PM -0800, Abhinav Kumar wrote: > > > > > > On 12/16/2024 4:43 PM, Jessica Zhang wrote: > > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources > > > from dpu_crtc_assign_resources(). > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > [quic_abhinavk@quicinc.com: cleaned up formatting] > > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 29 +++++++++++++++++++++++++++++ > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- > > > 2 files changed, 31 insertions(+), 2 deletions(-) > > > > > > > <snip> > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > index 2b999a0558b2a016644ed5d25bf54ab45c38d1d9..a895d48fe81ccc71d265e089992786e8b6268b1b 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > @@ -1138,7 +1138,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > > > struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC]; > > > struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC]; > > > struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC]; > > > - int num_ctl, num_pp, num_dsc; > > > + int num_pp, num_dsc, num_ctl; > > > unsigned int dsc_mask = 0; > > > int i; > > > @@ -1166,7 +1166,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > > > drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp, > > > ARRAY_SIZE(hw_pp)); > > > num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, > > > - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > > > + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); > > > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) > > > dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i]) > > > > > > > This chunk of diff is unnecessary. You are just changing the order of > > defines and fixing alignment. Does not have to be in this change. > > I can drop it while applying. And with that in place: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 186ed84f59f16997716fe216e635b8dce07a63a1..b4bfded3d53025853cee112ca598533ece290318 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1183,6 +1183,7 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate) } #define MAX_HDISPLAY_SPLIT 1080 +#define MAX_CHANNELS_PER_CRTC 2 static struct msm_display_topology dpu_crtc_get_topology( struct drm_crtc *crtc, @@ -1227,9 +1228,14 @@ static struct msm_display_topology dpu_crtc_get_topology( static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state) { + struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_CRTC]; + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_CRTC]; + struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_CRTC]; + int i, num_lm, num_ctl, num_dspp; struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); struct dpu_global_state *global_state; struct msm_display_topology topology; + struct dpu_crtc_state *cstate; int ret; /* @@ -1251,6 +1257,29 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_stat if (ret) return ret; + cstate = to_dpu_crtc_state(crtc_state); + + num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, + crtc, DPU_HW_BLK_CTL, hw_ctl, + ARRAY_SIZE(hw_ctl)); + num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, + crtc, DPU_HW_BLK_LM, hw_lm, + ARRAY_SIZE(hw_lm)); + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, + crtc, DPU_HW_BLK_DSPP, hw_dspp, + ARRAY_SIZE(hw_dspp)); + + for (i = 0; i < num_lm; i++) { + int ctl_idx = (i < num_ctl) ? i : (num_ctl-1); + + cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]); + cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]); + if (i < num_dspp) + cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]); + } + + cstate->num_mixers = num_lm; + return 0; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 2b999a0558b2a016644ed5d25bf54ab45c38d1d9..a895d48fe81ccc71d265e089992786e8b6268b1b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1138,7 +1138,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC]; struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC]; struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC]; - int num_ctl, num_pp, num_dsc; + int num_pp, num_dsc, num_ctl; unsigned int dsc_mask = 0; int i; @@ -1166,7 +1166,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp, ARRAY_SIZE(hw_pp)); num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl)); for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])