[v2,0/6] i915: Simplify mmio handling & add new DG2 shadow table

Message

Matt Roper Sept. 10, 2021, 8:10 p.m. UTC

Our uncore MMIO functions for reading/writing registers have become very
complicated over time.  There's significant macro magic used to generate
several nearly-identical functions that only really differ in terms of
which platform-specific shadow register table they should check on write
operations.  We can significantly simplify our MMIO handlers by storing
a reference to the current platform's shadow table within the 'struct
intel_uncore' the same way we already do for forcewake; this allows us
to consolidate the multiple variants of each 'write' function down to
just a single 'fwtable' version that gets the shadow table out of the
uncore struct rather than hardcoding the name of a specific platform's
table.  We can do similar consolidation on the MMIO read side by
creating a single-entry forcewake table to replace the open-coded range
check they had been using previously.

The final patch of the series adds a new shadow table for DG2; this
becomes quite clean and simple now, given the refactoring in the first
five patches.

Aside from simplifying the code signficantly, this series reduces the
size of the generated .ko in exchange for adding an extra pointer
indirection to access the tables.  The size deltas (for just the first
five patches, before we add an additional table in the final patch) are:

Old:
        $ size drivers/gpu/drm/i915/i915.ko
           text    data     bss     dec     hex filename
        2865921   88972    2912 2957805  2d21ed drivers/gpu/drm/i915/i915.ko

New:
        $ size drivers/gpu/drm/i915/i915.ko
           text    data     bss     dec     hex filename
        2854181   88236    2912 2945329  2cf131 drivers/gpu/drm/i915/i915.ko

The code size deltas will become larger as we add more platforms; we
already add one new platform table in the final patch of this series and
our next few platforms are all expected to bring new shadow tables as
well.

I don't think the impact of the indirect table reference for shadow
tables should be a concern for a few reasons:
 * The stored table + indirect lookup design is already deemed good
   enough for forcewake, which is used more frequently (both reads and
   writes, compared to shadow tables which are only used for writes) and
   operates on much larger tables.
 * Performance-critical sections of the code or those read/writing lots
   of registers in a batch usually do an explicit grab of the relevant
   forcewake domains and then perform their MMIO operations via *_fw()
   functions without considering shadowed registers and bypassing all of
   the table lookups.
 * In v2 of the series, we still apply NEEDS_FORCE_WAKE() checks that
   will bypass all of the forcewake and shadow logic for display
   register writes.

v2:
 - Drop orphaned definition of __gen6_reg_read_fw_domains. (Tvrtko)
 - Restore NEEDS_FORCE_WAKE() check to
   __fwtable_reg_{read,write}_fw_domains, but update the definition of
   NEEDS_FORCE_WAKE to also return 'true' on offsets above
   GEN11_BSD_RING_BASE for compatibility with gen11+ platforms. (Chris,
   Tvrtko).

Cc: Tvrtko Ursulin <[e-mail]>
Cc: Chris Wilson <[e-mail]>

Matt Roper (6):
  drm/i915/uncore: Convert gen6/gen7 read operations to fwtable
  drm/i915/uncore: Associate shadow table with uncore
  drm/i915/uncore: Replace gen8 write functions with general fwtable
  drm/i915/uncore: Drop gen11/gen12 mmio write handlers
  drm/i915/uncore: Drop gen11 mmio read handlers
  drm/i915/dg2: Add DG2-specific shadow register table

 drivers/gpu/drm/i915/intel_uncore.c           | 200 ++++++++++--------
 drivers/gpu/drm/i915/intel_uncore.h           |   7 +
 drivers/gpu/drm/i915/selftests/intel_uncore.c |   1 +
 3 files changed, 115 insertions(+), 93 deletions(-)