Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/i915: NULL pointer deref in intel_hdmi_frl_ltsl() on systems with broken VBT - [Low] drm/i915: unused crtc_state parameter in intel_hdmi_frl_ltsl() --
commit bf532cf416aeac573dbb2724089b2e009ada86f1 Author: Ankit Nautiyal <[email protected]> drm/i915/intel_hdmi: Add helper to disable FRL This commit adds a function to reset the SCDC FRL configuration and Transcoder FRL configuration registers. > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c [ ... ] > @@ -3824,3 +3824,31 @@ int intel_hdmi_start_frl(struct intel_encoder *encoder, > > return -EINVAL; > } > + > +static void intel_hdmi_frl_ltsl(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state) [Severity: Low] Is the crtc_state parameter needed here? Looking at the function body, it appears to be passed in but never referenced. Removing it might help clean up the function signature. > +{ > + struct intel_display *display = to_intel_display(encoder); > + struct i2c_adapter *adapter = > + intel_gmbus_get_adapter(display, intel_hdmi_ddc_pin(encoder)); > + > + drm_scdc_disable_frl(adapter); [Severity: High] Could this lead to a NULL pointer dereference on systems with a misconfigured VBT? If a system has a missing or conflicting DDC pin, intel_hdmi_ddc_pin() can return 0. Consequently, intel_hdmi_init_connector() fails and no HDMI connector is registered. However, if the BIOS left the port enabled in HDMI FRL mode, the driver's hardware state readout (intel_hdmi_frl_get_config()) will still set crtc_state->frl.enable = true because it unconditionally trusts the hardware register state. When intel_sanitize_encoder() later cleans up this connector-less encoder, it calls the newly added intel_hdmi_disable_frl(). Since frl.enable is true, this invokes intel_hdmi_frl_ltsl(), which fetches the adapter for pin 0 (GMBUS_PIN_DISABLED), receiving a NULL pointer. Passing this NULL adapter to drm_scdc_disable_frl() will then cause a crash when it is subsequently dereferenced in i2c_transfer(). Should there be a NULL check for the adapter before calling drm_scdc_disable_frl()? > + drm_scdc_clear_update_flags(adapter, SCDC_FLT_UPDATE); > +} [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
