On Sat, Mar 7, 2026 at 2:44 AM Dmitry Baryshkov
<[email protected]> 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 <[email protected]>
> > ---
> > 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
