Re: [PATCH 1/3] drm/i915/gmbus: Switch GMBUS fully to _fw() register accesses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915: accidental use of intel_de_wait_ms() defeats DC optimization
--

commit 1836a89e6bc24b1ddcaa1396b56e118dff0e2e2c
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm/i915/gmbus: Switch GMBUS fully to _fw() register accesses

This commit switches GMBUS code to use _fw() register accesses rather than
_notrace() variants. The primary goal is to avoid exiting DC5/6 during PCH
register accesses, which was causing significant performance penalties.

> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 60a70dea5d856..09171f3db298d 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c

[ ... ]

> @@ -454,7 +454,7 @@ gmbus_wait_idle(struct intel_display *display)
>  	add_wait_queue(&display->gmbus.wait_queue, &wait);
>  	intel_de_write_fw(display, GMBUS4(display), irq_enable);
>  
> -	ret = intel_de_wait_fw_ms(display, GMBUS2(display), GMBUS_ACTIVE, 0, 10, NULL);
> +	ret = intel_de_wait_ms(display, GMBUS2(display), GMBUS_ACTIVE, 0, 10, NULL);

[Severity: Medium]
Does replacing intel_de_wait_fw_ms() with intel_de_wait_ms() defeat the DC
optimization intended by this commit?

Looking at intel_de_wait_ms() in intel_de.c, it acquires wakelocks and does
standard register reads:

int intel_de_wait_ms(...)
{
    ...
    intel_dmc_wl_get(display, reg);

    ret = intel_de_wait_for_register(display, reg, mask, value,
                                     timeout_ms * 1000,
                                     intel_de_read,
                                     out_value, false);
    ...
}

Since the commit message notes that the extra north FPGA_DBG register
accesses cause DC5/6 exits, it seems calling intel_de_wait_ms() here will pull
in the uncore lock acquisitions and DMC wakelocks that force the hardware out
of DC5/6. 

Could this unintended swap cause a performance regression during display
operations that require GMBUS transfers?

>  
>  	intel_de_write_fw(display, GMBUS4(display), 0);
>  	remove_wait_queue(&display->gmbus.wait_queue, &wait);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930115303.24285-1-ville.syrjala@linux.intel.com?part=1