Re: [Intel-gfx] [PATCH v5 2/5] drm/i915/gt: Drop invalidate_csb_entries

[Michael Cheng <michael.cheng@intel.com>]

Ah, sorry thanks for pointing that out. We did discuss previously. I 
will go ahead and change it.

On 2022-02-07 3:57 a.m., Tvrtko Ursulin wrote:
>
> On 04/02/2022 16:37, Michael Cheng wrote:
>> Drop invalidate_csb_entries and directly call drm_clflush_virt_range.
>> This allows for one less function call, and prevent complier errors when
>> building for non-x86 architectures.
>>
>> v2(Michael Cheng): Drop invalidate_csb_entries function and directly
>>            invoke drm_clflush_virt_range. Thanks to Tvrtko for the
>>            sugguestion.
>>
>> Signed-off-by: Michael Cheng <michael.cheng@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 12 +++---------
>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
>> b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> index 9bb7c863172f..7500c06562da 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> @@ -1646,12 +1646,6 @@ cancel_port_requests(struct 
>> intel_engine_execlists * const execlists,
>>       return inactive;
>>   }
>>   -static void invalidate_csb_entries(const u64 *first, const u64 *last)
>> -{
>> -    clflush((void *)first);
>> -    clflush((void *)last);
>> -}
>> -
>>   /*
>>    * Starting with Gen12, the status has a new format:
>>    *
>> @@ -1999,7 +1993,7 @@ process_csb(struct intel_engine_cs *engine, 
>> struct i915_request **inactive)
>>        * the wash as hardware, working or not, will need to do the
>>        * invalidation before.
>>        */
>> -    invalidate_csb_entries(&buf[0], &buf[num_entries - 1]);
>> +    drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0]));
>>         /*
>>        * We assume that any event reflects a change in context flow
>> @@ -2783,8 +2777,8 @@ static void reset_csb_pointers(struct 
>> intel_engine_cs *engine)
>>         /* Check that the GPU does indeed update the CSB entries! */
>>       memset(execlists->csb_status, -1, (reset_value + 1) * 
>> sizeof(u64));
>> -    invalidate_csb_entries(&execlists->csb_status[0],
>> -                   &execlists->csb_status[reset_value]);
>> +    drm_clflush_virt_range(&execlists->csb_status[0],
>> + sizeof(&execlists->csb_status[reset_value]));
>
> Hm I thought we covered this already, should be:
>
> drm_clflush_virt_range(&execlists->csb_status[0],
>                execlists->csb_size * sizeof(execlists->csb_status[0]));
>
> Regards,
>
> Tvrtko
>
>>         /* Once more for luck and our trusty paranoia */
>>       ENGINE_WRITE(engine, RING_CONTEXT_STATUS_PTR,