Re: [Intel-xe] [PATCH 06/22] drm/xe: Kernel doc GT TLB invalidations

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Feb 13, 2023 at 03:21:36PM -0800, Matt Roper wrote:
> On Fri, Feb 03, 2023 at 03:23:53PM -0500, Rodrigo Vivi wrote:
> > From: Matthew Brost <matthew.brost@intel.com>
> > 
> > Document all exported functions.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 52 ++++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > index 23094d364583..1cb4d3a6bc57 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > @@ -14,6 +14,15 @@ guc_to_gt(struct xe_guc *guc)
> >  	return container_of(guc, struct xe_gt, uc.guc);
> >  }
> >  
> > +/**
> > + * xe_gt_tlb_invalidation_init - Initialize GT TLB invalidation state
> > + * @gt: graphics tile
> > + *
> > + * Initialize GT TLB invalidation state, purely software initialization, should
> > + * be called once during driver load.
> > + *
> > + * Return: 0 on success, negative error code on error.
> > + */
> >  int xe_gt_tlb_invalidation_init(struct xe_gt *gt)
> >  {
> >  	gt->tlb_invalidation.seqno = 1;
> > @@ -24,7 +33,13 @@ int xe_gt_tlb_invalidation_init(struct xe_gt *gt)
> >  	return 0;
> >  }
> >  
> > -void xe_gt_tlb_invalidation_reset(struct xe_gt *gt)
> > +/**
> > + * xe_gt_tlb_invalidation_reset - Initialize GT TLB invalidation reset
> 
> The description here is confusing.  We're not really "initializing"
> anything here.
> 
> > + * @gt: graphics tile
> > + *
> > + * Signal any pending invalidation fences, should be called during a GT reset
> 
> Is it confirmed that a GDRST-initiated reset implicitly invalidates all
> the engine TLBs (and thus just signalling all the fences is sufficient)?
> Or does the GuC take care of this itself while it is being
> (re)initialized?  I know there are a lot of parts of the GT that don't
> actually get reset when requesting GRDOM_FULL, but it's never been very
> well documented exactly what those are.

Since I'm trying to avoid more rebases with these already reviewed and
merged patches, I created an issue to track the changes needed and to
ensure that you get the answers here:

https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/212

> 
> 
> Matt
> 
> > + */
> > + void xe_gt_tlb_invalidation_reset(struct xe_gt *gt)
> >  {
> >  	struct xe_gt_tlb_invalidation_fence *fence, *next;
> >  
> > @@ -82,6 +97,19 @@ static int send_tlb_invalidation(struct xe_guc *guc,
> >  	return ret;
> >  }
> >  
> > +/**
> > + * xe_gt_tlb_invalidation - Issue a TLB invalidation on this GT
> > + * @gt: graphics tile
> > + * @fence: invalidation fence which will be signal on TLB invalidation
> > + * completion, can be NULL
> > + *
> > + * Issue a full TLB invalidation on the GT. Completion of TLB is asynchronous
> > + * and caller can either use the invalidation fence or seqno +
> > + * xe_gt_tlb_invalidation_wait to wait for completion.
> > + *
> > + * Return: Seqno which can be passed to xe_gt_tlb_invalidation_wait on success,
> > + * negative error code on error.
> > + */
> >  int xe_gt_tlb_invalidation(struct xe_gt *gt,
> >  			   struct xe_gt_tlb_invalidation_fence *fence)
> >  {
> > @@ -100,6 +128,16 @@ static bool tlb_invalidation_seqno_past(struct xe_gt *gt, int seqno)
> >  	return false;
> >  }
> >  
> > +/**
> > + * xe_gt_tlb_invalidation_wait - Wait for TLB to complete
> > + * @gt: graphics tile
> > + * @seqno: seqno to wait which was returned from xe_gt_tlb_invalidation
> > + *
> > + * Wait for 200ms for a TLB invalidation to complete, in practice we always
> > + * should receive the TLB invalidation within 200ms.
> > + *
> > + * Return: 0 on success, -ETIME on TLB invalidation timeout
> > + */
> >  int xe_gt_tlb_invalidation_wait(struct xe_gt *gt, int seqno)
> >  {
> >  	struct xe_device *xe = gt_to_xe(gt);
> > @@ -122,6 +160,18 @@ int xe_gt_tlb_invalidation_wait(struct xe_gt *gt, int seqno)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * xe_guc_tlb_invalidation_done_handler - TLB invalidation done handler
> > + * @guc: guc
> > + * @msg: message indicating TLB invalidation done
> > + * @len: length of message
> > + *
> > + * Parse seqno of TLB invalidation, wake any waiters for seqno, and signal any
> > + * invalidation fences for seqno. Algorithm for this depends on seqno being
> > + * received in-order and asserts this assumption.
> > + *
> > + * Return: 0 on success, -EPROTO for malformed messages.
> > + */
> >  int xe_guc_tlb_invalidation_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
> >  {
> >  	struct xe_gt *gt = guc_to_gt(guc);
> > -- 
> > 2.39.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation