Re: [PATCH] drm/msm/gpu: Add submit queue queries

[Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On Mon, Apr 30, 2018 at 03:20:53PM -0600, Jordan Crouse wrote:
> Add the capability to query information from a submit queue.
> The first available parameter is for querying the number of
> GPU faults (hangs) that can be attributed to the queue.
> 
> This is useful for implementing context robustness. A UMD
> context can regularly query the number of faults to see
> if it is responsible for any. If so it can invalidate itself.
> 
> This is also helpful for testing by confirming to the user mode
> driver if a particular command stream caused a fault (or not as
> the case may be).
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/msm_drv.c         | 12 +++++++++++-
>  drivers/gpu/drm/msm/msm_drv.h         |  2 ++
>  drivers/gpu/drm/msm/msm_gpu.c         |  3 +++
>  drivers/gpu/drm/msm/msm_submitqueue.c | 21 +++++++++++++++++++++
>  include/uapi/drm/msm_drm.h            | 12 ++++++++++++
>  5 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 30cd514d8f7c..d01fb101d5ff 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -32,9 +32,10 @@
>   * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW +
>   *           SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for
>   *           MSM_GEM_INFO ioctl.
> + * - 1.4.0 - Add SUBMITQUERY_QUERY ioctl.
>   */
>  #define MSM_VERSION_MAJOR	1
> -#define MSM_VERSION_MINOR	3
> +#define MSM_VERSION_MINOR	4
>  #define MSM_VERSION_PATCHLEVEL	0
>  
>  static const struct drm_mode_config_funcs mode_config_funcs = {
> @@ -803,6 +804,14 @@ static int msm_ioctl_submitqueue_new(struct drm_device *dev, void *data,
>  		args->flags, &args->id);
>  }
>  
> +static int msm_ioctl_submitqueue_query(struct drm_device *dev, void *data,
> +		struct drm_file *file)
> +{
> +	struct drm_msm_submitqueue_query *args = data;
> +
> +	return msm_submitqueue_query(dev, file->driver_priv, args->id,
> +		args->param, args->data, args->len);
> +}
>  
>  static int msm_ioctl_submitqueue_close(struct drm_device *dev, void *data,
>  		struct drm_file *file)
> @@ -823,6 +832,7 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
>  	DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE,  msm_ioctl_gem_madvise,  DRM_AUTH|DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW,   msm_ioctl_submitqueue_new,   DRM_AUTH|DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close, DRM_AUTH|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, DRM_AUTH|DRM_RENDER_ALLOW),
>  };
>  
>  static const struct vm_operations_struct vm_ops = {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 48ed5b9a8580..56c666f25aa1 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -320,6 +320,8 @@ struct msm_gpu_submitqueue *msm_submitqueue_get(struct msm_file_private *ctx,
>  		u32 id);
>  int msm_submitqueue_create(struct drm_device *drm, struct msm_file_private *ctx,
>  		u32 prio, u32 flags, u32 *id);
> +int msm_submitqueue_query(struct drm_device *drm, struct msm_file_private *ctx,
> +		u32 id, u32 param, u64 data, u32 len);
>  int msm_submitqueue_remove(struct msm_file_private *ctx, u32 id);
>  void msm_submitqueue_close(struct msm_file_private *ctx);
>  
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 1c09acfb4028..925532197584 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -324,6 +324,9 @@ static void recover_worker(struct work_struct *work)
>  	if (submit) {
>  		struct task_struct *task;
>  
> +		/* Increment the submitqueue fault count */
> +		submit->queue->faults++;
> +
>  		rcu_read_lock();
>  		task = pid_task(submit->pid, PIDTYPE_PID);
>  		if (task) {
> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
> index 5115f75b5b7f..e12f2bd0347d 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
> @@ -120,6 +120,27 @@ int msm_submitqueue_init(struct drm_device *drm, struct msm_file_private *ctx)
>  	return msm_submitqueue_create(drm, ctx, default_prio, 0, NULL);
>  }
>  
> +int msm_submitqueue_query(struct drm_device *drm, struct msm_file_private *ctx,
> +		u32 id,	u32 param, u64 data, u32 len)
> +{
> +	struct msm_gpu_submitqueue *queue = msm_submitqueue_get(ctx, id);
> +	int ret = -EINVAL;
> +
> +	if (!queue)
> +		return -ENOENT;
> +
> +	if (param == MSM_SUBMITQUEUE_PARAM_FAULTS) {
> +		size_t size = min_t(size_t, len, sizeof(queue->faults));
> +
> +		if (copy_to_user(u64_to_user_ptr(data), &queue->faults, size))
> +			ret = -EFAULT;
> +	}

I'll push a new patch to fix it, but pointing out for posterity that we get here
with ret == -EINVAL on success.  There should be ret = 0 before the bracket.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno