[Intel-xe] [PATCH 2/2] drm/xe: Drop xe_mmio_write64()

[Souza, Jose]

On Tue, 2023-08-22 at 07:15 -0700, Lucas De Marchi wrote:
> On Tue, Aug 22, 2023 at 07:12:48AM -0700, Lucas De Marchi wrote:
> > On Mon, Aug 21, 2023 at 04:43:24PM -0700, Matt Roper wrote:
> > > The only possible 64-bit register writes in the driver come from the
> > > highly questionable MMIO ioctl.  That ioctl's register write support
> > > only operates for userspace running as root and cannot be used by any
> > > real userspace; it exists solely to support the "xe_reg" debug tool in
> > 
> > mesa also uses it for some operations to avoid the extra latency of
> > setting up a bb. Have we confirmed none of those accesses are 64b?
> 
> but you are actually only removing the write side, so this is fine and
> the rest of my comment can be ignored. Sorry for the noise.

Maybe add something to the uAPI about 64bits writes not being supported? not everyone is going to read Xe KMD source code to figure it is not
supported, with that both patches are: Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> Lucas De Marchi
> 
> > 
> > humn...
> > 
> > 	mesa $ git grep DRM_XE_MMIO -- ':^include/drm-uapi/xe_drm.h'
> > 	src/intel/common/xe/intel_gem.c:      .flags = DRM_XE_MMIO_READ | DRM_XE_MMIO_64BIT,
> > 
> > Should we turn those into a 2x32 access? There was an issue in gitlab
> > about completely removing the mmio ioctl, but that got blocked because
> > of these kind of timestamps read.
> > 
> > +Jose
> > 
> > 
> > It would also be useful to check media and compute drivers
> > 
> > 
> > > IGT.  Since the spec indicates that hardware does not officially support
> > > 64-bit register accesses, there's no reason to allow such 64-bit writes,
> > > even for debugging.
> > > 
> > > Bspec: 60027
> > > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > > ---
> > > drivers/gpu/drm/xe/xe_mmio.c |  3 ---
> > > drivers/gpu/drm/xe/xe_mmio.h | 11 -----------
> > > 2 files changed, 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> > > index 932aa4ca59c8..568cd106d0ec 100644
> > > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > > @@ -490,9 +490,6 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
> > > 			}
> > > 			xe_mmio_write32(gt, reg, args->value);
> > > 			break;
> > > -		case DRM_XE_MMIO_64BIT:
> > 
> > if/when we can remove we also have to remove the DRM_XE_MMIO_64BIT
> > define
> > 
> > Lucas De Marchi
> > 
> > > -			xe_mmio_write64(gt, reg, args->value);
> > > -			break;
> > > 		default:
> > > 			drm_dbg(&xe->drm, "Invalid MMIO bit size");
> > > 			fallthrough;
> > > diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> > > index 892e96e50463..7c525d27cafb 100644
> > > --- a/drivers/gpu/drm/xe/xe_mmio.h
> > > +++ b/drivers/gpu/drm/xe/xe_mmio.h
> > > @@ -75,17 +75,6 @@ static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> > > 	return old;
> > > }
> > > 
> > > -static inline void xe_mmio_write64(struct xe_gt *gt,
> > > -				   struct xe_reg reg, u64 val)
> > > -{
> > > -	struct xe_tile *tile = gt_to_tile(gt);
> > > -
> > > -	if (reg.addr < gt->mmio.adj_limit)
> > > -		reg.addr += gt->mmio.adj_offset;
> > > -
> > > -	writeq(val, tile->mmio.regs + reg.addr);
> > > -}
> > > -
> > > /*
> > > * Intel hardware officially only supports GTTMMADR register reads of 32 bits
> > > * or smaller.  Although we often seem to get reasonable values back from a
> > > -- 
> > > 2.41.0
> > >