Re: [PATCH v2 56/56] drm/i915/hdmi: Allow Pipe joiner for HDMI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915/hdmi: TMDS validation bypasses max plane size, breaking wide modes and FRL joiner fallback
- [High] drm/i915: NULL pointer deref in intel_hdmi_has_joiner for SDVO
--

commit ede50d511c51a1eac2ca7425fa38667d98b1e4a1
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/hdmi: Allow Pipe joiner for HDMI

Pipe joiner is used only with FRL mode, so allow it only when the platform
and the sink both support FRL.

> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
[ ... ]
> @@ -4378,3 +4378,11 @@ void intel_hdmi_frl_get_config(struct intel_crtc_state *crtc_state)
>  
>  	intel_hdmi_frl_dfm_read(crtc_state);
>  }
> +
> +bool intel_hdmi_has_joiner(struct intel_hdmi *intel_hdmi)
> +{
> +	struct intel_display *display = to_intel_display(intel_hdmi);
> +	struct intel_encoder *encoder = &hdmi_to_dig_port(intel_hdmi)->base;
> +
> +	return HAS_HDMI_FRL(display) && intel_bios_hdmi_max_frl_rate(encoder->devdata);
> +}

[Severity: High]
Does the existing TMDS validation bypass the max plane size, breaking the wide
modes and the FRL joiner fallback that this patch relies on?

In intel_hdmi_mode_valid_format(), if the TMDS clock is valid, it returns
MODE_OK immediately:

        status = intel_hdmi_tmds_mode_clock_valid(&connector->base, clock,
                                                  has_hdmi_sink, sink_format);

        if (status == MODE_OK)
                return status;

This bypasses intel_mode_valid_max_plane_size(), meaning a mode with an
hdisplay greater than 5120 but a valid TMDS clock (like 7680x1080@60Hz) is
incorrectly advertised as valid to userspace.

Then during atomic commit in intel_hdmi_compute_config(), it tries TMDS first:

        ret = intel_hdmi_compute_formats(encoder, pipe_config,
                                         conn_state, false);

        if (ret && enable_frl) {
                ret = intel_hdmi_compute_formats(encoder, pipe_config,
                                                 conn_state, true);

Since TMDS only checks the clock and ignores the plane size, it succeeds and
skips the FRL fallback. Later, intel_plane_atomic_check() calling
skl_plane_check() rejects the un-split plane because the width exceeds the max
width of 5120, returning -EINVAL. This causes atomic commits to fail for
advertised modes and prevents the driver from falling back to FRL, where the
pipe joiner could have correctly handled the wide mode.

> diff --git a/drivers/gpu/drm/i915/display/intel_joiner.c b/drivers/gpu/drm/i915/display/intel_joiner.c
> --- a/drivers/gpu/drm/i915/display/intel_joiner.c
> +++ b/drivers/gpu/drm/i915/display/intel_joiner.c
[ ... ]
> @@ -33,6 +34,9 @@ bool intel_joiner_connector_can_join(struct intel_connector *connector)
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  	case DRM_MODE_CONNECTOR_eDP:
>  		return intel_dp_has_joiner(intel_attached_dp(connector));
> +	case DRM_MODE_CONNECTOR_HDMIA:
> +	case DRM_MODE_CONNECTOR_HDMIB:
> +		return intel_hdmi_has_joiner(intel_attached_hdmi(connector));

[Severity: High]
Does this code handle SDVO HDMI connectors correctly? When this is called for
SDVO HDMI connectors on older platforms, the connector type is initialized to
DRM_MODE_CONNECTOR_HDMIA. However, SDVO HDMI connectors are not backed by an
intel_digital_port.

When intel_attached_hdmi() is called for an SDVO connector, it calls
enc_to_dig_port(), which evaluates to NULL because the encoder type is
INTEL_OUTPUT_SDVO. Then enc_to_intel_hdmi() takes the address of &NULL->hdmi,
resulting in an invalid pointer offset.

When this invalid pointer is passed to intel_hdmi_has_joiner(), the
to_intel_display() macro calls hdmi_to_dig_port(). This reverses the offset
back to NULL and dereferences it to find the device pointer, causing an
unconditional kernel panic during boot.

>  	default:
>  		return false;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012035806.3361439-1-ankit.k.nautiyal@intel.com?part=56