diff mbox series

[i-g-t,12/24] lib/i915: Improve gem_context error messages

Message ID 20190322092155.1656-12-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [i-g-t,01/24] i915/gem_exec_latency: Measure the latency of context switching | expand

Commit Message

Chris Wilson March 22, 2019, 9:21 a.m. UTC 
Avoid embedding the DRM_IOCTL() macro into the error message as it is
unreadable, and instead always wrap the ioctl with a self-descriptive
helper.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_context.c | 72 ++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 38 deletions(-)

Comments

Tvrtko Ursulin March 26, 2019, 10:14 a.m. UTC | #1 
On 22/03/2019 09:21, Chris Wilson wrote:
> Avoid embedding the DRM_IOCTL() macro into the error message as it is
> unreadable, and instead always wrap the ioctl with a self-descriptive
> helper.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   lib/i915/gem_context.c | 72 ++++++++++++++++++++----------------------
>   1 file changed, 34 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
> index 16004685e..8b4d5b704 100644
> --- a/lib/i915/gem_context.c
> +++ b/lib/i915/gem_context.c
> @@ -113,16 +113,16 @@ uint32_t gem_context_create(int fd)
>   
>   int __gem_context_destroy(int fd, uint32_t ctx_id)
>   {
> -	struct drm_i915_gem_context_destroy destroy;
> -	int ret;
> +	struct drm_i915_gem_context_destroy destroy = { ctx_id };
> +	int err = 0;
>   
> -	memset(&destroy, 0, sizeof(destroy));
> -	destroy.ctx_id = ctx_id;
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
>   
> -	ret = igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy);
> -	if (ret)
> -		return -errno;
> -	return 0;
> +	errno = 0;
> +	return err;
>   }
>   
>   /**
> @@ -134,21 +134,20 @@ int __gem_context_destroy(int fd, uint32_t ctx_id)
>    */
>   void gem_context_destroy(int fd, uint32_t ctx_id)
>   {
> -	struct drm_i915_gem_context_destroy destroy;
> -
> -	memset(&destroy, 0, sizeof(destroy));
> -	destroy.ctx_id = ctx_id;
> -
> -	do_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy);
> +	igt_assert_eq(__gem_context_destroy(fd, ctx_id), 0);
>   }
>   
>   int __gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
>   {
> -	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, p))
> -		return -errno;
> +	int err = 0;
> +
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, p)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
>   
>   	errno = 0;
> -	return 0;
> +	return err;
>   }
>   
>   /**
> @@ -161,18 +160,22 @@ int __gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
>    */
>   void gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
>   {
> -	igt_assert(__gem_context_get_param(fd, p) == 0);
> +	igt_assert_eq(__gem_context_get_param(fd, p), 0);
>   }
>   
> -
>   int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
>   {
> -	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, p))
> -		return -errno;
> +	int err = 0;
> +
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, p)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
>   
>   	errno = 0;
> -	return 0;
> +	return err;
>   }
> +
>   /**
>    * gem_context_set_param:
>    * @fd: open i915 drm file descriptor
> @@ -183,7 +186,7 @@ int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
>    */
>   void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
>   {
> -	igt_assert(__gem_context_set_param(fd, p) == 0);
> +	igt_assert_eq(__gem_context_set_param(fd, p), 0);
>   }
>   
>   /**
> @@ -196,14 +199,9 @@ void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
>    */
>   void gem_context_require_param(int fd, uint64_t param)
>   {
> -	struct drm_i915_gem_context_param p;
> -
> -	p.ctx_id = 0;
> -	p.param = param;
> -	p.value = 0;
> -	p.size = 0;
> +	struct drm_i915_gem_context_param p = { .param = param };
>   
> -	igt_require(igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &p) == 0);
> +	igt_require(__gem_context_get_param(fd, &p) == 0);
>   }
>   
>   void gem_context_require_bannable(int fd)
> @@ -252,13 +250,11 @@ void gem_context_require_bannable(int fd)
>    */
>   int __gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
>   {
> -	struct drm_i915_gem_context_param p;
> -
> -	memset(&p, 0, sizeof(p));
> -	p.ctx_id = ctx_id;
> -	p.size = 0;
> -	p.param = DRM_I915_CONTEXT_PARAM_PRIORITY;
> -	p.value = prio;
> +	struct drm_i915_gem_context_param p = {
> +		.ctx_id = ctx_id,
> +		.param = DRM_I915_CONTEXT_PARAM_PRIORITY,
> +		.value = prio,
> +	};
>   
>   	return __gem_context_set_param(fd, &p);
>   }
> @@ -273,5 +269,5 @@ int __gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
>    */
>   void gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
>   {
> -	igt_assert(__gem_context_set_priority(fd, ctx_id, prio) == 0);
> +	igt_assert_eq(__gem_context_set_priority(fd, ctx_id, prio), 0);
>   }
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 16004685e..8b4d5b704 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -113,16 +113,16 @@  uint32_t gem_context_create(int fd)
 
 int __gem_context_destroy(int fd, uint32_t ctx_id)
 {
-	struct drm_i915_gem_context_destroy destroy;
-	int ret;
+	struct drm_i915_gem_context_destroy destroy = { ctx_id };
+	int err = 0;
 
-	memset(&destroy, 0, sizeof(destroy));
-	destroy.ctx_id = ctx_id;
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy)) {
+		err = -errno;
+		igt_assume(err);
+	}
 
-	ret = igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy);
-	if (ret)
-		return -errno;
-	return 0;
+	errno = 0;
+	return err;
 }
 
 /**
@@ -134,21 +134,20 @@  int __gem_context_destroy(int fd, uint32_t ctx_id)
  */
 void gem_context_destroy(int fd, uint32_t ctx_id)
 {
-	struct drm_i915_gem_context_destroy destroy;
-
-	memset(&destroy, 0, sizeof(destroy));
-	destroy.ctx_id = ctx_id;
-
-	do_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy);
+	igt_assert_eq(__gem_context_destroy(fd, ctx_id), 0);
 }
 
 int __gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
 {
-	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, p))
-		return -errno;
+	int err = 0;
+
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, p)) {
+		err = -errno;
+		igt_assume(err);
+	}
 
 	errno = 0;
-	return 0;
+	return err;
 }
 
 /**
@@ -161,18 +160,22 @@  int __gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
  */
 void gem_context_get_param(int fd, struct drm_i915_gem_context_param *p)
 {
-	igt_assert(__gem_context_get_param(fd, p) == 0);
+	igt_assert_eq(__gem_context_get_param(fd, p), 0);
 }
 
-
 int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
 {
-	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, p))
-		return -errno;
+	int err = 0;
+
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, p)) {
+		err = -errno;
+		igt_assume(err);
+	}
 
 	errno = 0;
-	return 0;
+	return err;
 }
+
 /**
  * gem_context_set_param:
  * @fd: open i915 drm file descriptor
@@ -183,7 +186,7 @@  int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
  */
 void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
 {
-	igt_assert(__gem_context_set_param(fd, p) == 0);
+	igt_assert_eq(__gem_context_set_param(fd, p), 0);
 }
 
 /**
@@ -196,14 +199,9 @@  void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p)
  */
 void gem_context_require_param(int fd, uint64_t param)
 {
-	struct drm_i915_gem_context_param p;
-
-	p.ctx_id = 0;
-	p.param = param;
-	p.value = 0;
-	p.size = 0;
+	struct drm_i915_gem_context_param p = { .param = param };
 
-	igt_require(igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &p) == 0);
+	igt_require(__gem_context_get_param(fd, &p) == 0);
 }
 
 void gem_context_require_bannable(int fd)
@@ -252,13 +250,11 @@  void gem_context_require_bannable(int fd)
  */
 int __gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
 {
-	struct drm_i915_gem_context_param p;
-
-	memset(&p, 0, sizeof(p));
-	p.ctx_id = ctx_id;
-	p.size = 0;
-	p.param = DRM_I915_CONTEXT_PARAM_PRIORITY;
-	p.value = prio;
+	struct drm_i915_gem_context_param p = {
+		.ctx_id = ctx_id,
+		.param = DRM_I915_CONTEXT_PARAM_PRIORITY,
+		.value = prio,
+	};
 
 	return __gem_context_set_param(fd, &p);
 }
@@ -273,5 +269,5 @@  int __gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
  */
 void gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
 {
-	igt_assert(__gem_context_set_priority(fd, ctx_id, prio) == 0);
+	igt_assert_eq(__gem_context_set_priority(fd, ctx_id, prio), 0);
 }