Re: [PATCH 03/14] drm/xe: Runtime PM wake on every IOCTL

[Francois Dugast <francois.dugast@intel.com>]

On Thu, Feb 15, 2024 at 02:34:19PM -0500, Rodrigo Vivi wrote:
> Let's ensure our PCI device is awaken on every IOCTL entry.
> Let's increase the runtime_pm protection and start moving
> that to the outer bounds.
> 
> v2: minor typo fix and renaming function to make it clear
>     that is intended to be used by ioctl only. (Matt)
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Francois Dugast <francois.dugast@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_device.c | 32 ++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_pm.c     | 15 +++++++++++++++
>  drivers/gpu/drm/xe/xe_pm.h     |  1 +
>  3 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 641af3c7fd34..f763385b941b 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -141,15 +141,43 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
>  			  DRM_RENDER_ALLOW),
>  };
>  
> +static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct drm_file *file_priv = file->private_data;
> +	struct xe_device *xe = to_xe_device(file_priv->minor->dev);
> +	long ret;
> +
> +	ret = xe_pm_runtime_get_ioctl(xe);
> +	if (ret >= 0)
> +		ret = drm_ioctl(file, cmd, arg);
> +	xe_pm_runtime_put(xe);
> +
> +	return ret;
> +}
> +
> +static long xe_drm_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct drm_file *file_priv = file->private_data;
> +	struct xe_device *xe = to_xe_device(file_priv->minor->dev);
> +	long ret;
> +
> +	ret = xe_pm_runtime_get_ioctl(xe);
> +	if (ret >= 0)
> +		ret = drm_compat_ioctl(file, cmd, arg);
> +	xe_pm_runtime_put(xe);
> +
> +	return ret;
> +}
> +
>  static const struct file_operations xe_driver_fops = {
>  	.owner = THIS_MODULE,
>  	.open = drm_open,
>  	.release = drm_release_noglobal,
> -	.unlocked_ioctl = drm_ioctl,
> +	.unlocked_ioctl = xe_drm_ioctl,
>  	.mmap = drm_gem_mmap,
>  	.poll = drm_poll,
>  	.read = drm_read,
> -	.compat_ioctl = drm_compat_ioctl,
> +	.compat_ioctl = xe_drm_compat_ioctl,
>  	.llseek = noop_llseek,
>  #ifdef CONFIG_PROC_FS
>  	.show_fdinfo = drm_show_fdinfo,
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index d1c105f0eda0..b7a0e3cc594a 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -429,6 +429,21 @@ int xe_pm_runtime_put(struct xe_device *xe)
>  	return pm_runtime_put(xe->drm.dev);
>  }
>  
> +/**
> + * xe_pm_runtime_get_ioctl - Get a runtime_pm reference before ioctl
> + * @xe: xe device instance
> + *
> + * Returns: Any number greater than or equal to 0 for success, negative error
> + * code otherwise.
> + */
> +int xe_pm_runtime_get_ioctl(struct xe_device *xe)
> +{
> +	if (WARN_ON(xe_pm_read_callback_task(xe) == current))
> +		return -ELOOP;
> +
> +	return pm_runtime_get_sync(xe->drm.dev);
> +}
> +
>  /**
>   * xe_pm_runtime_get_if_active - Get a runtime_pm reference if device active
>   * @xe: xe device instance
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 75919eba1746..7f5884babb29 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -27,6 +27,7 @@ bool xe_pm_runtime_suspended(struct xe_device *xe);
>  int xe_pm_runtime_suspend(struct xe_device *xe);
>  int xe_pm_runtime_resume(struct xe_device *xe);
>  int xe_pm_runtime_get(struct xe_device *xe);
> +int xe_pm_runtime_get_ioctl(struct xe_device *xe);
>  int xe_pm_runtime_put(struct xe_device *xe);
>  int xe_pm_runtime_get_if_active(struct xe_device *xe);
>  void xe_pm_assert_unbounded_bridge(struct xe_device *xe);
> -- 
> 2.43.0
>