Re: [Intel-gfx] [PATCH v3 1/4] drm/i915: Define and use GuC and CTB TLB invalidation routines

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

On 10/3/2023 09:41, Andi Shyti wrote:
> [...]
>
>>> -		mmio_invalidate_full(gt);
>>> +		if (INTEL_GUC_SUPPORTS_TLB_INVALIDATION(guc)) {
>>> +			if (intel_guc_is_ready(guc))
>>> +				intel_guc_invalidate_tlb_full(guc);
>>> +		} else {
>>> +			/*
>>> +			 * Fall back to old path if GuC is disabled.
>>> +			 * This is safe because GuC is not enabled and not writing to MMIO.
>>> +			 */
>> It is safe for intel_guc_is_ready() transitioning from false to true during GuC init? No way for some path to start issuing invalidations as that is happening?
>>
>>> +			mmio_invalidate_full(gt);
>>> +		}
> supernitpick: as we are at this, brackets are not required.
Braces are required on the first half of the 'if' because it is a double 
if and the else applies to the top level not the inner level. And my 
understanding of the style guide is that lop-sided bracing is incorrect. 
i.e. never have "} else". Plus while it might be syntactically valid to 
not have braces around the five line else clause because it is only one 
actual code statement, it massively helps readability of the code to 
have the braces present.

John.

>
> Andi