[Intel-gfx] [PATCH 6/9] drm/i915/execlists: Reset CSB write pointer after reset

[Tvrtko Ursulin]

On 28/06/2018 11:17, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-06-28 11:06:25)
>>
>> On 27/06/2018 22:07, Chris Wilson wrote:
>>> On HW reset, the HW clears the write pointer (to 0). But since it also
>>> writes its first CSB entry to slot 0, we need to reset the write pointer
>>> back to the element before (so the first entry we read is 0).
>>>
>>> This is required for the next patch, where we trust the CSB completely!
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_lrc.c | 19 +++++++++++++++++--
>>>    1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 368a8c74d11d..8b111a268697 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -884,6 +884,21 @@ static void reset_irq(struct intel_engine_cs *engine)
>>>        clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>>>    }
>>>    
>>> +static void reset_csb_pointers(struct intel_engine_execlists *execlists)
>>> +{
>>> +     /*
>>> +      * After a reset, the HW starts writing into CSB entry [0]. We
>>> +      * therefore have to set our HEAD pointer back one entry so that
>>> +      * the *first* entry we check is entry 0. To complicate this further,
>>> +      * as we don't wait for the first interrupt after reset, we have to
>>> +      * fake the HW write to point back to the last entry so that our
>>> +      * inline comparison of our cached head position against the last HW
>>> +      * write works even before the first interrupt.
>>> +      */
>>> +     execlists->csb_head = GEN8_CSB_ENTRIES - 1;
>>> +     WRITE_ONCE(*execlists->csb_write, (GEN8_CSB_ENTRIES - 1) | 0xff << 16);
>>
>> Use _MASKED_FIELD and GEN8_CSB_WRITE_PTR_MASK?
> 
> Hah, there goes my attempt to kill off unused magic.

At least _MASKED_FIELD makes it clearer.

But the u8 trick is still evil since here you even explicitly do a fake 
masked write on hwsp. Ugly and evil. How about storing 
execlists->csb_write_default at init time and applying it here?

Regards,

Tvrtko