Re: [PATCH 3/9] drm/i915/hdmi: Restructure 4:2:0 vs. 4:4:4 mode validation

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

On 3/31/2026 5:23 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Restructure the HDMI mode validation to resemble the new
> intel_hdmi_compute_formats(). Keeping the two in sync helps
> to avoid different bugs in each.
>
> The main difference between mode_valid() and
> intel_hdmi_compute_formats() is that we don't want the
> Hail Mary RGB fallback for "4:2:0 only" modes.
>
> Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>   drivers/gpu/drm/i915/display/intel_hdmi.c | 60 ++++++++++++-----------
>   1 file changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 16873fc7bcb9..95bd38c620d1 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2045,6 +2045,27 @@ intel_hdmi_sink_format_valid(struct intel_connector *connector,
>   	}
>   }
>   
> +static enum drm_mode_status
> +intel_hdmi_mode_valid_format(struct intel_connector *connector,
> +			     const struct drm_display_mode *mode,
> +			     int clock, bool has_hdmi_sink,
> +			     enum intel_output_format sink_format)
> +{
> +	struct intel_display *display = to_intel_display(connector);
> +	enum drm_mode_status status;
> +
> +	status = intel_hdmi_sink_format_valid(connector, mode,
> +					      has_hdmi_sink, sink_format);
> +	if (status != MODE_OK)
> +		return status;
> +
> +	status = intel_pfit_mode_valid(display, mode, sink_format, 0);
> +	if (status != MODE_OK)
> +		return status;
> +
> +	return intel_hdmi_mode_clock_valid(&connector->base, clock, has_hdmi_sink, sink_format);
> +}
> +
>   static enum drm_mode_status
>   intel_hdmi_mode_valid(struct drm_connector *_connector,
>   		      const struct drm_display_mode *mode)
> @@ -2052,12 +2073,11 @@ intel_hdmi_mode_valid(struct drm_connector *_connector,
>   	struct intel_connector *connector = to_intel_connector(_connector);
>   	struct intel_display *display = to_intel_display(connector);
>   	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
> +	const struct drm_display_info *info = &connector->base.display_info;
>   	enum drm_mode_status status;
>   	int clock = mode->clock;
>   	int max_dotclk = display->cdclk.max_dotclk_freq;
>   	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->base.state);
> -	bool ycbcr_420_only;
> -	enum intel_output_format sink_format;
>   
>   	status = intel_cpu_transcoder_mode_valid(display, mode);
>   	if (status != MODE_OK)
> @@ -2084,36 +2104,20 @@ intel_hdmi_mode_valid(struct drm_connector *_connector,
>   	if (clock > 600000)
>   		return MODE_CLOCK_HIGH;
>   
> -	ycbcr_420_only = drm_mode_is_420_only(&connector->base.display_info, mode);
> +	if (drm_mode_is_420_only(info, mode)) {
> +		status = intel_hdmi_mode_valid_format(connector, mode, clock, has_hdmi_sink,
> +						      INTEL_OUTPUT_FORMAT_YCBCR420);
> +	} else {
> +		status = intel_hdmi_mode_valid_format(connector, mode, clock, has_hdmi_sink,
> +						      INTEL_OUTPUT_FORMAT_RGB);
>   
> -	if (ycbcr_420_only)
> -		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> -	else
> -		sink_format = INTEL_OUTPUT_FORMAT_RGB;
> -
> -	status = intel_pfit_mode_valid(display, mode, sink_format, 0);
> +		if (status != MODE_OK && drm_mode_is_420_also(info, mode))
> +			status = intel_hdmi_mode_valid_format(connector, mode, clock, has_hdmi_sink,
> +							      INTEL_OUTPUT_FORMAT_YCBCR420);
> +	}
>   	if (status != MODE_OK)
>   		return status;
>   
> -	status = intel_hdmi_mode_clock_valid(&connector->base, clock, has_hdmi_sink, sink_format);
> -	if (status != MODE_OK) {
> -		if (ycbcr_420_only ||
> -		    !connector->base.ycbcr_420_allowed ||
> -		    !drm_mode_is_420_also(&connector->base.display_info, mode))
> -			return status;
> -
> -		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
> -
> -		status = intel_pfit_mode_valid(display, mode, sink_format, 0);
> -		if (status != MODE_OK)
> -			return status;
> -
> -		status = intel_hdmi_mode_clock_valid(&connector->base, clock, has_hdmi_sink,
> -						     sink_format);
> -		if (status != MODE_OK)
> -			return status;
> -	}
> -
>   	return intel_mode_valid_max_plane_size(display, mode, 1);
>   }
>