Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation functions

[Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>]

On Fri, 5 Aug 2022 10:24:25 +0100
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

> On 05/08/2022 10:08, Andi Shyti wrote:
> > Hi Randy,
> >   
> >>> +/**
> >>> + * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
> >>> + * @gt: GT structure  
> >>
> >> In multiple places (here and below) it would be nice to know what a
> >> GT structure is. I looked thru multiple C and header files yesterday
> >> and didn't find any comments about it.
> >>
> >> Just saying that @gt is a GT structure isn't very helpful, other
> >> than making kernel-doc shut up.  
> > 
> > the 'gt' belongs to the drivers/gpu/drm/i915/gt/ subsystem and
> > it's widely used a throughout i915.
> > 
> > I think it's inappropriate to describe it just here. On the other
> > hand I agree that a better documentation is required for the GT
> > itself where other parts can point to.  

GT is actually a well-understood term for GPU developers. It is an alias
for:

	https://en.wikipedia.org/wiki/Intel_Graphics_Technology

It is basically the "core" of the GPU, where the engine units sit.

I agree with Andi: terms like this should likely be defined on a glossary
at i915.rst file.

> Yeah agreed there is no point of copy pasting some explanation all over 
> the place. Could we just do s/GT structure/struct intel_gt/, or "pointer 
> to struct intel_gt to operate on", would that be good enough?

IMO, it won't make any difference. kerneldoc already says that the
parameter has struct intel_gt type on its output:

	.. c:function:: void intel_gt_fini_tlb (struct intel_gt *gt)

	   free TLB-specific vars

	**Parameters**

	``struct intel_gt *gt``
	  GT structure
	
	**Description**

	Frees any resources needed by TLB cache invalidation logic.

This struct somewhat is similar to struct device. This is a container
struct that has the common data needed to control the GT hardware.

Almost all functions that work with GT needs it. There's not much sense
to describe it everywhere. What makes sense is to have struct intel_gt
documented at intel_gt_types.h, letting the build system to generate 
hiperlinks to it.

This is easier said than done...

Regards,
Mauro