Re: [PATCH 10/16] drm/msm/dpu: add support to disable CDM block during encoder cleanup
On Fri, 1 Dec 2023 at 01:58, Abhinav Kumar wrote: > > > > On 8/30/2023 5:14 PM, Dmitry Baryshkov wrote: > > On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar > > wrote: > >> > >> In preparation of setting up CDM block, add the logic to disable it > >> properly during encoder cleanup. > >> > >> Signed-off-by: Abhinav Kumar > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 ++ > >> 2 files changed, 10 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> index 582680804016..1b1e07292a9e 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> @@ -26,6 +26,7 @@ > >> #include "dpu_hw_dspp.h" > >> #include "dpu_hw_dsc.h" > >> #include "dpu_hw_merge3d.h" > >> +#include "dpu_hw_cdm.h" > >> #include "dpu_formats.h" > >> #include "dpu_encoder_phys.h" > >> #include "dpu_crtc.h" > >> @@ -2097,6 +2098,13 @@ void dpu_encoder_helper_phys_cleanup(struct > >> dpu_encoder_phys *phys_enc) > >> phys_enc->hw_pp->merge_3d->idx); > >> } > >> > >> + if (phys_enc->hw_cdm && phys_enc->hw_cdm->ops.bind_pingpong_blk && > >> phys_enc->hw_pp) { > >> + phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm, > >> + false, > >> phys_enc->hw_pp->idx); > > > > But it was already bound in the cdm->enable, wasn't it? Yes :-( > > > > This is disable. I think you missed the "false" parameter. > > > Also the update_pending_flush_cdm should be called even for DPU < 5.0, > > where there should be no bind_pingpong_blk callback. > > > > Ack. This is a good catch! > > >> + if (phys_enc->hw_ctl->ops.update_pending_flush_cdm) > >> + > >> phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl); > >> + } > >> + > >> if (dpu_enc->dsc) { > >> dpu_encoder_unprep_dsc(dpu_enc); > >> dpu_enc->dsc = NULL; > >> 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 24dbc28be4f8..510c1c41ddbc 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > >> @@ -150,6 +150,7 @@ enum dpu_intr_idx { > >>* @hw_pp: Hardware interface to the ping pong registers > >>* @hw_intf: Hardware interface to the intf registers > >>* @hw_wb: Hardware interface to the wb registers > >> + * @hw_cdm:Hardware interface to the CDM registers > >>* @dpu_kms: Pointer to the dpu_kms top level > >>* @cached_mode: DRM mode cached at mode_set time, acted on in > >> enable > >>* @enabled: Whether the encoder has enabled and running a mode > >> @@ -178,6 +179,7 @@ struct dpu_encoder_phys { > >> struct dpu_hw_pingpong *hw_pp; > >> struct dpu_hw_intf *hw_intf; > >> struct dpu_hw_wb *hw_wb; > >> + struct dpu_hw_cdm *hw_cdm; > >> struct dpu_kms *dpu_kms; > >> struct drm_display_mode cached_mode; > >> enum dpu_enc_split_role split_role; > >> -- > >> 2.40.1 > >> > > > > -- With best wishes Dmitry
Re: [PATCH 10/16] drm/msm/dpu: add support to disable CDM block during encoder cleanup
On 8/30/2023 5:14 PM, Dmitry Baryshkov wrote: On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar wrote: In preparation of setting up CDM block, add the logic to disable it properly during encoder cleanup. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 582680804016..1b1e07292a9e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -26,6 +26,7 @@ #include "dpu_hw_dspp.h" #include "dpu_hw_dsc.h" #include "dpu_hw_merge3d.h" +#include "dpu_hw_cdm.h" #include "dpu_formats.h" #include "dpu_encoder_phys.h" #include "dpu_crtc.h" @@ -2097,6 +2098,13 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc) phys_enc->hw_pp->merge_3d->idx); } + if (phys_enc->hw_cdm && phys_enc->hw_cdm->ops.bind_pingpong_blk && phys_enc->hw_pp) { + phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm, + false, phys_enc->hw_pp->idx); But it was already bound in the cdm->enable, wasn't it? This is disable. I think you missed the "false" parameter. Also the update_pending_flush_cdm should be called even for DPU < 5.0, where there should be no bind_pingpong_blk callback. Ack. This is a good catch! + if (phys_enc->hw_ctl->ops.update_pending_flush_cdm) + phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl); + } + if (dpu_enc->dsc) { dpu_encoder_unprep_dsc(dpu_enc); dpu_enc->dsc = NULL; 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 24dbc28be4f8..510c1c41ddbc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -150,6 +150,7 @@ enum dpu_intr_idx { * @hw_pp: Hardware interface to the ping pong registers * @hw_intf: Hardware interface to the intf registers * @hw_wb: Hardware interface to the wb registers + * @hw_cdm:Hardware interface to the CDM registers * @dpu_kms: Pointer to the dpu_kms top level * @cached_mode: DRM mode cached at mode_set time, acted on in enable * @enabled: Whether the encoder has enabled and running a mode @@ -178,6 +179,7 @@ struct dpu_encoder_phys { struct dpu_hw_pingpong *hw_pp; struct dpu_hw_intf *hw_intf; struct dpu_hw_wb *hw_wb; + struct dpu_hw_cdm *hw_cdm; struct dpu_kms *dpu_kms; struct drm_display_mode cached_mode; enum dpu_enc_split_role split_role; -- 2.40.1
Re: [PATCH 10/16] drm/msm/dpu: add support to disable CDM block during encoder cleanup
On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar wrote: > > In preparation of setting up CDM block, add the logic to disable it > properly during encoder cleanup. > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 582680804016..1b1e07292a9e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -26,6 +26,7 @@ > #include "dpu_hw_dspp.h" > #include "dpu_hw_dsc.h" > #include "dpu_hw_merge3d.h" > +#include "dpu_hw_cdm.h" > #include "dpu_formats.h" > #include "dpu_encoder_phys.h" > #include "dpu_crtc.h" > @@ -2097,6 +2098,13 @@ void dpu_encoder_helper_phys_cleanup(struct > dpu_encoder_phys *phys_enc) > phys_enc->hw_pp->merge_3d->idx); > } > > + if (phys_enc->hw_cdm && phys_enc->hw_cdm->ops.bind_pingpong_blk && > phys_enc->hw_pp) { > + phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm, > + false, > phys_enc->hw_pp->idx); But it was already bound in the cdm->enable, wasn't it? Also the update_pending_flush_cdm should be called even for DPU < 5.0, where there should be no bind_pingpong_blk callback. > + if (phys_enc->hw_ctl->ops.update_pending_flush_cdm) > + > phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl); > + } > + > if (dpu_enc->dsc) { > dpu_encoder_unprep_dsc(dpu_enc); > dpu_enc->dsc = NULL; > 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 24dbc28be4f8..510c1c41ddbc 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > @@ -150,6 +150,7 @@ enum dpu_intr_idx { > * @hw_pp: Hardware interface to the ping pong registers > * @hw_intf: Hardware interface to the intf registers > * @hw_wb: Hardware interface to the wb registers > + * @hw_cdm:Hardware interface to the CDM registers > * @dpu_kms: Pointer to the dpu_kms top level > * @cached_mode: DRM mode cached at mode_set time, acted on in enable > * @enabled: Whether the encoder has enabled and running a mode > @@ -178,6 +179,7 @@ struct dpu_encoder_phys { > struct dpu_hw_pingpong *hw_pp; > struct dpu_hw_intf *hw_intf; > struct dpu_hw_wb *hw_wb; > + struct dpu_hw_cdm *hw_cdm; > struct dpu_kms *dpu_kms; > struct drm_display_mode cached_mode; > enum dpu_enc_split_role split_role; > -- > 2.40.1 > -- With best wishes Dmitry