Re: [PATCH 2/3] drm/i915/tgl: Implement Wa_1604555607

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: Michel Thierry <michel.thierry@intel.com>
Subject: Re: [PATCH 2/3] drm/i915/tgl: Implement Wa_1604555607
Date: Fri, 26 Jul 2019 14:15:51 +0100	[thread overview]
Message-ID: <54eaffac-0d15-109d-6d9b-98eb6b2a604f@linux.intel.com> (raw)
In-Reply-To: <156409982772.30723.16146937485087701539@skylake-alporthouse-com>

On 26/07/2019 01:10, Chris Wilson wrote:
> Quoting Lucas De Marchi (2019-07-26 01:02:25)
>> From: Michel Thierry <michel.thierry@intel.com>
>>
>> Implement Wa_1604555607 (set the DS pairing timer to 128 cycles).
>> FF_MODE2 is part of the register state context, that's why it is
>> implemented here.
>>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +++++++
>>   drivers/gpu/drm/i915/i915_reg.h             | 5 +++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> index a6eb9c6e87ec..3235ef355dfd 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> @@ -572,6 +572,13 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
>>   static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>>                                       struct i915_wa_list *wal)
>>   {
>> +       u32 val;
>> +
>> +       /* Wa_1604555607:tgl */
>> +       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);
> 
> It will do a rmw on application, so you just need
> 	wa_write_masked_or(wal, FF_MODE2,
> 		           FF_MODE2_TDS_TIMER_MASK, FF_MODE2_TDS_TIMER_128);

Not with ctx was unfortunately, no rmw there, just lri.

To be less misleading perhaps:

   wa_write(wal, FF_MODE2, val);

?

It only affects verification, do we want to verify the whole register or 
just the FF_MODE2_TDS_TIMER_MASK bits. Since the code reads it and sets 
it, it may want to check it whole.

Regards,

Tvrtko

> 
>>   }
>>   
>>   static void
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 54ea250000be..fbbb89f6ca2f 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7771,6 +7771,11 @@ enum {
>>   #define   PIXEL_ROUNDING_TRUNC_FB_PASSTHRU     (1 << 15)
>>   #define   PER_PIXEL_ALPHA_BYPASS_EN            (1 << 7)
>>   
>> +#define FF_MODE2                       _MMIO(0x6604)
>> +#define   FF_MODE2_TDS_TIMER_SHIFT     (16)
>> +#define   FF_MODE2_TDS_TIMER_128       (4 << FF_MODE2_TDS_TIMER_SHIFT)
>> +#define   FF_MODE2_TDS_TIMER_MASK      (0xff << FF_MODE2_TDS_TIMER_SHIFT)
> 
> #define FF_MODE2_TDS_TIMER_MASK REG_GENMASK(23, 16)
> #define FF_MODE2_TDS_TIMER_128 REG_FIELD_PREP(FF_MODE2_TDS_TIMER_MASK, 4)
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx