drm/msm/dsi: setup RC model properly

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

Posted by Pengyu Luo 1 month ago

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.
+		 */
+		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
+			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

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

Posted by Dmitry Baryshkov 1 month ago

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.

> +		 */
> +		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.

> +			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

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

Posted by Pengyu Luo 1 month ago

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