Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/i915/de: NULL display->mmio.uncore when intel_de_init() is skipped --
commit 0c47f694d22974ac99844e39b462f72389dd014b Author: Ville Syrjälä <[email protected]> drm/i915/de: Introduce dislay->mmio and intel_de_init() This commit adds a display->mmio struct and intel_de_init() to initialize display->mmio.uncore, and updates __to_uncore() to return display->mmio.uncore instead of calling to_intel_uncore(display->drm). [ ... ] > diff --git a/drivers/gpu/drm/i915/display/intel_de.h > b/drivers/gpu/drm/i915/display/intel_de.h > index d0a239a369807..f1135ee5071b4 100644 > --- a/drivers/gpu/drm/i915/display/intel_de.h > +++ b/drivers/gpu/drm/i915/display/intel_de.h > @@ -14,9 +14,11 @@ > > static inline struct intel_uncore *__to_uncore(struct intel_display *display) > { > - return to_intel_uncore(display->drm); > + return display->mmio.uncore; > } [ ... ] > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c > b/drivers/gpu/drm/i915/display/intel_display_device.c > index c74256359c253..f1bd1252a9602 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.c > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > @@ -1798,6 +1798,13 @@ static void > __intel_display_device_info_runtime_init(struct intel_display *displ > struct intel_display_runtime_info *display_runtime = > DISPLAY_RUNTIME_INFO(display); > enum pipe pipe; > > + /* > + * TODO end of intel_display_device_probe() would be a more > + * logical place, but at the moment i915 calls that super > + * early when things aren't fully set up yet. > + */ > + intel_de_init(display); [Severity: High] Can this leave display->mmio.uncore NULL and cause a NULL pointer dereference when HAS_DISPLAY(display) is already false before runtime init? In drivers/gpu/drm/i915/display/intel_display_device.c, intel_display_device_info_runtime_init() only calls __intel_display_device_info_runtime_init() when HAS_DISPLAY(display) is true: void intel_display_device_info_runtime_init(struct intel_display *display) { if (HAS_DISPLAY(display)) __intel_display_device_info_runtime_init(display); On devices where intel_display_device_probe() sets DISPLAY_INFO() to no_display—such as Ivybridge Quanta server SKUs (INTEL_IVB_Q_IDS via has_no_display()), PCI IDs without an entry in intel_display_ids like Arctic Sound-M (INTEL_ATS_M_IDS where find_platform_desc() returns NULL), or GMD_ID platforms (INTEL_MTL_IDS / INTEL_ARL_IDS) where probe_gmdid_display() reads 0 from GMD_ID_DISPLAY—HAS_DISPLAY(display) is false and __intel_display_device_info_runtime_init() is skipped. Because __to_uncore(display) in drivers/gpu/drm/i915/display/intel_de.h now returns display->mmio.uncore instead of computing to_intel_uncore(display->drm), any subsequent unconditional intel_de_*() call will pass a NULL struct intel_uncore pointer to intel_uncore_read*() or intel_uncore_write*(): 1. On INTEL_IVB_Q_IDS, ilk_irq_handler() in drivers/gpu/drm/i915/i915_irq.c unconditionally calls ilk_display_irq_master_disable() and ilk_display_irq_master_enable() in drivers/gpu/drm/i915/display/intel_display_irq.c on every interrupt, which call intel_de_read_fw(display, DEIER) and intel_de_write_fw(display, DEIER, ...). 2. On INTEL_ATS_M_IDS and headless GMD_ID platforms, gen11_irq_handler() and dg1_irq_handler() unconditionally call gen11_gu_misc_irq_ack() in drivers/gpu/drm/i915/display/intel_display_irq.c when GEN11_GU_MISC_IRQ is set, which calls intel_de_read_fw(display, GEN11_GU_MISC_IIR) and intel_de_write_fw(display, GEN11_GU_MISC_IIR, iir). 3. During system resume, i915_drm_resume() in drivers/gpu/drm/i915/i915_driver.c unconditionally calls intel_gmbus_reset(display) in drivers/gpu/drm/i915/display/intel_gmbus.c, which calls intel_de_write_fw(display, GMBUS0(display), 0) and intel_de_write_fw(display, GMBUS4(display), 0). Should intel_de_init(display) be called unconditionally at the start of intel_display_device_info_runtime_init() before the HAS_DISPLAY(display) check? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
