[Intel-gfx] [PATCH v7 2/5] drm/i915: Define and use GuC and CTB TLB invalidation routines

[John Harrison]

On 10/6/2023 03:20, Nirmoy Das wrote:
>
> On 10/6/2023 12:11 PM, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>>
>> Andi asked me to summarize what I think is unaddressed review 
>> feedback so far in order to consolidate and enable hopefully things 
>> to move forward. So I will try to re-iterate the comments and 
>> questions below.
>>
>> But also note that there is a bunch of new valid comments from John 
>> against v7 which I will not repeat.
>>
>> On 05/10/2023 20:35, Jonathan Cavitt wrote:
>>> ...
>>> +enum intel_guc_tlb_invalidation_type {
>>> +    INTEL_GUC_TLB_INVAL_FULL = 0x0,
>>> +    INTEL_GUC_TLB_INVAL_GUC = 0x3,
>>
>> New question - are these names coming from the GuC iface? I find it 
>> confusing that full does not include GuC but maybe it is just me. So 
>> maybe full should be called GT or something? Although then again it 
>> wouldn't be clear GT does not include the GuC.. bummer. GPU? Dunno. 
>> Minor confusion I guess so can keep as is.
>
> I agree this is bit confusing name. We are using 
> INTEL_GUC_TLB_INVAL_GUC to invalidate ggtt, how about 
> INTEL_GUC_TLB_INVAL_GGTT ?
>
The GuC interface spec says:

    GUC_TLB_INV_TYPE_TLB_INV_FULL_INTRA_VF = 0x00
    Full TLB invalidation within a VF. Invalidates VF’s TLBs only if
    that VF is active, will invalidate across all engines.

    GUC_TLB_INV_TYPE_TLB_INV_GUC = 0x03
    Guc TLB Invalidation.


So the 'GUC' type is not GGTT, it is the TLBs internal to GuC itself is 
how I would read the above. Whereas 'FULL' is everything that is per-VF, 
aka everything in the GT that is beyond the GuC level - i.e. the 
engines, EUs and everything from there on.

So I think the INVAL_GUC name is correct. But maybe INVAL_FULL should be 
called INVAL_VF? Or INVAL_ENGINES if you don't like using the VF term in 
a non-SRIOV capable driver?

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20231006/3c890233/attachment-0001.htm>