Re: [Intel-xe] [PATCH 2/6] drm/xe: Use full_gt batchbuffer allocation for media tiles.

[Matt Roper <matthew.d.roper@intel.com>]

On Tue, Apr 04, 2023 at 08:59:27AM +0200, Maarten Lankhorst wrote:
> 
> On 2023-04-03 22:27, Matt Roper wrote:
> > On Fri, Mar 31, 2023 at 12:24:15PM +0200, Maarten Lankhorst wrote:
> > > This fixes an oops on media tiles, because we don't initialise the
> > > bb pool.
> > > ---
> > >   drivers/gpu/drm/xe/xe_gt.c | 10 +++++++---
> > >   1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > index bc821f431c45..c7a2e9baabfb 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > @@ -237,6 +237,10 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
> > >   		struct xe_engine *e, *nop_e;
> > >   		struct xe_vm *vm;
> > >   		void *default_lrc;
> > > +		struct xe_gt *full_gt = gt;
> > > +
> > > +		if (xe_gt_is_media_type(gt))
> > > +			full_gt = xe_find_full_gt(gt);
> > >   		if (gt->default_lrc[hwe->class])
> > >   			continue;
> > > @@ -262,7 +266,7 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
> > >   		}
> > >   		/* Prime golden LRC with known good state */
> > > -		err = emit_wa_job(gt, e);
> > > +		err = emit_wa_job(full_gt, e);
> > If we pass the primary GT here, then we'll take the wrong path in
> > bb_prefetch().  That doesn't actually matter very much (we'll just
> > overestimate the padding needed for media engines, which is safe and
> > something we already do for BCS engines), but we should probably just
> > drop !xe_gt_is_media_type(gt) condition there as part of this patch
> > since it will always be true now.
> 
> Yeah makes sense. That check doesn't strictly speaking seem to be needed, so
> I'm fine with dropping it.
> 
> We could always request the engine to be specified, but I don't think we
> really use those batchbuffers beyond initialisation outside of
> migration/VM_BIND,
> 
> which is done on a BCS engine anyway.
> 
> Can I have your r-b with that check removed?

Yeah,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks.


Matt

> 
> ~Maarten
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation