| Message ID | 20241009-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-20-v1-2-139511076a9f@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: Support different number of DSC | expand |
On Wed, 9 Oct 2024 at 09:39, Jun Nie <jun.nie@linaro.org> wrote: > > Only 2 DSC engines are allowed, or no DSC is involved currently. Can't parse this phrase. > We need 4 DSC in quad-pipe topology in future. So let's only configure > DSC engines in use, instread of maximum number of DSC engines. Nit: instead > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 39700b13e92f3..e8400b494687c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1871,10 +1871,13 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, > ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); > } > > -static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > - struct drm_dsc_config *dsc) > +static void dpu_encoder_prep_dsc(struct drm_encoder *drm_enc) > { > /* coding only for 2LM, 2enc, 1 dsc config */ > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(drm_enc->crtc->state); > + struct drm_dsc_config *dsc = dpu_enc->dsc; Why? This doesn't seem to be related to num_dscs introduction. > + int num_dsc = cstate->num_dscs; > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > struct dpu_hw_ctl *ctl = enc_master->hw_ctl; > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; [...] > @@ -1953,7 +1956,7 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) > } > > if (dpu_enc->dsc) > - dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); > + dpu_encoder_prep_dsc(drm_enc); > } > > bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc) > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 06:10写道： > > On Wed, 9 Oct 2024 at 09:39, Jun Nie <jun.nie@linaro.org> wrote: > > > > Only 2 DSC engines are allowed, or no DSC is involved currently. > > Can't parse this phrase. How about this: If DSC is enabled, the only case is with 2 DSC engines so far. > > > We need 4 DSC in quad-pipe topology in future. So let's only configure > > DSC engines in use, instread of maximum number of DSC engines. > > Nit: instead Yep. > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > index 39700b13e92f3..e8400b494687c 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > @@ -1871,10 +1871,13 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, > > ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); > > } > > > > -static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > > - struct drm_dsc_config *dsc) > > +static void dpu_encoder_prep_dsc(struct drm_encoder *drm_enc) > > { > > /* coding only for 2LM, 2enc, 1 dsc config */ > > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(drm_enc->crtc->state); > > + struct drm_dsc_config *dsc = dpu_enc->dsc; > > Why? This doesn't seem to be related to num_dscs introduction. You mean the comments above these 3 lines? Yeah, it should be removed. > > > + int num_dsc = cstate->num_dscs; > > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > > struct dpu_hw_ctl *ctl = enc_master->hw_ctl; > > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; > > [...] > > > @@ -1953,7 +1956,7 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) > > } > > > > if (dpu_enc->dsc) > > - dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); > > + dpu_encoder_prep_dsc(drm_enc); > > } > > > > bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc) > > > > -- > > 2.34.1 > > > > > -- > With best wishes > Dmitry
On Thu, 10 Oct 2024 at 04:47, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 06:10写道： > > > > On Wed, 9 Oct 2024 at 09:39, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > Only 2 DSC engines are allowed, or no DSC is involved currently. > > > > Can't parse this phrase. > > How about this: > If DSC is enabled, the only case is with 2 DSC engines so far. Just: Currently if DSC support is requested, the driver only supports using 2 DSC blocks. > > > > > > > We need 4 DSC in quad-pipe topology in future. So let's only configure > > > DSC engines in use, instread of maximum number of DSC engines. > > > > Nit: instead > > Yep. > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++++----- > > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > index 39700b13e92f3..e8400b494687c 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > @@ -1871,10 +1871,13 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, > > > ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); > > > } > > > > > > -static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > > > - struct drm_dsc_config *dsc) > > > +static void dpu_encoder_prep_dsc(struct drm_encoder *drm_enc) > > > { > > > /* coding only for 2LM, 2enc, 1 dsc config */ > > > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(drm_enc->crtc->state); > > > + struct drm_dsc_config *dsc = dpu_enc->dsc; > > > > Why? This doesn't seem to be related to num_dscs introduction. > > You mean the comments above these 3 lines? Yeah, it should be removed. No, I mean that this whole chunk isn't related to the num_dsc support. There is no need to change function arguments. > > > > > + int num_dsc = cstate->num_dscs; > > > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > > > struct dpu_hw_ctl *ctl = enc_master->hw_ctl; > > > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; > > > > [...] > > > > > @@ -1953,7 +1956,7 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) > > > } > > > > > > if (dpu_enc->dsc) > > > - dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); > > > + dpu_encoder_prep_dsc(drm_enc); > > > } > > > > > > bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc) > > > > > > -- > > > 2.34.1 > > > > > > > > > -- > > With best wishes > > Dmitry
On Wed, Oct 09, 2024 at 02:38:43PM GMT, Jun Nie wrote: > Only 2 DSC engines are allowed, or no DSC is involved currently. > We need 4 DSC in quad-pipe topology in future. So let's only configure > DSC engines in use, instread of maximum number of DSC engines. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 39700b13e92f3..e8400b494687c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1871,10 +1871,13 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, > ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); > } > > -static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > - struct drm_dsc_config *dsc) > +static void dpu_encoder_prep_dsc(struct drm_encoder *drm_enc) > { > /* coding only for 2LM, 2enc, 1 dsc config */ > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(drm_enc->crtc->state); > + struct drm_dsc_config *dsc = dpu_enc->dsc; > + int num_dsc = cstate->num_dscs; I have been thinking about this part for a while. Please move num_dscs to the dpu_encoder_virt structure. The DSC blocks are logically related to the encoder, so having this field in dpu_crtc_state seems incorrect. > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > struct dpu_hw_ctl *ctl = enc_master->hw_ctl; > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 39700b13e92f3..e8400b494687c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1871,10 +1871,13 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); } -static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, - struct drm_dsc_config *dsc) +static void dpu_encoder_prep_dsc(struct drm_encoder *drm_enc) { /* coding only for 2LM, 2enc, 1 dsc config */ + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(drm_enc->crtc->state); + struct drm_dsc_config *dsc = dpu_enc->dsc; + int num_dsc = cstate->num_dscs; struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; struct dpu_hw_ctl *ctl = enc_master->hw_ctl; struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; @@ -1886,7 +1889,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, u32 initial_lines; int i; - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { + for (i = 0; i < num_dsc; i++) { hw_pp[i] = dpu_enc->hw_pp[i]; hw_dsc[i] = dpu_enc->hw_dsc[i]; @@ -1915,7 +1918,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, enc_ip_w = intf_ip_w / 2; initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) + for (i = 0; i < num_dsc; i++) dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines); } @@ -1953,7 +1956,7 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) } if (dpu_enc->dsc) - dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); + dpu_encoder_prep_dsc(drm_enc); } bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc)
Only 2 DSC engines are allowed, or no DSC is involved currently. We need 4 DSC in quad-pipe topology in future. So let's only configure DSC engines in use, instread of maximum number of DSC engines. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)