Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: remove unneeded clflush calls

[John Harrison <john.c.harrison@intel.com>]

On 9/15/2021 12:24, Belgaumkar, Vinay wrote:
> On 9/14/2021 12:51 PM, Lucas De Marchi wrote:
>> The clflush calls here aren't doing anything since we are not writting
>> something and flushing the cache lines to be visible to GuC. Here the
>> intention seems to be to make sure whatever GuC has written is visible
>> to the CPU before we read them. However a clflush from the CPU side is
>> the wrong instruction to use.
Is there a right instruction to use? Either we need to verify that no 
flush/invalidate is required or we need to add in a replacement that 
does the correct thing?

John.

>>
>>  From code inspection on the other clflush() calls in i915/gt/uc/ these
>> are the only ones with this behavrior. The others are apparently making
>> sure what we write is visible to GuC.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> index 65a3e7fdb2b2..2e996b77df80 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> @@ -108,7 +108,6 @@ static u32 slpc_get_state(struct intel_guc_slpc 
>> *slpc)
>>         GEM_BUG_ON(!slpc->vma);
>>   -    drm_clflush_virt_range(slpc->vaddr, sizeof(u32));
>>       data = slpc->vaddr;
>>         return data->header.global_state;
>> @@ -172,8 +171,6 @@ static int slpc_query_task_state(struct 
>> intel_guc_slpc *slpc)
>>           drm_err(&i915->drm, "Failed to query task state (%pe)\n",
>>               ERR_PTR(ret));
>>   -    drm_clflush_virt_range(slpc->vaddr, SLPC_PAGE_SIZE_BYTES);
>> -
>
> LGTM.
> Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
>>       return ret;
>>   }
>>