[Intel-xe] [PATCH v15 05/10] drm/xe/mmio: grab mem_access in xe_mmio_ioctl

[Matt Roper]

On Wed, Jul 19, 2023 at 09:38:07AM +0100, Matthew Auld wrote:
> Any kind of device memory access should first ensure the device is not
> suspended, mmio included.

I know things already went off track quite a while back, but it sounds
like this is taking us even farther down the wrong path.
xe_device_mem_access_get was intended as a place to hook in things like
the ugly workarounds that we used to have on early PVC steppings where
we need special care when the CPU might access VRAM.  Although we need
to be out of D3 to access VRAM, we shouldn't be using
xe_device_mem_access for general D3 management that's unrelated to VRAM
access; we should be using actual runtime PM for that (and then
xe_device_mem_access_get should either have an assertion to ensure that
runtime PM was already grabbed at a higher level, or it should grab its
own wakeref).

As far as I know, we don't actually have anything that needs
xe_device_mem_access right now (since we're not supporting old
pre-production workarounds for PVC).  In theory we should be able to
just rip it out of the driver completely with no impact as long we're
handling device-level runtime PM and GT-level forcewake properly
everywhere.

If we're too far down this path to fix things now and want to just
repurpose xe_device_mem_access as an alternate reference counting layer
over top of runtime PM, then we should probably rename it to something
that more accurately reflects what we're using it for now.  And then we
can re-add an actual memory access layer independently if/when it
becomes necessary to deal with future hardware ugliness.


Matt

> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Reviewed-by: Matthew Brost <matthew.brost at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_mmio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index 448b874c7a3c..8d0f07261bfd 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -483,6 +483,7 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
>  	 */
>  	reg = XE_REG(args->addr);
>  
> +	xe_device_mem_access_get(xe);
>  	xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  
>  	if (args->flags & DRM_XE_MMIO_WRITE) {
> @@ -526,6 +527,7 @@ int xe_mmio_ioctl(struct drm_device *dev, void *data,
>  
>  exit:
>  	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	xe_device_mem_access_put(xe);
>  
>  	return ret;
>  }
> -- 
> 2.41.0
> 

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