Re: [PATCH 1/9] drm/i915/de: Introduce dislay->mmio and intel_de_init()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 04 Nov 2026, sashiko-bot@kernel.org wrote:
> 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ä <ville.syrjala@linux.intel.com>
>
> 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?

Seems annoying but valid, at least until we figure something else out.

BR,
Jani.



-- 
Jani Nikula, Intel