On Wed, 09 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote: > On 08/02/2022 17:16, Jani Nikula wrote: >> Underscore prefix the index macros, and place >> INTEL_HWS_CSB_WRITE_INDEX() as a macro next to them, to declutter >> i915_drv.h. >> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/gt/intel_engine.h | 6 ++++-- >> drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 2 +- >> drivers/gpu/drm/i915/gvt/execlist.c | 2 +- >> drivers/gpu/drm/i915/i915_drv.h | 8 -------- >> 4 files changed, 6 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h >> index 0e353d8c2bc8..faf26ed37d01 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_engine.h >> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h >> @@ -180,8 +180,10 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value) >> #define I915_GEM_HWS_SCRATCH 0x80 >> >> #define I915_HWS_CSB_BUF0_INDEX 0x10 >> -#define I915_HWS_CSB_WRITE_INDEX 0x1f >> -#define ICL_HWS_CSB_WRITE_INDEX 0x2f >> +#define _I915_HWS_CSB_WRITE_INDEX 0x1f >> +#define _ICL_HWS_CSB_WRITE_INDEX 0x2f > > I don't quite get why would these two be the only ones which need > underscore prefix? The others are used directly, these two should only be used via INTEL_HWS_CSB_WRITE_INDEX(), like they are. That's the hint with the underscores. Matches what's done in i915_reg.h for example for register instances and choosing the right register instance. > >> +#define INTEL_HWS_CSB_WRITE_INDEX(__i915) \ >> + (GRAPHICS_VER(__i915) >= 11 ? _ICL_HWS_CSB_WRITE_INDEX : _I915_HWS_CSB_WRITE_INDEX) > > Secondly, on the point of the best new home for it, it is better than > i915_drv.h that is for sure. But is it the best I am not sure.