[Intel-gfx] [PATCH 06/11] drm/i915/execlists: Unify CSB access pointers

[Daniele Ceraolo Spurio]

On 04/06/18 07:53, Tvrtko Ursulin wrote:
> 
> [CC Daniele for his great documentation searching skills.]
> 
> On 31/05/2018 19:51, Chris Wilson wrote:
>> Following the removal of the last workarounds, the only CSB mmio access
>> is for the old vGPU interface. The mmio registers presented by vGPU do
>> not require forcewake and can be treated as ordinary volatile memory,
>> i.e. they behave just like the HWSP access just at a different location.
>> We can reduce the CSB access inside the irq handler to a set of
>> read/write/buffer pointers and treat the various paths identically and
>> not worry about forcewake.
>>
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/intel_engine_cs.c  |  12 ---
>>   drivers/gpu/drm/i915/intel_lrc.c        | 116 ++++++++++--------------
>>   drivers/gpu/drm/i915/intel_ringbuffer.h |  23 +++--
>>   3 files changed, 65 insertions(+), 86 deletions(-)
> 
> [snip]
> 
>> @@ -1103,16 +1083,11 @@ static void process_csb(struct intel_engine_cs 
>> *engine)
>>           } else {
>>               port_set(port, port_pack(rq, count));
>>           }
>> -    }
>> -
>> -    if (head != execlists->csb_head) {
>> -        execlists->csb_head = head;
>> -        writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK, head << 8),
>> -               i915->regs + 
>> i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
>> -    }
>> +    } while (head != tail);
>> -    if (unlikely(fw))
>> -        intel_uncore_forcewake_put(i915, execlists->fw_domains);
>> +    writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK, head << 8),
>> +           execlists->csb_read);
> 
> This is the write of RING_CONTEXT_STATUS_PTR and the mystery is whether 
> it is or isn't a shadowed register. We don't have it in the shadowed 
> list in intel_uncore.c, but we stopped taking forcewake for it after 
> HWSP conversion and it seems to work regardless. I think if we could 
> find that it is officially shadowed it would be good to put it in the 
> list so it is documented properly in code.
> 

I couldn't find the list of shadowed registers for gen8-9, but the gen10 
list (Bspec: 18333) seem to indicate that among the registers involved 
here only ELSP and TAIL are shadowed.

AFAIK up to gen10 the write of head to RING_CONTEXT_STATUS_PTR is for SW 
use to track the head position of the next CSB to read, HW only cares 
about the tail, so given that on non-gvt we only read back the register 
after a reset we shouldn't have issues even if a write is missed (logs 
aside). With gen11 there is a new possible usage of the head value with 
the new CSB_STATUS register, which can be used to read the CSB pointed 
by head and then automatically bump the head value. Using this register 
would require forcewake and would also deprecate the manual head update 
so we'd be covered if we ever wanted to use it.

Daniele

> Regards,
> 
> Tvrtko