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

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

On 2/27/2025 4:56 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]
>
> --v3
> -Add Signed-off-by from William too [Ankit]
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: William Tseng <william.tseng@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 6e7151346382..affe9913f1ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -320,7 +320,9 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
>   	 * upto uncompressed bpp-1, hence add calculations for all the rc
>   	 * parameters
>   	 */
> -	if (DISPLAY_VER(dev_priv) >= 13) {
> +	if (DISPLAY_VER(dev_priv) >= 13 &&
> +	    (vdsc_cfg->dsc_version_minor != 1 ||
> +	     intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {

This should be !intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI)

I think it would be better to use a function for special handling for 
DSI panel with DSC1.1.

(I am not very sure what should be an appropriate name for this but just 
to give an example)

bool is_mipi_dsi_dsc_1_1()
{
	return vdsc_cfg->dsc_version_minor == 1 &&
		   intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI);
}

The condition will then become:

if (DISPLAY_VER(dev_priv) >= 13 && !is_mipi_dsi_dsc_1_1())
	calculate_rc_params(vdsc_cfg);


With this we also need to document about why we are not using calculate_rc_params for MIPI DSI with DSC1.1 in comment above the function.

Regards,
Ankit

>   		calculate_rc_params(vdsc_cfg);
>   	} else {
>   		if ((compressed_bpp == 8 ||