Re: [PATCH] drm/msm/dsi: setup RC model properly

[Pengyu Luo <mitltlatltl@gmail.com>]

On Sat, Mar 7, 2026 at 2:44 AM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Fri, Mar 06, 2026 at 05:25:53PM +0800, Pengyu Luo wrote:
> > Using incorrect parameters does not seem to affect the display, but we
> > should use the correct in accordance with the DSC 1.1 or 1.2.
> >
> > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c | 31 +++++++++++++++++++++++++++---
> >  1 file changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index b60b26ddb0..276c63d2ac 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -1843,10 +1843,35 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> >       drm_dsc_set_const_params(dsc);
> >       drm_dsc_set_rc_buf_thresh(dsc);
> >
> > -     /* DPU supports only pre-SCR panels */
> > -     ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> > +     if (dsc->dsc_version_major != 1) {
> > +             DRM_DEV_ERROR(&msm_host->pdev->dev, "Unsupported DSC version: %x.%x\n",
> > +                           dsc->dsc_version_major, dsc->dsc_version_minor);
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     switch (dsc->dsc_version_minor) {
> > +     case 1:
> > +             /*
> > +              * For DSC1.1. the upstream lacks SCR parameters, the downstream
> > +              * parameters are unverified here, we support pre-SCR only.
>
> It doesn't. It's the same as DRM_DSC_1_2_444. Please send a patch adding
> the comment.
>

Indeed, I didn't remember this correctly, and I recheck it now

> > +              */
> > +             ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> > +             break;
> > +     case 2:
> > +             if (dsc->native_422)
> > +                     ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_2_422);
> > +             else if (dsc->native_420)
> > +                     ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_2_420);
> > +             else
>
> It's not that we support 422/420 output... But yes, it's easier to fix
> it now.
>

Yes, this will fallback to 444 unconditionally now , since I noticed
dsc programming takes 422/420 into account, I added it here too.
But calculations don't take them into account, like we use scr_bpc * 3
in dsi_adjust_pclk_for_compression(). Should I remove them and add a
comment?

Best wishes,
Pengyu

> > +                     ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_2_444);
> > +             break;
> > +     default:
> > +             ret = -EOPNOTSUPP;
> > +     }
> > +
> >       if (ret) {
> > -             DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
> > +             DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters for DSC version: %x.%x\n",
> > +                           dsc->dsc_version_major, dsc->dsc_version_minor);
> >               return ret;
> >       }
> >
> > --
> > 2.53.0
> >
>
> --
> With best wishes
> Dmitry