Re: [Intel-xe] [PATCH] drm/xe: fix mcr semaphore locking for MTL

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

On Wed, Jul 26, 2023 at 04:45:50PM -0700, Matt Roper wrote:
> On Wed, Jul 26, 2023 at 03:25:28PM -0700, Daniele Ceraolo Spurio wrote:
> > in commit 8604f978f45b ("drm/xe: Convert xe_mmio_wait32 to us so we can
> > stop using wait_for_us.") the mcr semaphore register read was
> > accidentally switched from waiting for the register to go to 1 to
> > waiting for the register to go to 0, so we need to flip it back.
> > 
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

I just pushed this since I have changes on top of xe_mmio_wait32 that would
conflict with this.

Thanks for the patch and review and sorry for the mistake previous patch.

> 
> > ---
> >  drivers/gpu/drm/xe/xe_gt_mcr.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
> > index c56815af0686..9eb7a6a1348d 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
> > @@ -420,10 +420,11 @@ static void mcr_lock(struct xe_gt *gt)
> >  	/*
> >  	 * Starting with MTL we also need to grab a semaphore register
> >  	 * to synchronize with external agents (e.g., firmware) that now
> > -	 * shares the same steering control register.
> > +	 * shares the same steering control register. The semaphore is obtained
> > +	 * when a read to the relevant register returns 1.
> >  	 */
> >  	if (GRAPHICS_VERx100(xe) >= 1270)
> > -		ret = xe_mmio_wait32(gt, STEER_SEMAPHORE, 0, 0x1, 10, NULL,
> > +		ret = xe_mmio_wait32(gt, STEER_SEMAPHORE, 0x1, 0x1, 10, NULL,
> >  				     true);
> >  
> >  	drm_WARN_ON_ONCE(&xe->drm, ret == -ETIMEDOUT);
> > @@ -431,7 +432,7 @@ static void mcr_lock(struct xe_gt *gt)
> >  
> >  static void mcr_unlock(struct xe_gt *gt)
> >  {
> > -	/* Release hardware semaphore */
> > +	/* Release hardware semaphore - this is done by writing 1 to the register */
> >  	if (GRAPHICS_VERx100(gt_to_xe(gt)) >= 1270)
> >  		xe_mmio_write32(gt, STEER_SEMAPHORE, 0x1);
> >  
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation