On Fri, Dec 20, 2024 at 04:32:34AM +0200, Dmitry Baryshkov wrote:
> On Mon, Dec 16, 2024 at 05:47:50PM -0800, Abhinav Kumar wrote:
> >
> >
> > On 12/16/2024 4:43 PM, Jessica Zhang wrote:
> > > From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > >
> > > All resource allocation is centered around the LMs. Then other blocks
> > > (except DSCs) are allocated basing on the LMs that was selected, and LM
> > > powers up the CRTC rather than the encoder.
> > >
> > > Moreover if at some point the driver supports encoder cloning,
> > > allocating resources from the encoder will be incorrect, as all clones
> > > will have different encoder IDs, while LMs are to be shared by these
> > > encoders.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > > [quic_abhinavk@xxxxxxxxxxx: Refactored resource allocation for CDM]
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > [quic_jesszhan@xxxxxxxxxxx: Changed to grabbing exising global state]
> > > Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 ++++++++++
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 256 ++++++++++------------------
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 8 +
> > > 3 files changed, 181 insertions(+), 169 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 9f6ffd344693ecfb633095772a31ada5613345dc..186ed84f59f16997716fe216e635b8dce07a63a1 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -1182,6 +1182,78 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
> > > return false;
> > > }
> >
> > <snip>
> >
> > > +static bool dpu_encoder_needs_dsc_merge(struct drm_encoder *drm_enc)
> > > {
> > > - struct dpu_crtc_state *cstate;
> > > - struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
> > > - struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> > > - struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC];
> > > - int num_lm, num_ctl, num_dspp, i;
> > > -
> > > - cstate = to_dpu_crtc_state(crtc_state);
> > > -
> > > - memset(cstate->mixers, 0, sizeof(cstate->mixers));
> > > -
> > > - 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));
> > > - num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > - drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
> > > - num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > - drm_enc->crtc, DPU_HW_BLK_DSPP, hw_dspp,
> > > - ARRAY_SIZE(hw_dspp));
> > > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> > > + u32 num_intf = 0;
> > > + u32 num_dsc = 0;
> > > + int i;
> > > - for (i = 0; i < num_lm; i++) {
> > > - int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> > > + for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
> > > + if (dpu_enc->phys_encs[i])
> > > + num_intf++;
> > > - 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]);
> > > - cstate->mixers[i].hw_dspp = i < num_dspp ? to_dpu_hw_dspp(hw_dspp[i]) : NULL;
> > > - }
> > > + /* We only support 2 DSC mode (with 2 LM and 1 INTF) */
> > > + if (dpu_enc->dsc)
> > > + num_dsc += 2;
> >
> > As we requested in v3, can you please explain why we have num_dsc +=2
> > instead of just num_dsc = 2? If we are hard-coding 2:2:1, this should be
> > just num_dsc = 2.
>
> I'll drop it while applying a first part of the series. Granted that
> num_dsc is initialized to 0 few lines above, it should be fine.
>
> If later there is a need to change the lane, it can be done in a
> consequent patch.
Forgot to add:
With that change,
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
--
With best wishes
Dmitry
