[PATCH] drm/i915/xe3lpd: Prune modes for YUV420

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Suraj Kandpal
Sent: Monday, June 9, 2025 9:46 AM
To: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Kandpal, Suraj <suraj.kandpal at intel.com>
Subject: [PATCH] drm/i915/xe3lpd: Prune modes for YUV420
> 
> We only support resolution up to 4k for single pipe when using
> YUV420 format so we prune these modes and restrict the plane size
> at src. This is because pipe scaling will not support YUV420 scaling
> for hwidth > 4096.
> 
> --v2
> -Use output format to check [Ville]
> -Add Bspec references
> -Modify commit messge to point to why this is needed
> 
> Bspec: 49247, 50441
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d04609460e8c..2691eeb50a26 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1419,6 +1419,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	struct intel_display *display = to_intel_display(_connector->dev);
>  	struct intel_connector *connector = to_intel_connector(_connector);
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	enum intel_output_format sink_format, output_format;
>  	const struct drm_display_mode *fixed_mode;
>  	int target_clock = mode->clock;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
> @@ -1452,6 +1453,14 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  						     mode->hdisplay, target_clock);
>  	max_dotclk *= num_joined_pipes;
>  
> +	sink_format = intel_dp_sink_format(connector, mode);
> +	output_format = intel_dp_output_format(connector, sink_format);
> +	if (num_joined_pipes == 1) {
> +		if (output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +		    mode->hdisplay > 4096)
> +			return MODE_NO_420;
> +	}

These nested if statements can probably be combined:

"""
	if (num_joined_pipes == 1 &&
	    output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
	    mode->hdisplay > 4096)
		return MODE_NO_420;
"""

However, I think this is a matter of style preference, so I won't block on it.
And if there's a practical reason the if statements are broken up like this, then
that's also good enough reason to leave it as is.

Otherwise:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> +
>  	if (target_clock > max_dotclk)
>  		return MODE_CLOCK_HIGH;
>  
> @@ -1467,11 +1476,8 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  					   intel_dp_mode_min_output_bpp(connector, mode));
>  
>  	if (intel_dp_has_dsc(connector)) {
> -		enum intel_output_format sink_format, output_format;
>  		int pipe_bpp;
>  
> -		sink_format = intel_dp_sink_format(connector, mode);
> -		output_format = intel_dp_output_format(connector, sink_format);
>  		/*
>  		 * TBD pass the connector BPC,
>  		 * for now U8_MAX so that max BPC on that platform would be picked
> -- 
> 2.34.1
> 
>