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

[Andi Shyti]

Hi John,

> > > > -		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.

You are right, the 'else' would connect with the innermost 'if'
and besides gcc complains with a warning like this:

   warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]

Thanks,
Andi