Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm: NULL pointer dereference in intel_bios_hdmi_max_frl_rate() via devdata - [Medium] drm/i915: FRL rate incorrectly clamped to 0 when VBT specifies platform limit --
commit 7ad38de828b7ee44dcbd47b61d9385aa21e165af Author: Ankit Nautiyal <[email protected]> drm/i915/hdmi: Add new data members for FRL configuration This commit adds new members to crtc_state and intel_hdmi to identify FRL-supporting HDMI sinks. It also tracks the FRL training information after a successful link training. > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index b6fa1a1558b82..69534eea78a15 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c [ ... ] > @@ -2545,6 +2547,38 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector > *_connector) > } > } > > +static void > +intel_hdmi_reset_frl_config(struct intel_hdmi *intel_hdmi) > +{ > + intel_hdmi->frl.trained = false; > + intel_hdmi->frl.lanes = 0; > + intel_hdmi->frl.rate_gbps = 0; > + intel_hdmi->frl.ffe_level = 0; > +} > + > +static void > +intel_hdmi_init_frl_config(struct intel_connector *connector) > +{ > + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > + struct intel_encoder *encoder = &hdmi_to_dig_port(intel_hdmi)->base; > + int max_src_frl_rate = > + DIV_ROUND_UP(intel_bios_hdmi_max_frl_rate(encoder->devdata), > 1000000) * 4; [Severity: High] Is there a risk of a NULL pointer dereference here? When intel_hdmi_set_edid() detects a digital display, it calls intel_hdmi_init_frl_config() which unconditionally evaluates encoder->devdata in the call to intel_bios_hdmi_max_frl_rate(). However, the devdata field in struct intel_encoder is documented as: /* VBT information for this encoder (may be NULL for older platforms) */ If devdata is NULL, intel_bios_hdmi_max_frl_rate() will immediately dereference it: int intel_bios_hdmi_max_frl_rate(const struct intel_bios_encoder_data *devdata) { struct intel_display *display = devdata->display; ... Could this cause a crash upon connecting an HDMI display on platforms that lack VBT port data? > + int max_sink_dsc_frl_rate = > + intel_hdmi_sink_dsc_max_frl_rate(&connector->base); > + int max_sink_frl_rate = > + intel_hdmi_sink_max_frl_rate(&connector->base); > + > + intel_hdmi->has_sink_hdmi_21 = max_sink_frl_rate > 0; > + > + intel_hdmi->max_frl_rate = min(max_sink_frl_rate, > + max_src_frl_rate); [Severity: Medium] Does this clamp the maximum FRL rate to 0 Gbps for capable platforms? When the VBT specifies HDMI_MAX_FRL_RATE_PLATFORM, or when no limit is provided for platforms other than Meteorlake, intel_bios_hdmi_max_frl_rate() returns 0. This 0 value is converted to a max_src_frl_rate of 0, which is then passed into this min() assignment. As a result, intel_hdmi->max_frl_rate becomes 0. Will this unintentionally disable FRL on capable platforms that rely on the default platform limit? > + > + intel_hdmi->max_dsc_frl_rate = min(max_sink_dsc_frl_rate, > + max_src_frl_rate); > + > + intel_hdmi_reset_frl_config(intel_hdmi); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
