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 tvrtko.ursulin@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk
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(-)