[Intel-gfx] [PATCH 2/2] drm/i915: Skip the Wa_1604555607 verification

[Lucas De Marchi]

On Wed, Nov 20, 2019 at 05:50:51PM +0000, Tvrtko Ursulin wrote:
>
>On 20/11/2019 17:31, Ramalingam C wrote:
>>At TGL A0 stepping, FF_MODE2 register read back is broken, hence
>>disabling the WA verification.
>>
>>Helper function called wa_write_masked_or_no_verify is defined for the
>>same purpose.
>>
>>Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
>>cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>---
>>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>index 93efefa205d6..1698330c6f23 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>@@ -160,6 +160,20 @@ wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
>>  	_wa_add(wal, &wa);
>>  }
>>+static void
>>+wa_write_masked_or_no_verify(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
>>+		   u32 val)
>>+{
>>+	struct i915_wa wa = {
>>+		.reg  = reg,
>>+		.mask = mask,
>>+		.val  = val,
>>+		.read = 0,
>>+	};
>>+
>>+	_wa_add(wal, &wa);
>>+}
>>+
>>  static void
>>  wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>>  {
>>@@ -578,7 +592,11 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>>  	val = intel_uncore_read(engine->uncore, FF_MODE2);
>>  	val &= ~FF_MODE2_TDS_TIMER_MASK;
>>  	val |= FF_MODE2_TDS_TIMER_128;
>>-	wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val);
>>+	if (IS_TGL_REVID(engine->uncore->i915, 0, TGL_REVID_A0))
>
>There is engine->i915.
>
>>+		wa_write_masked_or_no_verify(wal, FF_MODE2,
>>+					     FF_MODE2_TDS_TIMER_MASK, val);
>>+	else
>>+		wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val);
>
>You did not think suggested alternative where read mask is directly 
>passed in would work better? It would read a bit more compact:
>
>  __wa_write_masked_or(..., IS_TGL_REVID(..) ? 0 : val)
>
>Up to you what you prefer, I guess the explicit _no_verify brings some 
>self-documenting benefits.
>
>Also, do you think there is value in having two patches instead of 
>just squashing into one? I would be tempted to squash.

yep, either squash or reorder the patches (the first one being about the
helper only) so we don't have a broken behavior in this point of the
branch.

Lucas De Marchi

>
>Regards,
>
>Tvrtko
>
>>  }
>>  static void
>>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx