On Thu, 05 Nov 2026, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > Stop using the heavier intel_uncore_{read,write}() and > just use intel_uncore_{read,write}_fw() for all display > register accesses. > > The main functional change here is that we now stop > grabbing uncore.lock for display registers. Its job > is now fully taken over by the display mmio.lock. > > The one little thing we want to keep from the uncore > implementation is the runtime pm asserts (sadly xe > doesn't implement them, but i915 does). We have the > parent interface for it, so just use it. That is one > extra vfunc call, but not sure there's any way around > it. > > One somewhat unexpected side effect (or bonus) is that > on ILK we also lose ilk_dummy_write(), which is currently > done ahead of all (non-fw) mmio accesses. The only > purpose of the dummy write is to wake the GT from > RC6 ahead of the actual mmio access. Display has > nothing to do with RC6, and I don't think we actually > ever enabled RC6 on ILK, so the whole thing is pointless. > > As a result of eliminating ilk_dummy_write() display > register accesses become faster on ILK (numbers in > nanoseconds): > | fw | non-fw | > read | 140 | 430 -> 140 | > write | 170 | 470 -> 170 | > > Register accesses on TGL also get a bit faster, > compared against the original code before the > display mmio vfuncs were introduced: > | fw | non-fw | theoretical non-fw(*) > read | 230 | 970 -> 840 | 2*230+230 = 690 > write | 100 | 820 -> 740 | 2*230+100 = 560 > > (*) two FPGA_DBG reads + the actual register access > > I presume the improvement was due to some combination of > eliminating the extra debug lock and just reducing the > runtime overhead in general. The remaining delta between > theory and practice seems to be mostly due to mmio.lock. > We'll probably want to think of ways to eliminate the lock > entirely, on the platforms that don't need it for register > access serialization... > > Signed-off-by: Ville Syrjälä <[email protected]>
I guess would like to see mmio tracing pulled into display side too. Reviewed-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_de.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_de.c > b/drivers/gpu/drm/i915/display/intel_de.c > index 0a51105e0fba..397145bc7e5b 100644 > --- a/drivers/gpu/drm/i915/display/intel_de.c > +++ b/drivers/gpu/drm/i915/display/intel_de.c > @@ -9,6 +9,7 @@ > > #include "intel_de.h" > #include "intel_display_regs.h" > +#include "intel_display_rpm.h" > > static int intel_de_wait_for_register(struct intel_display *display, > intel_reg_t reg, u32 mask, u32 value, > @@ -341,11 +342,13 @@ __intel_de_read(struct intel_display *display, > intel_reg_t reg, > unsigned long irqflags; > u32 val; > > + assert_display_rpm_held(display); > + > __mmio_wakelock_pre(display, reg, mmio_flags); > __mmio_lock_pre(display, &irqflags, mmio_flags); > __mmio_unclaimed_pre(display, reg, false, mmio_flags); > > - val = intel_uncore_read(__to_uncore(display), reg); > + val = intel_de_read_fw(display, reg); > > __mmio_unclaimed_post(display, reg, false, mmio_flags); > __mmio_lock_post(display, irqflags, mmio_flags); > @@ -360,11 +363,13 @@ __intel_de_write(struct intel_display *display, > intel_reg_t reg, u32 val, > { > unsigned long irqflags; > > + assert_display_rpm_held(display); > + > __mmio_wakelock_pre(display, reg, mmio_flags); > __mmio_lock_pre(display, &irqflags, mmio_flags); > __mmio_unclaimed_pre(display, reg, true, mmio_flags); > > - intel_uncore_write(__to_uncore(display), reg, val); > + intel_de_write_fw(display, reg, val); > > __mmio_unclaimed_post(display, reg, true, mmio_flags); > __mmio_lock_post(display, irqflags, mmio_flags); -- Jani Nikula, Intel
