All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
@ 2014-07-17 16:39 Rodrigo Vivi
  2014-07-18  0:23 ` Ben Widawsky
  0 siblings, 1 reply; 6+ messages in thread
From: Rodrigo Vivi @ 2014-07-17 16:39 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Rodrigo Vivi

With the increasing number of rings,
we probably have more information to print than we were printing.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 36a7960..0beeebf 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -242,6 +242,8 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 				  struct drm_device *dev,
 				  struct drm_i915_error_ring *ring)
 {
+	int i;
+
 	if (!ring->valid)
 		return;
 
@@ -264,23 +266,15 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 	if (INTEL_INFO(dev)->gen >= 6) {
 		err_printf(m, "  RC PSMI: 0x%08x\n", ring->rc_psmi);
 		err_printf(m, "  FAULT_REG: 0x%08x\n", ring->fault_reg);
-		err_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
-			   ring->semaphore_mboxes[0],
-			   ring->semaphore_seqno[0]);
-		err_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
-			   ring->semaphore_mboxes[1],
-			   ring->semaphore_seqno[1]);
-		if (HAS_VEBOX(dev)) {
-			err_printf(m, "  SYNC_2: 0x%08x [last synced 0x%08x]\n",
-				   ring->semaphore_mboxes[2],
-				   ring->semaphore_seqno[2]);
+		for (i = 0; i < I915_NUM_RINGS - 1; i++) {
+			err_printf(m, "  SYNC_%d: 0x%08x [last synced 0x%08x]\n",
+				   i, ring->semaphore_mboxes[i],
+				   ring->semaphore_seqno[i]);
 		}
 	}
 	if (USES_PPGTT(dev)) {
 		err_printf(m, "  GFX_MODE: 0x%08x\n", ring->vm_info.gfx_mode);
-
 		if (INTEL_INFO(dev)->gen >= 8) {
-			int i;
 			for (i = 0; i < 4; i++)
 				err_printf(m, "  PDP%d: 0x%016llx\n",
 					   i, ring->vm_info.pdp[i]);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
  2014-07-18  0:42     ` Ben Widawsky
@ 2014-07-17 17:58       ` Rodrigo Vivi
  2014-07-18  2:42         ` Ben Widawsky
  0 siblings, 1 reply; 6+ messages in thread
From: Rodrigo Vivi @ 2014-07-17 17:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Rodrigo Vivi

With the increasing number of rings,
we probably have more information to print than we were printing.

v2: Loop only over active rings and print info with ring names.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 36a7960..b1848e0 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -242,6 +242,10 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 				  struct drm_device *dev,
 				  struct drm_i915_error_ring *ring)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_engine_cs *from, *to;
+	int i, j;
+
 	if (!ring->valid)
 		return;
 
@@ -264,23 +268,19 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 	if (INTEL_INFO(dev)->gen >= 6) {
 		err_printf(m, "  RC PSMI: 0x%08x\n", ring->rc_psmi);
 		err_printf(m, "  FAULT_REG: 0x%08x\n", ring->fault_reg);
-		err_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
-			   ring->semaphore_mboxes[0],
-			   ring->semaphore_seqno[0]);
-		err_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
-			   ring->semaphore_mboxes[1],
-			   ring->semaphore_seqno[1]);
-		if (HAS_VEBOX(dev)) {
-			err_printf(m, "  SYNC_2: 0x%08x [last synced 0x%08x]\n",
-				   ring->semaphore_mboxes[2],
-				   ring->semaphore_seqno[2]);
+		for_each_ring(from, dev_priv, i) {
+			for_each_ring(to, dev_priv, j) {
+				int idx = intel_ring_sync_index(from, to);
+				err_printf(m, "  SYNC[%s -> %s]: 0x%08x [last synced 0x%08x]\n",
+					   from->name, to->name,
+					   ring->semaphore_mboxes[idx],
+					   ring->semaphore_seqno[idx]);
+			}
 		}
 	}
 	if (USES_PPGTT(dev)) {
 		err_printf(m, "  GFX_MODE: 0x%08x\n", ring->vm_info.gfx_mode);
-
 		if (INTEL_INFO(dev)->gen >= 8) {
-			int i;
 			for (i = 0; i < 4; i++)
 				err_printf(m, "  PDP%d: 0x%016llx\n",
 					   i, ring->vm_info.pdp[i]);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
  2014-07-17 16:39 [PATCH] drm/i915: print full error ring semaphore mboxes and sync Rodrigo Vivi
@ 2014-07-18  0:23 ` Ben Widawsky
  2014-07-18  0:36   ` Vivi, Rodrigo
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Widawsky @ 2014-07-18  0:23 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Thu, Jul 17, 2014 at 09:39:55AM -0700, Rodrigo Vivi wrote:
> With the increasing number of rings,
> we probably have more information to print than we were printing.

After our discussion were you going to send a new patch?

[snip]


-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
  2014-07-18  0:23 ` Ben Widawsky
@ 2014-07-18  0:36   ` Vivi, Rodrigo
  2014-07-18  0:42     ` Ben Widawsky
  0 siblings, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2014-07-18  0:36 UTC (permalink / raw)
  To: Widawsky, Benjamin; +Cc: intel-gfx

You mean you prefer the the loop with  for (i = 0; i < hweight(ring_mask); i++) {
instead? I thought you were ok with either and I preferred this one just to be on the safest side and let userspace parse it properly.

Or do you prefer that other version with double loop but with names of rings?

Thanks,
Rodrigo.

-----Original Message-----
From: Widawsky, Benjamin 
Sent: Thursday, July 17, 2014 5:23 PM
To: Vivi, Rodrigo
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.

On Thu, Jul 17, 2014 at 09:39:55AM -0700, Rodrigo Vivi wrote:
> With the increasing number of rings,
> we probably have more information to print than we were printing.

After our discussion were you going to send a new patch?

[snip]


-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
  2014-07-18  0:36   ` Vivi, Rodrigo
@ 2014-07-18  0:42     ` Ben Widawsky
  2014-07-17 17:58       ` Rodrigo Vivi
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Widawsky @ 2014-07-18  0:42 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx

On Thu, Jul 17, 2014 at 05:36:52PM -0700, Vivi, Rodrigo wrote:
> You mean you prefer the the loop with  for (i = 0; i < hweight(ring_mask); i++) {
> instead? I thought you were ok with either and I preferred this one just to be on the safest side and let userspace parse it properly.

I can live with either. I guess it's a little more obvious if we only
capture the relevant data on platforms that have actual registers. gen8
is special IMO here because we're writing to memory. But either way this
is an improvement.

> 
> Or do you prefer that other version with double loop but with names of rings?

I think it would be cool if we could have it as such, but I am not sure
what others think:
SYNC[RCS->VCS]: 0x%08x

But whatever.


Anyway, I was just asking because I wasn't sure if I should wait for
another patch.

> 
> Thanks,
> Rodrigo.
> 
> -----Original Message-----
> From: Widawsky, Benjamin 
> Sent: Thursday, July 17, 2014 5:23 PM
> To: Vivi, Rodrigo
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
> 
> On Thu, Jul 17, 2014 at 09:39:55AM -0700, Rodrigo Vivi wrote:
> > With the increasing number of rings,
> > we probably have more information to print than we were printing.
> 
> After our discussion were you going to send a new patch?
> 
> [snip]
> 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.
  2014-07-17 17:58       ` Rodrigo Vivi
@ 2014-07-18  2:42         ` Ben Widawsky
  0 siblings, 0 replies; 6+ messages in thread
From: Ben Widawsky @ 2014-07-18  2:42 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Ben Widawsky

On Thu, Jul 17, 2014 at 10:58:17AM -0700, Rodrigo Vivi wrote:
> With the increasing number of rings,
> we probably have more information to print than we were printing.
> 
> v2: Loop only over active rings and print info with ring names.
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 36a7960..b1848e0 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -242,6 +242,10 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>  				  struct drm_device *dev,
>  				  struct drm_i915_error_ring *ring)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_engine_cs *from, *to;
> +	int i, j;
> +
>  	if (!ring->valid)
>  		return;
>  
> @@ -264,23 +268,19 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		err_printf(m, "  RC PSMI: 0x%08x\n", ring->rc_psmi);
>  		err_printf(m, "  FAULT_REG: 0x%08x\n", ring->fault_reg);
> -		err_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> -			   ring->semaphore_mboxes[0],
> -			   ring->semaphore_seqno[0]);
> -		err_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> -			   ring->semaphore_mboxes[1],
> -			   ring->semaphore_seqno[1]);
> -		if (HAS_VEBOX(dev)) {
> -			err_printf(m, "  SYNC_2: 0x%08x [last synced 0x%08x]\n",
> -				   ring->semaphore_mboxes[2],
> -				   ring->semaphore_seqno[2]);
> +		for_each_ring(from, dev_priv, i) {
> +			for_each_ring(to, dev_priv, j) {
> +				int idx = intel_ring_sync_index(from, to);
If you plan to get the gen8 gaps in the object:

if (i == j && !IS_GEN8())
	continue;

If not, with the existing code:
if (i == j)
	continue;

Currently, I am also in favor of
for(i = 0; i < NUM_RINGS - 1; i++) {
	if (!(INTEL_INFO(dev)->ring_mask & (1<<i)))
		continue;
	for(j = 0; j < NUM_RINGS - 1; j++) {
		if (!(INTEL_INFO(dev)->ring_mask & (j<<i)))
			continue;

		err_printf(...)
	}
}

or use if (ring_is_initialied()) instead of checking ring mask. For
error state, I prefer to use the mask because who knows what has
happened to the rings on reset or something.

Whatever you like. Fix the if (i == j) and lgtm

> +				err_printf(m, "  SYNC[%s -> %s]: 0x%08x [last synced 0x%08x]\n",
> +					   from->name, to->name,
> +					   ring->semaphore_mboxes[idx],
> +					   ring->semaphore_seqno[idx]);
> +			}



>  		}
>  	}
>  	if (USES_PPGTT(dev)) {
>  		err_printf(m, "  GFX_MODE: 0x%08x\n", ring->vm_info.gfx_mode);
> -

Should probably remove this if you do a respin.

>  		if (INTEL_INFO(dev)->gen >= 8) {
> -			int i;
>  			for (i = 0; i < 4; i++)
>  				err_printf(m, "  PDP%d: 0x%016llx\n",
>  					   i, ring->vm_info.pdp[i]);
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-18  2:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-17 16:39 [PATCH] drm/i915: print full error ring semaphore mboxes and sync Rodrigo Vivi
2014-07-18  0:23 ` Ben Widawsky
2014-07-18  0:36   ` Vivi, Rodrigo
2014-07-18  0:42     ` Ben Widawsky
2014-07-17 17:58       ` Rodrigo Vivi
2014-07-18  2:42         ` Ben Widawsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.