Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/28/2025 8:55 PM, Suraj Kandpal wrote:
> Some DSI panel vendors end up hardcoding PPS params because of which
> it does not listen to the params sent from the source. We use the
> default config tables for DSI panels when using DSC 1.1 rather than
> calculate our own rc parameters.
>
> --v2
> -Use intel_crtc_has_type [Jani]
>
> --v4
> -Use a function to check Mipi dsi dsc 1.1 condition [Ankit]
> -Add documentation for using this condition [Ankit]
> -Rebase
>
> --v5
> -Pass only the crtc_state [Jani]
> -Fixup the comment [Jani]
> -Check for dsc major version [Jani]
> -Use co-developed-by tag [Jani]
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Co-developed-by: William Tseng <william.tseng@intel.com>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_vdsc.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 3ed64c17bdff..04ba9f6b7ea2 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -259,6 +259,15 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config
>   	return 0;
>   }
>   
> +static bool is_dsi_dsc_1_1(struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> +
> +	return vdsc_cfg->dsc_version_major == 1 &&
> +		vdsc_cfg->dsc_version_minor == 1 &&
> +		intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI);
> +}
> +
>   int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
>   {
>   	struct intel_display *display = to_intel_display(pipe_config);
> @@ -317,8 +326,14 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
>   	 * From XE_LPD onwards we supports compression bpps in steps of 1
>   	 * upto uncompressed bpp-1, hence add calculations for all the rc
>   	 * parameters
> +	 *
> +	 * We also don't want to calculate all rc parameters when the panel
> +	 * is MIPI DSI and it's using DSC 1.1. The reason being that some
> +	 * DSI panels vendors have hardcoded PPS params in the VBT causing
> +	 * the parameters sent from the source to be ignore. This causes a
> +	 * noise in the display.

If I understand correctly, the issue is seen because in 
calculate_rc_params() we are interpolating some of the params for 
varying DSC variables like bits_per_pixel.

The derived params though based on the DSC1.2a C-Model, may have a bit 
difference from the original parameters set which generally is fine and 
is known to be working.

However for some DSI panels that support DSC1.1, few of the PPS params 
are hard coded. Due to this, there is some mismatch with derived rc 
parameters with the parameters the panel expects.

Furthermore for DSI panels we are currently using  bits_per_pixel 
(compressed bpp) hardcoded from VBT, (unlike other encoders where we 
find the optimum compressed bpp) so dont need to rely on interpolation, 
as we can get the required rc parameters from the tables.

So for such a case it makes sense to avoid using calculate_rc_params and 
fallback to the specific rc parameters from the table.

Perhaps we can document some of this as this is not vert explicit from 
comment.


In any case the change looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


>   	 */
> -	if (DISPLAY_VER(display) >= 13) {
> +	if (DISPLAY_VER(display) >= 13 && !is_dsi_dsc_1_1(pipe_config)) {
>   		calculate_rc_params(vdsc_cfg);
>   	} else {
>   		if ((compressed_bpp == 8 ||