[3/8] drm/i915/gt: Check for conflicting RING_NONPRIV

Commit Message

Umesh Nerlige Ramappa Aug. 30, 2021, 7:38 p.m. UTC

From: Chris Wilson <[e-mail]>

Strip the encoded bits from the register offset so that we only use the
address for looking up the RING_NONPRIV entry.

Signed-off-by: Chris Wilson <[e-mail]>
Reviewed-by: Umesh Nerlige Ramappa <[e-mail]>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 66 +++++++++++++--------
 1 file changed, 42 insertions(+), 24 deletions(-)

Comments

kernel test robot Aug. 31, 2021, 3:39 a.m. UTC | #1

Hi Umesh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next next-20210830]
[cannot apply to tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Umesh-Nerlige-Ramappa/Enable-triggered-perf-query-for-Xe_HP/20210831-034105
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a004-20210830 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/81219176b43209d12b8212d1281fbfed9546087f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Umesh-Nerlige-Ramappa/Enable-triggered-perf-query-for-Xe_HP/20210831-034105
        git checkout 81219176b43209d12b8212d1281fbfed9546087f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/gt/intel_workarounds.c:88:12: warning: function 'reg_flags' is not needed and will not be emitted [-Wunneeded-internal-declaration]
   static u32 reg_flags(i915_reg_t reg)
              ^
   1 warning generated.


vim +/reg_flags +88 drivers/gpu/drm/i915/gt/intel_workarounds.c

    87	
  > 88	static u32 reg_flags(i915_reg_t reg)
    89	{
    90		return i915_mmio_reg_offset(reg) & ~RING_FORCE_TO_NONPRIV_ADDRESS_MASK;
    91	}
    92	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 6928f250cafe..df452a718200 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -80,18 +80,44 @@  static void wa_init_finish(struct i915_wa_list *wal)
 			 wal->wa_count, wal->name, wal->engine_name);
 }
 
+static u32 reg_offset(i915_reg_t reg)
+{
+	return i915_mmio_reg_offset(reg) & RING_FORCE_TO_NONPRIV_ADDRESS_MASK;
+}
+
+static u32 reg_flags(i915_reg_t reg)
+{
+	return i915_mmio_reg_offset(reg) & ~RING_FORCE_TO_NONPRIV_ADDRESS_MASK;
+}
+
+__maybe_unused
+static bool is_nonpriv_flags_valid(u32 flags)
+{
+	/* Check only valid flag bits are set */
+	if (flags & ~RING_FORCE_TO_NONPRIV_MASK_VALID)
+		return false;
+
+	/* NB: Only 3 out of 4 enum values are valid for access field */
+	if ((flags & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+	    RING_FORCE_TO_NONPRIV_ACCESS_INVALID)
+		return false;
+
+	return true;
+}
+
 static int wa_index(struct i915_wa_list *wal, i915_reg_t reg)
 {
-	unsigned int addr = i915_mmio_reg_offset(reg);
 	int start = 0, end = wal->count;
+	u32 addr = reg_offset(reg);
 
 	/* addr and wal->list[].reg, both include the R/W flags */
 	while (start < end) {
 		unsigned int mid = start + (end - start) / 2;
+		u32 pos = reg_offset(wal->list[mid].reg);
 
-		if (i915_mmio_reg_offset(wal->list[mid].reg) < addr)
+		if (pos < addr)
 			start = mid + 1;
-		else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr)
+		else if (pos > addr)
 			end = mid;
 		else
 			return mid;
@@ -117,13 +143,22 @@  static void __wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 	struct i915_wa *wa_;
 	int index;
 
+	GEM_BUG_ON(!is_nonpriv_flags_valid(reg_flags(wa->reg)));
+
 	index = wa_index(wal, wa->reg);
 	if (index >= 0) {
 		wa_ = &wal->list[index];
 
+		if (i915_mmio_reg_offset(wa->reg) !=
+		    i915_mmio_reg_offset(wa_->reg)) {
+			DRM_ERROR("Discarding incompatible w/a for reg %04x\n",
+				  reg_offset(wa->reg));
+			return;
+		}
+
 		if ((wa->clr | wa_->clr) && !(wa->clr & ~wa_->clr)) {
 			DRM_ERROR("Discarding overwritten w/a for reg %04x (clear: %08x, set: %08x)\n",
-				  i915_mmio_reg_offset(wa_->reg),
+				  reg_offset(wa_->reg),
 				  wa_->clr, wa_->set);
 
 			wa_->set &= ~wa->clr;
@@ -141,10 +176,8 @@  static void __wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 	*wa_ = *wa;
 
 	while (wa_-- > wal->list) {
-		GEM_BUG_ON(i915_mmio_reg_offset(wa_[0].reg) ==
-			   i915_mmio_reg_offset(wa_[1].reg));
-		if (i915_mmio_reg_offset(wa_[1].reg) >
-		    i915_mmio_reg_offset(wa_[0].reg))
+		GEM_BUG_ON(reg_offset(wa_[0].reg) == reg_offset(wa_[1].reg));
+		if (reg_offset(wa_[1].reg) > reg_offset(wa_[0].reg))
 			break;
 
 		swap(wa_[1], wa_[0]);
@@ -160,7 +193,7 @@  static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 	if (IS_ALIGNED(wal->count, grow) && /* Either uninitialized or full. */
 	    wa_list_grow(wal, ALIGN(wal->count + 1, grow), GFP_KERNEL)) {
 		DRM_ERROR("Unable to store w/a for reg %04x\n",
-			  i915_mmio_reg_offset(wa->reg));
+			  reg_offset(wa->reg));
 		return;
 	}
 
@@ -1367,21 +1400,6 @@  bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from)
 	return wa_list_verify(gt, &gt->i915->gt_wa_list, from);
 }
 
-__maybe_unused
-static bool is_nonpriv_flags_valid(u32 flags)
-{
-	/* Check only valid flag bits are set */
-	if (flags & ~RING_FORCE_TO_NONPRIV_MASK_VALID)
-		return false;
-
-	/* NB: Only 3 out of 4 enum values are valid for access field */
-	if ((flags & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
-	    RING_FORCE_TO_NONPRIV_ACCESS_INVALID)
-		return false;
-
-	return true;
-}
-
 static void
 whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 {