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 <[email protected]> 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/[email protected]?part=56
