All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix interrupt request miss problem in bsd ring for g4x
@ 2011-04-26 10:12 Feng, Boqun
  2011-04-26 14:28 ` Chris Wilson
  2011-04-26 17:55 ` Keith Packard
  0 siblings, 2 replies; 10+ messages in thread
From: Feng, Boqun @ 2011-04-26 10:12 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Feng, Boqun <boqun.feng@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e9e6f71..6606ca7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -609,8 +609,12 @@ ring_get_irq(struct intel_ring_buffer *ring, u32 flag)
 	       return false;
 
 	spin_lock(&ring->irq_lock);
-	if (ring->irq_refcount++ == 0)
-		ironlake_enable_irq(dev_priv, flag);
+	if (ring->irq_refcount++ == 0) {
+        if (HAS_PCH_SPLIT(dev))
+		    ironlake_enable_irq(dev_priv, flag);
+        else
+            i915_enable_irq(dev_priv, flag);
+    }
 	spin_unlock(&ring->irq_lock);
 
 	return true;
@@ -623,8 +627,12 @@ ring_put_irq(struct intel_ring_buffer *ring, u32 flag)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 
 	spin_lock(&ring->irq_lock);
-	if (--ring->irq_refcount == 0)
-		ironlake_disable_irq(dev_priv, flag);
+	if (--ring->irq_refcount == 0) {
+        if (HAS_PCH_SPLIT(dev))
+		    ironlake_disable_irq(dev_priv, flag);
+        else
+            i915_disable_irq(dev_priv, flag);
+    }
 	spin_unlock(&ring->irq_lock);
 }
 
@@ -666,12 +674,22 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
 static bool
 bsd_ring_get_irq(struct intel_ring_buffer *ring)
 {
-	return ring_get_irq(ring, GT_BSD_USER_INTERRUPT);
+	struct drm_device *dev = ring->dev;
+
+    if (HAS_PCH_SPLIT(dev))
+	    return ring_get_irq(ring, GT_BSD_USER_INTERRUPT);
+    else
+        return ring_get_irq(ring, I915_BSD_USER_INTERRUPT);
 }
 static void
 bsd_ring_put_irq(struct intel_ring_buffer *ring)
 {
-	ring_put_irq(ring, GT_BSD_USER_INTERRUPT);
+	struct drm_device *dev = ring->dev;
+    
+    if (HAS_PCH_SPLIT(dev))
+	    ring_put_irq(ring, GT_BSD_USER_INTERRUPT);
+    else
+        ring_put_irq(ring, I915_BSD_USER_INTERRUPT);
 }
 
 static int
-- 
1.7.3.2

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-26 10:12 [PATCH] fix interrupt request miss problem in bsd ring for g4x Feng, Boqun
@ 2011-04-26 14:28 ` Chris Wilson
  2011-04-27  6:08   ` Feng, Boqun
  2011-04-26 17:55 ` Keith Packard
  1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2011-04-26 14:28 UTC (permalink / raw)
  To: Feng, Boqun, intel-gfx

Only, the first 2 chunks are required. The gen6+ paths are unaffected.
And be careful of your whitespace.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-26 10:12 [PATCH] fix interrupt request miss problem in bsd ring for g4x Feng, Boqun
  2011-04-26 14:28 ` Chris Wilson
@ 2011-04-26 17:55 ` Keith Packard
  2011-04-27  4:58   ` Feng, Boqun
  1 sibling, 1 reply; 10+ messages in thread
From: Keith Packard @ 2011-04-26 17:55 UTC (permalink / raw)
  To: Feng, Boqun, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 242 bytes --]

On Tue, 26 Apr 2011 18:12:52 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote:
> Signed-off-by: Feng, Boqun <boqun.feng@intel.com>

Please add a description here of what the bug was and how this fixes it.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-26 17:55 ` Keith Packard
@ 2011-04-27  4:58   ` Feng, Boqun
  0 siblings, 0 replies; 10+ messages in thread
From: Feng, Boqun @ 2011-04-27  4:58 UTC (permalink / raw)
  To: Keith Packard, intel-gfx

We need this fix to enable h264 decoding on G4x.
And I will resend a patch, more clear and standard.

-----Original Message-----
From: Keith Packard [mailto:keithp@keithp.com] 
Sent: Wednesday, April 27, 2011 1:55 AM
To: Feng, Boqun; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] fix interrupt request miss problem in bsd ring for g4x

On Tue, 26 Apr 2011 18:12:52 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote:
> Signed-off-by: Feng, Boqun <boqun.feng@intel.com>

Please add a description here of what the bug was and how this fixes it.

-- 
keith.packard@intel.com

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-26 14:28 ` Chris Wilson
@ 2011-04-27  6:08   ` Feng, Boqun
  2011-04-27  7:39     ` Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Feng, Boqun @ 2011-04-27  6:08 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

I am very sorry for my careless about whitespace.

But my patch will not affect gen6+ paths, for gen6+, it use gen6_bsd_ring
, bsd_ring is only used by g4x and ironlake.
Besides, since bsd_ring_get_irq/bsd_ring_put_irq/ring_get_irq/ring_put_irq 
are only used by bsd_ring, can we use a patch to merge them into two function? 

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Tuesday, April 26, 2011 10:29 PM
To: Feng, Boqun; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] fix interrupt request miss problem in bsd ring for g4x

Only, the first 2 chunks are required. The gen6+ paths are unaffected.
And be careful of your whitespace.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-27  6:08   ` Feng, Boqun
@ 2011-04-27  7:39     ` Chris Wilson
  2011-04-27  8:23       ` Feng, Boqun
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2011-04-27  7:39 UTC (permalink / raw)
  To: Feng, Boqun, intel-gfx

On Wed, 27 Apr 2011 14:08:57 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote:
> I am very sorry for my careless about whitespace.
> 
> But my patch will not affect gen6+ paths, for gen6+, it use gen6_bsd_ring
> , bsd_ring is only used by g4x and ironlake.

Reviewer error, sorry. Saw the gen6_* in the diff header as the function
affected and believed it.

> Besides, since bsd_ring_get_irq/bsd_ring_put_irq/ring_get_irq/ring_put_irq 
> are only used by bsd_ring, can we use a patch to merge them into two function? 
Yes, once upon a time they differed, now they are the same so please do
merge them and give them a more useful name: g4x_ring_* so that there is a
constant reminder that g4x also has a BSD ring and that the functions are
not expected to be used with earlier chipsets.

Daniel has done similar things for gen6 once we decided to drop the
pre-production workarounds.

Obviously that is a separate patch to the bug fix.  Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-27  7:39     ` Chris Wilson
@ 2011-04-27  8:23       ` Feng, Boqun
  2011-04-27  9:15         ` Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Feng, Boqun @ 2011-04-27  8:23 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Err...I just send another two patches before read this letter. : )

Ironlake and g4x share the same bsd_ring, so they share the same
bsd_ring_put/get_irq functions of the ring. Given this, we can't just
change the function name to g4x_ring_put/get_irq. If we do so, we
need ironlake_ring_put/get_irq, too.
So I just use a if-else in bsd_ring_*_irq to find out the version of the
chipset and do different work.
Is that OK?


-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 
Sent: Wednesday, April 27, 2011 3:39 PM
To: Feng, Boqun; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] fix interrupt request miss problem in bsd ring for g4x

On Wed, 27 Apr 2011 14:08:57 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote:
> I am very sorry for my careless about whitespace.
> 
> But my patch will not affect gen6+ paths, for gen6+, it use gen6_bsd_ring
> , bsd_ring is only used by g4x and ironlake.

Reviewer error, sorry. Saw the gen6_* in the diff header as the function
affected and believed it.

> Besides, since bsd_ring_get_irq/bsd_ring_put_irq/ring_get_irq/ring_put_irq 
> are only used by bsd_ring, can we use a patch to merge them into two function? 
Yes, once upon a time they differed, now they are the same so please do
merge them and give them a more useful name: g4x_ring_* so that there is a
constant reminder that g4x also has a BSD ring and that the functions are
not expected to be used with earlier chipsets.

Daniel has done similar things for gen6 once we decided to drop the
pre-production workarounds.

Obviously that is a separate patch to the bug fix.  Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-27  8:23       ` Feng, Boqun
@ 2011-04-27  9:15         ` Chris Wilson
  2011-04-27 11:07           ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2011-04-27  9:15 UTC (permalink / raw)
  To: Feng Boqun, intel-gfx

On Wed, 27 Apr 2011 16:23:00 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote:
> Err...I just send another two patches before read this letter. : )
> 
> Ironlake and g4x share the same bsd_ring, so they share the same
> bsd_ring_put/get_irq functions of the ring. Given this, we can't just
> change the function name to g4x_ring_put/get_irq. If we do so, we
> need ironlake_ring_put/get_irq, too.
> So I just use a if-else in bsd_ring_*_irq to find out the version of the
> chipset and do different work.
> Is that OK?

That's ok. We are slowly pushing the branches out to the initialisation so
that the code paths each generation takes are a little clearer.

At the moment, I'm more concerned about making sure our functions are
consistently named and prefixed with the chipset they first work with.

So we have:
  intel_ -> general functions, used by all
  i8xx_ -> gen2
  i915_ -> gen3 (915/945)
  g33_, pineview_ -> gen3 (blk/pnv) # perhaps just g33 as pnv = g33 + mobile?
  i965_ -> gen4 (brw/crl)
  g4x_ -> gen4 (egl/ctg)
  ironlake_, sandybridge_, ivybridge_ -> etc 

So ironlake can call a g4x function, but never vice versa.

[And I'd love to be able to bring the same level of consistency to our
register names. Along with making sure that they correspond with public
docs.]

The current best practice for enablement is then to introduce new
functions for new chipsets (copy and paste, then modify) ensure the
hardware is working, then reduce until we have the minimal code with no
regressions.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-27  9:15         ` Chris Wilson
@ 2011-04-27 11:07           ` Daniel Vetter
  2011-04-27 16:07             ` Ben Widawsky
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2011-04-27 11:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Apr 27, 2011 at 11:15 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> At the moment, I'm more concerned about making sure our functions are
> consistently named and prefixed with the chipset they first work with.
>
> So we have:
>  intel_ -> general functions, used by all
>  i8xx_ -> gen2
>  i915_ -> gen3 (915/945)
>  g33_, pineview_ -> gen3 (blk/pnv) # perhaps just g33 as pnv = g33 + mobile?
>  i965_ -> gen4 (brw/crl)
>  g4x_ -> gen4 (egl/ctg)
>  ironlake_, sandybridge_, ivybridge_ -> etc
>
> So ironlake can call a g4x function, but never vice versa.

Very-Much-Wanted-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Can you put that somewhere prominent in the sources (a new file
naming_conventions.txt)?
Perhaps with the guidelines I've snipped away ...
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH] fix interrupt request miss problem in bsd ring for g4x
  2011-04-27 11:07           ` Daniel Vetter
@ 2011-04-27 16:07             ` Ben Widawsky
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Widawsky @ 2011-04-27 16:07 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Wed, Apr 27, 2011 at 01:07:24PM +0200, Daniel Vetter wrote:
> On Wed, Apr 27, 2011 at 11:15 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > At the moment, I'm more concerned about making sure our functions are
> > consistently named and prefixed with the chipset they first work with.
> >
> > So we have:
> > ?intel_ -> general functions, used by all
> > ?i8xx_ -> gen2
> > ?i915_ -> gen3 (915/945)
> > ?g33_, pineview_ -> gen3 (blk/pnv) # perhaps just g33 as pnv = g33 + mobile?
> > ?i965_ -> gen4 (brw/crl)
> > ?g4x_ -> gen4 (egl/ctg)
> > ?ironlake_, sandybridge_, ivybridge_ -> etc
> >
> > So ironlake can call a g4x function, but never vice versa.
> 
> Very-Much-Wanted-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Can you put that somewhere prominent in the sources (a new file
> naming_conventions.txt)?
> Perhaps with the guidelines I've snipped away ...
> -Daniel

Acked!

Though I don't feel what you said is explicit enough, and therefore
needs more clarity which Daniel asked for. ironlake should be able to
call intel_*, and probably most g4x_*, but maybe it can't use some i965
functions, and certainly can't use many i8xx functions.

Ben

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

end of thread, other threads:[~2011-04-27 16:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-26 10:12 [PATCH] fix interrupt request miss problem in bsd ring for g4x Feng, Boqun
2011-04-26 14:28 ` Chris Wilson
2011-04-27  6:08   ` Feng, Boqun
2011-04-27  7:39     ` Chris Wilson
2011-04-27  8:23       ` Feng, Boqun
2011-04-27  9:15         ` Chris Wilson
2011-04-27 11:07           ` Daniel Vetter
2011-04-27 16:07             ` Ben Widawsky
2011-04-26 17:55 ` Keith Packard
2011-04-27  4:58   ` Feng, Boqun

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.