On Fri, Nov 06, 2026 at 05:37:59PM +0200, Jani Nikula wrote: > 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.
That'll happen eventually. The current problem is that (unline i915 core stuff) xe traces even the _fw() accesses, so if we switch to a display specific tracepoint right now we'll get double traces on xe if the xe tracpoint is also enabled. But perhaps that's fine as a temporary state of affairs? > > 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 -- Ville Syrjälä Intel
