diff mbox series

[v2] drm/msm: UAPI error reporting

Message ID 20241230160034.18671-1-robdclark@gmail.com
State New
Headers show
Series [v2] drm/msm: UAPI error reporting | expand

Commit Message

Rob Clark Dec. 30, 2024, 4 p.m. UTC
From: Rob Clark <robdclark@chromium.org>

Debugging incorrect UAPI usage tends to be a bit painful, so add a
helper macro to make it easier to add debug logging which can be enabled
at runtime via drm.debug.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 21 ++++----
 drivers/gpu/drm/msm/msm_drv.c           |  2 +-
 drivers/gpu/drm/msm/msm_drv.h           |  7 +++
 drivers/gpu/drm/msm/msm_gem_submit.c    | 64 +++++++++++--------------
 drivers/gpu/drm/msm/msm_submitqueue.c   |  2 +-
 5 files changed, 47 insertions(+), 49 deletions(-)

Comments

Akhil P Oommen Jan. 2, 2025, 8:07 p.m. UTC | #1
On 12/30/2024 9:30 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Debugging incorrect UAPI usage tends to be a bit painful, so add a
> helper macro to make it easier to add debug logging which can be enabled
> at runtime via drm.debug.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 21 ++++----
>  drivers/gpu/drm/msm/msm_drv.c           |  2 +-
>  drivers/gpu/drm/msm/msm_drv.h           |  7 +++
>  drivers/gpu/drm/msm/msm_gem_submit.c    | 64 +++++++++++--------------
>  drivers/gpu/drm/msm/msm_submitqueue.c   |  2 +-
>  5 files changed, 47 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 076be0473eb5..9649c0bd0a38 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -310,10 +310,11 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  		     uint32_t param, uint64_t *value, uint32_t *len)
>  {
>  	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> +	struct drm_device *drm = gpu->dev;
>  
>  	/* No pointer params yet */
>  	if (*len != 0)
> -		return -EINVAL;
> +		return UERR(EINVAL, drm, "invalid len");
>  
>  	switch (param) {
>  	case MSM_PARAM_GPU_ID:
> @@ -365,12 +366,12 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  		return 0;
>  	case MSM_PARAM_VA_START:
>  		if (ctx->aspace == gpu->aspace)
> -			return -EINVAL;
> +			return UERR(EINVAL, drm, "requires per-process pgtables");
>  		*value = ctx->aspace->va_start;
>  		return 0;
>  	case MSM_PARAM_VA_SIZE:
>  		if (ctx->aspace == gpu->aspace)
> -			return -EINVAL;
> +			return UERR(EINVAL, drm, "requires per-process pgtables");
>  		*value = ctx->aspace->va_size;
>  		return 0;
>  	case MSM_PARAM_HIGHEST_BANK_BIT:
> @@ -386,14 +387,15 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  		*value = adreno_gpu->ubwc_config.macrotile_mode;
>  		return 0;
>  	default:
> -		DBG("%s: invalid param: %u", gpu->name, param);
> -		return -EINVAL;
> +		return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
>  	}
>  }
>  
>  int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  		     uint32_t param, uint64_t value, uint32_t len)
>  {
> +	struct drm_device *drm = gpu->dev;
> +
>  	switch (param) {
>  	case MSM_PARAM_COMM:
>  	case MSM_PARAM_CMDLINE:
> @@ -401,11 +403,11 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  		 * that should be a reasonable upper bound
>  		 */
>  		if (len > PAGE_SIZE)
> -			return -EINVAL;
> +			return UERR(EINVAL, drm, "invalid len");
>  		break;
>  	default:
>  		if (len != 0)
> -			return -EINVAL;
> +			return UERR(EINVAL, drm, "invalid len");
>  	}
>  
>  	switch (param) {
> @@ -434,11 +436,10 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>  	}
>  	case MSM_PARAM_SYSPROF:
>  		if (!capable(CAP_SYS_ADMIN))
> -			return -EPERM;
> +			return UERR(EPERM, drm, "invalid permissions");
>  		return msm_file_private_set_sysprof(ctx, gpu, value);
>  	default:
> -		DBG("%s: invalid param: %u", gpu->name, param);
> -		return -EINVAL;
> +		return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index c2dd8ef6d6dc..2aefb8becda0 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -538,7 +538,7 @@ static int msm_ioctl_gem_info_set_iova(struct drm_device *dev,
>  
>  	/* Only supported if per-process address space is supported: */
>  	if (priv->gpu->aspace == ctx->aspace)
> -		return -EOPNOTSUPP;
> +		return UERR(EOPNOTSUPP, dev, "requires per-process pgtables");
>  
>  	if (should_fail(&fail_gem_iova, obj->size))
>  		return -ENOMEM;
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index d8c9a1b19263..fee31680a6d5 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -28,6 +28,7 @@
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/display/drm_dsc.h>
>  #include <drm/msm_drm.h>
> @@ -506,6 +507,12 @@ void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
>  			   clockid_t clock_id,
>  			   enum hrtimer_mode mode);
>  
> +/* Helper for returning a UABI error with optional logging which can make
> + * it easier for userspace to understand what it is doing wrong.
> + */
> +#define UERR(err, drm, fmt, ...) \
> +	({ DRM_DEV_DEBUG_DRIVER((drm)->dev, fmt, ##__VA_ARGS__); -(err); })
> +
>  #define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
>  #define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
>  
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index fba78193127d..be6e793f34bd 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -20,8 +20,8 @@
>  /* For userspace errors, use DRM_UT_DRIVER.. so that userspace can enable
>   * error msgs for debugging, but we don't spam dmesg by default
>   */
> -#define SUBMIT_ERROR(submit, fmt, ...) \
> -	DRM_DEV_DEBUG_DRIVER((submit)->dev->dev, fmt, ##__VA_ARGS__)
> +#define SUBMIT_ERROR(err, submit, fmt, ...) \
> +	UERR(err, (submit)->dev, fmt, ##__VA_ARGS__)
>  
>  /*
>   * Cmdstream submission:
> @@ -142,8 +142,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
>  
>  		if ((submit_bo.flags & ~MSM_SUBMIT_BO_FLAGS) ||
>  			!(submit_bo.flags & MANDATORY_FLAGS)) {
> -			SUBMIT_ERROR(submit, "invalid flags: %x\n", submit_bo.flags);
> -			ret = -EINVAL;
> +			ret = SUBMIT_ERROR(EINVAL, submit, "invalid flags: %x\n", submit_bo.flags);
>  			i = 0;
>  			goto out;
>  		}
> @@ -162,8 +161,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
>  		 */
>  		obj = idr_find(&file->object_idr, submit->bos[i].handle);
>  		if (!obj) {
> -			SUBMIT_ERROR(submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
> -			ret = -EINVAL;
> +			ret = SUBMIT_ERROR(EINVAL, submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
>  			goto out_unlock;
>  		}
>  
> @@ -206,14 +204,12 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
>  		case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
>  			break;
>  		default:
> -			SUBMIT_ERROR(submit, "invalid type: %08x\n", submit_cmd.type);
> -			return -EINVAL;
> +			return SUBMIT_ERROR(EINVAL, submit, "invalid type: %08x\n", submit_cmd.type);
>  		}
>  
>  		if (submit_cmd.size % 4) {
> -			SUBMIT_ERROR(submit, "non-aligned cmdstream buffer size: %u\n",
> -				     submit_cmd.size);
> -			ret = -EINVAL;
> +			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer size: %u\n",
> +					   submit_cmd.size);
>  			goto out;
>  		}
>  
> @@ -371,9 +367,8 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
>  		struct drm_gem_object **obj, uint64_t *iova)
>  {
>  	if (idx >= submit->nr_bos) {
> -		SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
> -			     idx, submit->nr_bos);
> -		return -EINVAL;
> +		return SUBMIT_ERROR(EINVAL, submit, "invalid buffer index: %u (out of %u)\n",
> +				    idx, submit->nr_bos);
>  	}
>  
>  	if (obj)
> @@ -392,10 +387,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
>  	uint32_t *ptr;
>  	int ret = 0;
>  
> -	if (offset % 4) {
> -		SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n", offset);
> -		return -EINVAL;
> -	}
> +	if (offset % 4)
> +		return SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer: %u\n", offset);
>  
>  	/* For now, just map the entire thing.  Eventually we probably
>  	 * to do it page-by-page, w/ kmap() if not vmap()d..
> @@ -414,9 +407,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
>  		uint64_t iova;
>  
>  		if (submit_reloc.submit_offset % 4) {
> -			SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
> -				     submit_reloc.submit_offset);
> -			ret = -EINVAL;
> +			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned reloc offset: %u\n",
> +					   submit_reloc.submit_offset);
>  			goto out;
>  		}
>  
> @@ -425,8 +417,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
>  
>  		if ((off >= (obj->size / 4)) ||
>  				(off < last_offset)) {
> -			SUBMIT_ERROR(submit, "invalid offset %u at reloc %u\n", off, i);
> -			ret = -EINVAL;
> +			ret = SUBMIT_ERROR(EINVAL, submit, "invalid offset %u at reloc %u\n", off, i);
>  			goto out;
>  		}
>  
> @@ -513,12 +504,12 @@ static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
>  
>  		if (syncobj_desc.point &&
>  		    !drm_core_check_feature(submit->dev, DRIVER_SYNCOBJ_TIMELINE)) {
> -			ret = -EOPNOTSUPP;
> +			ret = SUBMIT_ERROR(EOPNOTSUPP, submit, "syncobj timeline unsupported");
>  			break;
>  		}
>  
>  		if (syncobj_desc.flags & ~MSM_SUBMIT_SYNCOBJ_FLAGS) {
> -			ret = -EINVAL;
> +			ret = -SUBMIT_ERROR(EINVAL, submit, "invalid syncobj flags");
>  			break;
>  		}
>  
> @@ -531,7 +522,7 @@ static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
>  			syncobjs[i] =
>  				drm_syncobj_find(file, syncobj_desc.handle);
>  			if (!syncobjs[i]) {
> -				ret = -EINVAL;
> +				ret = SUBMIT_ERROR(EINVAL, submit, "invalid syncobj handle");

Just to be more useful, probably we can print the index or the handle
here. Anyway

Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com>


-Akhil.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 076be0473eb5..9649c0bd0a38 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -310,10 +310,11 @@  int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 		     uint32_t param, uint64_t *value, uint32_t *len)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+	struct drm_device *drm = gpu->dev;
 
 	/* No pointer params yet */
 	if (*len != 0)
-		return -EINVAL;
+		return UERR(EINVAL, drm, "invalid len");
 
 	switch (param) {
 	case MSM_PARAM_GPU_ID:
@@ -365,12 +366,12 @@  int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 		return 0;
 	case MSM_PARAM_VA_START:
 		if (ctx->aspace == gpu->aspace)
-			return -EINVAL;
+			return UERR(EINVAL, drm, "requires per-process pgtables");
 		*value = ctx->aspace->va_start;
 		return 0;
 	case MSM_PARAM_VA_SIZE:
 		if (ctx->aspace == gpu->aspace)
-			return -EINVAL;
+			return UERR(EINVAL, drm, "requires per-process pgtables");
 		*value = ctx->aspace->va_size;
 		return 0;
 	case MSM_PARAM_HIGHEST_BANK_BIT:
@@ -386,14 +387,15 @@  int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 		*value = adreno_gpu->ubwc_config.macrotile_mode;
 		return 0;
 	default:
-		DBG("%s: invalid param: %u", gpu->name, param);
-		return -EINVAL;
+		return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
 	}
 }
 
 int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 		     uint32_t param, uint64_t value, uint32_t len)
 {
+	struct drm_device *drm = gpu->dev;
+
 	switch (param) {
 	case MSM_PARAM_COMM:
 	case MSM_PARAM_CMDLINE:
@@ -401,11 +403,11 @@  int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 		 * that should be a reasonable upper bound
 		 */
 		if (len > PAGE_SIZE)
-			return -EINVAL;
+			return UERR(EINVAL, drm, "invalid len");
 		break;
 	default:
 		if (len != 0)
-			return -EINVAL;
+			return UERR(EINVAL, drm, "invalid len");
 	}
 
 	switch (param) {
@@ -434,11 +436,10 @@  int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 	}
 	case MSM_PARAM_SYSPROF:
 		if (!capable(CAP_SYS_ADMIN))
-			return -EPERM;
+			return UERR(EPERM, drm, "invalid permissions");
 		return msm_file_private_set_sysprof(ctx, gpu, value);
 	default:
-		DBG("%s: invalid param: %u", gpu->name, param);
-		return -EINVAL;
+		return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
 	}
 }
 
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index c2dd8ef6d6dc..2aefb8becda0 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -538,7 +538,7 @@  static int msm_ioctl_gem_info_set_iova(struct drm_device *dev,
 
 	/* Only supported if per-process address space is supported: */
 	if (priv->gpu->aspace == ctx->aspace)
-		return -EOPNOTSUPP;
+		return UERR(EOPNOTSUPP, dev, "requires per-process pgtables");
 
 	if (should_fail(&fail_gem_iova, obj->size))
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index d8c9a1b19263..fee31680a6d5 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -28,6 +28,7 @@ 
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/display/drm_dsc.h>
 #include <drm/msm_drm.h>
@@ -506,6 +507,12 @@  void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
 			   clockid_t clock_id,
 			   enum hrtimer_mode mode);
 
+/* Helper for returning a UABI error with optional logging which can make
+ * it easier for userspace to understand what it is doing wrong.
+ */
+#define UERR(err, drm, fmt, ...) \
+	({ DRM_DEV_DEBUG_DRIVER((drm)->dev, fmt, ##__VA_ARGS__); -(err); })
+
 #define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
 
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index fba78193127d..be6e793f34bd 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -20,8 +20,8 @@ 
 /* For userspace errors, use DRM_UT_DRIVER.. so that userspace can enable
  * error msgs for debugging, but we don't spam dmesg by default
  */
-#define SUBMIT_ERROR(submit, fmt, ...) \
-	DRM_DEV_DEBUG_DRIVER((submit)->dev->dev, fmt, ##__VA_ARGS__)
+#define SUBMIT_ERROR(err, submit, fmt, ...) \
+	UERR(err, (submit)->dev, fmt, ##__VA_ARGS__)
 
 /*
  * Cmdstream submission:
@@ -142,8 +142,7 @@  static int submit_lookup_objects(struct msm_gem_submit *submit,
 
 		if ((submit_bo.flags & ~MSM_SUBMIT_BO_FLAGS) ||
 			!(submit_bo.flags & MANDATORY_FLAGS)) {
-			SUBMIT_ERROR(submit, "invalid flags: %x\n", submit_bo.flags);
-			ret = -EINVAL;
+			ret = SUBMIT_ERROR(EINVAL, submit, "invalid flags: %x\n", submit_bo.flags);
 			i = 0;
 			goto out;
 		}
@@ -162,8 +161,7 @@  static int submit_lookup_objects(struct msm_gem_submit *submit,
 		 */
 		obj = idr_find(&file->object_idr, submit->bos[i].handle);
 		if (!obj) {
-			SUBMIT_ERROR(submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
-			ret = -EINVAL;
+			ret = SUBMIT_ERROR(EINVAL, submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
 			goto out_unlock;
 		}
 
@@ -206,14 +204,12 @@  static int submit_lookup_cmds(struct msm_gem_submit *submit,
 		case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
 			break;
 		default:
-			SUBMIT_ERROR(submit, "invalid type: %08x\n", submit_cmd.type);
-			return -EINVAL;
+			return SUBMIT_ERROR(EINVAL, submit, "invalid type: %08x\n", submit_cmd.type);
 		}
 
 		if (submit_cmd.size % 4) {
-			SUBMIT_ERROR(submit, "non-aligned cmdstream buffer size: %u\n",
-				     submit_cmd.size);
-			ret = -EINVAL;
+			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer size: %u\n",
+					   submit_cmd.size);
 			goto out;
 		}
 
@@ -371,9 +367,8 @@  static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
 		struct drm_gem_object **obj, uint64_t *iova)
 {
 	if (idx >= submit->nr_bos) {
-		SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
-			     idx, submit->nr_bos);
-		return -EINVAL;
+		return SUBMIT_ERROR(EINVAL, submit, "invalid buffer index: %u (out of %u)\n",
+				    idx, submit->nr_bos);
 	}
 
 	if (obj)
@@ -392,10 +387,8 @@  static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
 	uint32_t *ptr;
 	int ret = 0;
 
-	if (offset % 4) {
-		SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n", offset);
-		return -EINVAL;
-	}
+	if (offset % 4)
+		return SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer: %u\n", offset);
 
 	/* For now, just map the entire thing.  Eventually we probably
 	 * to do it page-by-page, w/ kmap() if not vmap()d..
@@ -414,9 +407,8 @@  static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
 		uint64_t iova;
 
 		if (submit_reloc.submit_offset % 4) {
-			SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
-				     submit_reloc.submit_offset);
-			ret = -EINVAL;
+			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned reloc offset: %u\n",
+					   submit_reloc.submit_offset);
 			goto out;
 		}
 
@@ -425,8 +417,7 @@  static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
 
 		if ((off >= (obj->size / 4)) ||
 				(off < last_offset)) {
-			SUBMIT_ERROR(submit, "invalid offset %u at reloc %u\n", off, i);
-			ret = -EINVAL;
+			ret = SUBMIT_ERROR(EINVAL, submit, "invalid offset %u at reloc %u\n", off, i);
 			goto out;
 		}
 
@@ -513,12 +504,12 @@  static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
 
 		if (syncobj_desc.point &&
 		    !drm_core_check_feature(submit->dev, DRIVER_SYNCOBJ_TIMELINE)) {
-			ret = -EOPNOTSUPP;
+			ret = SUBMIT_ERROR(EOPNOTSUPP, submit, "syncobj timeline unsupported");
 			break;
 		}
 
 		if (syncobj_desc.flags & ~MSM_SUBMIT_SYNCOBJ_FLAGS) {
-			ret = -EINVAL;
+			ret = -SUBMIT_ERROR(EINVAL, submit, "invalid syncobj flags");
 			break;
 		}
 
@@ -531,7 +522,7 @@  static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
 			syncobjs[i] =
 				drm_syncobj_find(file, syncobj_desc.handle);
 			if (!syncobjs[i]) {
-				ret = -EINVAL;
+				ret = SUBMIT_ERROR(EINVAL, submit, "invalid syncobj handle");
 				break;
 			}
 		}
@@ -588,14 +579,14 @@  static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev,
 		post_deps[i].point = syncobj_desc.point;
 
 		if (syncobj_desc.flags) {
-			ret = -EINVAL;
+			ret = UERR(EINVAL, dev, "invalid syncobj flags");
 			break;
 		}
 
 		if (syncobj_desc.point) {
 			if (!drm_core_check_feature(dev,
 			                            DRIVER_SYNCOBJ_TIMELINE)) {
-				ret = -EOPNOTSUPP;
+				ret = UERR(EOPNOTSUPP, dev, "syncobj timeline unsupported");
 				break;
 			}
 
@@ -609,7 +600,7 @@  static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev,
 		post_deps[i].syncobj =
 			drm_syncobj_find(file, syncobj_desc.handle);
 		if (!post_deps[i].syncobj) {
-			ret = -EINVAL;
+			ret = UERR(EINVAL, dev, "invalid syncobj handle");
 			break;
 		}
 	}
@@ -677,10 +668,10 @@  int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	 * be more clever to dispatch to appropriate gpu module:
 	 */
 	if (MSM_PIPE_ID(args->flags) != MSM_PIPE_3D0)
-		return -EINVAL;
+		return UERR(EINVAL, dev, "invalid pipe");
 
 	if (MSM_PIPE_FLAGS(args->flags) & ~MSM_SUBMIT_FLAGS)
-		return -EINVAL;
+		return UERR(EINVAL, dev, "invalid flags");
 
 	if (args->flags & MSM_SUBMIT_SUDO) {
 		if (!IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) ||
@@ -724,7 +715,7 @@  int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 		in_fence = sync_file_get_fence(args->fence_fd);
 
 		if (!in_fence) {
-			ret = -EINVAL;
+			ret = UERR(EINVAL, dev, "invalid in-fence");
 			goto out_unlock;
 		}
 
@@ -789,8 +780,8 @@  int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 		if (!submit->cmd[i].size ||
 			((submit->cmd[i].size + submit->cmd[i].offset) >
 				obj->size / 4)) {
-			SUBMIT_ERROR(submit, "invalid cmdstream size: %u\n", submit->cmd[i].size * 4);
-			ret = -EINVAL;
+			ret = UERR(EINVAL, dev, "invalid cmdstream size: %u\n",
+				   submit->cmd[i].size * 4);
 			goto out;
 		}
 
@@ -800,8 +791,7 @@  int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 			continue;
 
 		if (!gpu->allow_relocs) {
-			SUBMIT_ERROR(submit, "relocs not allowed\n");
-			ret = -EINVAL;
+			ret = UERR(EINVAL, dev, "relocs not allowed\n");
 			goto out;
 		}
 
@@ -827,7 +817,7 @@  int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 			(!args->fence || idr_find(&queue->fence_idr, args->fence))) {
 		spin_unlock(&queue->idr_lock);
 		idr_preload_end();
-		ret = -EINVAL;
+		ret = UERR(EINVAL, dev, "invalid in-fence-sn");
 		goto out;
 	}
 
diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
index 2fc3eaf81f44..7fed1de63b5d 100644
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -18,7 +18,7 @@  int msm_file_private_set_sysprof(struct msm_file_private *ctx,
 
 	switch (sysprof) {
 	default:
-		return -EINVAL;
+		return UERR(EINVAL, gpu->dev, "Invalid sysprof: %d", sysprof);
 	case 2:
 		pm_runtime_get_sync(&gpu->pdev->dev);
 		fallthrough;