Re: [Mesa-dev] [PATCH 46/53] i965/drm: Fold drm_bacon_gem_reset_stats into the callers.

[Chris Wilson]

On Tue, Apr 04, 2017 at 05:10:36PM -0700, Kenneth Graunke wrote:

Even a small justification?

I would have kept the bare ioctl wrapper

static int get_reset_stats(int fd, struct *stats)
{
int err;

err = 0;
if (drmIoctl(fd, DRM_IOCTL_I915_GET_RESET_STATS, stats))
err = -errno;

return err; /* reporting the actual error may be
   overkill, just habit! */
}


Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 46/53] i965/drm: Fold drm_bacon_gem_reset_stats into the callers.

[Kenneth Graunke]

---
 src/mesa/drivers/dri/i965/brw_bufmgr.h   |  5 
 src/mesa/drivers/dri/i965/brw_reset.c| 36 +---
 src/mesa/drivers/dri/i965/intel_bufmgr_gem.c | 32 -
 3 files changed, 17 insertions(+), 56 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h 
b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index 87e67602bd9..026cc00c792 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -317,11 +317,6 @@ int drm_bacon_reg_read(drm_bacon_bufmgr *bufmgr,
   uint32_t offset,
   uint64_t *result);
 
-int drm_bacon_get_reset_stats(drm_bacon_context *ctx,
- uint32_t *reset_count,
- uint32_t *active,
- uint32_t *pending);
-
 /** @{ */
 
 #if defined(__cplusplus)
diff --git a/src/mesa/drivers/dri/i965/brw_reset.c 
b/src/mesa/drivers/dri/i965/brw_reset.c
index b91ffd723b9..aa68102d2e3 100644
--- a/src/mesa/drivers/dri/i965/brw_reset.c
+++ b/src/mesa/drivers/dri/i965/brw_reset.c
@@ -23,6 +23,7 @@
 
 #include "main/context.h"
 
+#include 
 #include "brw_context.h"
 
 /**
@@ -34,10 +35,8 @@ GLenum
 brw_get_graphics_reset_status(struct gl_context *ctx)
 {
struct brw_context *brw = brw_context(ctx);
-   int err;
-   uint32_t reset_count;
-   uint32_t active;
-   uint32_t pending;
+   __DRIscreen *dri_screen = brw->screen->driScrnPriv;
+   struct drm_i915_reset_stats stats;
 
/* If hardware contexts are not being used (or
 * DRM_IOCTL_I915_GET_RESET_STATS is not supported), this function should
@@ -45,6 +44,9 @@ brw_get_graphics_reset_status(struct gl_context *ctx)
 */
assert(brw->hw_ctx != NULL);
 
+   memset(, 0, sizeof(stats));
+   drm_bacon_gem_context_get_id(brw->hw_ctx, _id);
+
/* A reset status other than NO_ERROR was returned last time. I915 returns
 * nonzero active/pending only if reset has been encountered and completed.
 * Return NO_ERROR from now on.
@@ -52,16 +54,14 @@ brw_get_graphics_reset_status(struct gl_context *ctx)
if (brw->reset_count != 0)
   return GL_NO_ERROR;
 
-   err = drm_bacon_get_reset_stats(brw->hw_ctx, _count, ,
-   );
-   if (err)
+   if (drmIoctl(dri_screen->fd, DRM_IOCTL_I915_GET_RESET_STATS, ) != 0)
   return GL_NO_ERROR;
 
/* A reset was observed while a batch from this context was executing.
 * Assume that this context was at fault.
 */
-   if (active != 0) {
-  brw->reset_count = reset_count;
+   if (stats.batch_active != 0) {
+  brw->reset_count = stats.reset_count;
   return GL_GUILTY_CONTEXT_RESET_ARB;
}
 
@@ -69,8 +69,8 @@ brw_get_graphics_reset_status(struct gl_context *ctx)
 * but the batch was not executing.  In this case, assume that the context
 * was not at fault.
 */
-   if (pending != 0) {
-  brw->reset_count = reset_count;
+   if (stats.batch_pending != 0) {
+  brw->reset_count = stats.reset_count;
   return GL_INNOCENT_CONTEXT_RESET_ARB;
}
 
@@ -80,16 +80,14 @@ brw_get_graphics_reset_status(struct gl_context *ctx)
 void
 brw_check_for_reset(struct brw_context *brw)
 {
-   uint32_t reset_count;
-   uint32_t active;
-   uint32_t pending;
-   int err;
+   __DRIscreen *dri_screen = brw->screen->driScrnPriv;
+   struct drm_i915_reset_stats stats;
+   memset(, 0, sizeof(stats));
+   drm_bacon_gem_context_get_id(brw->hw_ctx, _id);
 
-   err = drm_bacon_get_reset_stats(brw->hw_ctx, _count, ,
-   );
-   if (err)
+   if (drmIoctl(dri_screen->fd, DRM_IOCTL_I915_GET_RESET_STATS, ) != 0)
   return;
 
-   if (active > 0 || pending > 0)
+   if (stats.batch_active > 0 || stats.batch_pending > 0)
   _mesa_set_context_lost_dispatch(>ctx);
 }
diff --git a/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c 
b/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
index 75b1344286a..317265b65cf 100644
--- a/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
+++ b/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
@@ -1478,38 +1478,6 @@ drm_bacon_gem_context_destroy(drm_bacon_context *ctx)
 }
 
 int
-drm_bacon_get_reset_stats(drm_bacon_context *ctx,
- uint32_t *reset_count,
- uint32_t *active,
- uint32_t *pending)
-{
-   struct drm_i915_reset_stats stats;
-   int ret;
-
-   if (ctx == NULL)
-   return -EINVAL;
-
-   memclear(stats);
-
-   stats.ctx_id = ctx->ctx_id;
-   ret = drmIoctl(ctx->bufmgr->fd,
-  DRM_IOCTL_I915_GET_RESET_STATS,
-  );
-   if (ret == 0) {
-   if (reset_count != NULL)
-   *reset_count = stats.reset_count;
-
-   if (active != NULL)
-   *active = stats.batch_active;
-
-   if (pending != NULL)
-   *pending = stats.batch_pending;
-   }
-
-   return ret;