Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-17 Thread Ben Widawsky
On Thu, Jan 05, 2012 at 11:11:53PM +0100, Daniel Vetter wrote:
 With the new ducttape of much finer quality, this seems to be no
 longer necessary.
 
 Tested on my ivb and snb machine with the usual suspects of testcases.
 
 Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_irq.c |   11 ---
  1 files changed, 0 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index b9ba180..af54153 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -1779,17 +1779,6 @@ static void ironlake_irq_preinstall(struct drm_device 
 *dev)
   INIT_WORK(dev_priv-rps_work, gen6_pm_rps_work);
  
   I915_WRITE(HWSTAM, 0xeffe);
 - if (IS_GEN6(dev) || IS_GEN7(dev)) {
 - /* Workaround stalls observed on Sandy Bridge GPUs by
 -  * making the blitter command streamer generate a
 -  * write to the Hardware Status Page for
 -  * MI_USER_INTERRUPT.  This appears to serialize the
 -  * previous seqno write out before the interrupt
 -  * happens.
 -  */
 - I915_WRITE(GEN6_BLITTER_HWSTAM, ~GEN6_BLITTER_USER_INTERRUPT);
 - I915_WRITE(GEN6_BSD_HWSTAM, ~GEN6_BSD_USER_INTERRUPT);
 - }
  
   /* XXX hotplug from PCH */
  
---
This patch is a requirement for functional simulation on Gen7. It
appears to not break Gen6 afaics. Therefore:

Acked-by: Ben Widawsky b...@bwidawsk.net

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


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-09 Thread Keith Packard
On Thu,  5 Jan 2012 23:11:53 +0100, Daniel Vetter daniel.vet...@ffwll.ch 
wrote:

 With the new ducttape of much finer quality, this seems to be no
 longer necessary.
 
 Tested on my ivb and snb machine with the usual suspects of testcases.

Eric suggested that unless we have evidence that the new work-around
fixes bugs on SNB, that we should continue to use the HWSTAM work-around
on that chip and use the RC6 voodoo on IVB, thus not potentially causing
regressions on SNB.

For back-porting to older kernels, that's obviously the right plan. I
think it's also the right plan for newer kernels, but I'd love to hear
alternate views on the matter.

-- 
keith.pack...@intel.com


pgpqMqxAacKmx.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-09 Thread Daniel Vetter
On Mon, Jan 09, 2012 at 02:00:47PM -0800, Keith Packard wrote:
 On Thu,  5 Jan 2012 23:11:53 +0100, Daniel Vetter daniel.vet...@ffwll.ch 
 wrote:
 
  With the new ducttape of much finer quality, this seems to be no
  longer necessary.
  
  Tested on my ivb and snb machine with the usual suspects of testcases.
 
 Eric suggested that unless we have evidence that the new work-around
 fixes bugs on SNB, that we should continue to use the HWSTAM work-around
 on that chip and use the RC6 voodoo on IVB, thus not potentially causing
 regressions on SNB.
 
 For back-porting to older kernels, that's obviously the right plan. I
 think it's also the right plan for newer kernels, but I'd love to hear
 alternate views on the matter.

I honestly don't trust my patch, so I'd like to give it as much validation
as possible. Which means:
- Shove it into -next and beat on it there. We can ship current 3.3 with
  Eric's workaround - it's not great but at least this works.
- Enable the voodoo and revert the HWSTAM w/a also on snb - there are
  orders more snb machines in the wild than pre-production ivbs. I.e. this
  hopefully greatly increases our changes to find out whether the voodoo
  really works or if it is only pretty decent, but not perfect ducttape.
- See what happens and act accordingly (maybe reinstate the HWSTAM w/a if
  it's required). If things really work out when this hits mainline,
  backport the voodoo patch, leaving the HWSTAM in place for older
  kernels.

Yep, I'm officially paranoid about this ;-) rc6, forcewake and friends
have simply blown up too often in unpredictable ways ...
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-09 Thread Keith Packard
On Tue, 10 Jan 2012 00:39:52 +0100, Daniel Vetter dan...@ffwll.ch wrote:

 I honestly don't trust my patch, so I'd like to give it as much validation
 as possible. Which means:
 - Shove it into -next and beat on it there. We can ship current 3.3 with
   Eric's workaround - it's not great but at least this works.

Actually, sticking your patch in as a fix for 3.3 before RC1 means we'll
get loads more testing as more people test the RCs than will ever touch
drm-intel-next. Dave Airlie might have an opinion on whether that's
reasonable at this stage or not.

 - Enable the voodoo and revert the HWSTAM w/a also on snb - there are
   orders more snb machines in the wild than pre-production ivbs. I.e. this
   hopefully greatly increases our changes to find out whether the voodoo
   really works or if it is only pretty decent, but not perfect ducttape.

I suspect that the hardware is different enough between IVB and SNB that
SNB testing won't tell us all that much though. And, we have a working
SNB driver right now, with the HWSTAM work-around in place. I'd be
perfectly happy to use HWSTAM on SNB forever and use the forcewake
voodoo only on IVB.

Yeah, having the voodoo run on SNB would get a lot more testing, but
it's not going to increase our confidence on how well it works on IVB,
which is the only place it is actually needed.

 - See what happens and act accordingly (maybe reinstate the HWSTAM w/a if
   it's required). If things really work out when this hits mainline,
   backport the voodoo patch, leaving the HWSTAM in place for older
   kernels.

So, the nice thing about the two IVB work-arounds is that they can
co-exist in the kernel perfectly happily. We know that your voodoo
serves to keep the chip awake for a tiny interval after it has finished
drawing (essentially the time from the end of work to the interrupt ack
and forcewake disable), so it's not a significant additional power
drain.

We could leave the code for spinning in place and simply control that
with a module parameter. That would allow us to disable it now, and if
we find problems (or are particularly paranoid) we could disable it
before 3.3 ships with a 1-line patch.

 Yep, I'm officially paranoid about this ;-) rc6, forcewake and friends
 have simply blown up too often in unpredictable ways ...

We love our fancy hardware. The power savings brought about by rc6 are
impressive though; I only wish it didn't take so much software
support...

-- 
keith.pack...@intel.com


pgpbVLjoVFbhH.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-09 Thread Daniel Vetter
On Mon, Jan 09, 2012 at 06:09:13PM -0800, Keith Packard wrote:
 We could leave the code for spinning in place and simply control that
 with a module parameter. That would allow us to disable it now, and if
 we find problems (or are particularly paranoid) we could disable it
 before 3.3 ships with a 1-line patch.

Well, if you want to push it through fixes (I don't think it's too late
yet for such games) I agree that we want to keep around the spinning
workaround with a module option.

As Chris pointed out when submitting his rfc patch, we've now scored
missed irq issues on ivb, snb and some gen3 chips. So it looks like this
would be generally useful infrastructure. But in the case that we keep it
I have some pending review-comments on what's currently in ... ;-)

Cheers, Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-06 Thread Eugeni Dodonov
On Thu, Jan 5, 2012 at 20:11, Daniel Vetter daniel.vet...@ffwll.ch wrote:

 With the new ducttape of much finer quality, this seems to be no
 longer necessary.

 Tested on my ivb and snb machine with the usual suspects of testcases.

 Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch


Nothing like high-quality ducttape!

Reviewed-by: Eugeni Dodonov eugeni.dodo...@intel.com
Tested-by: Eugeni Dodonov eugeni.dodo...@intel.com

I wonder if we could drop the definitions of GEN6_*_HWSTAM from i915_reg.h
as well now...

-- 
Eugeni Dodonov
http://eugeni.dodonov.net/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-05 Thread Daniel Vetter
With the new ducttape of much finer quality, this seems to be no
longer necessary.

Tested on my ivb and snb machine with the usual suspects of testcases.

Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_irq.c |   11 ---
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b9ba180..af54153 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1779,17 +1779,6 @@ static void ironlake_irq_preinstall(struct drm_device 
*dev)
INIT_WORK(dev_priv-rps_work, gen6_pm_rps_work);
 
I915_WRITE(HWSTAM, 0xeffe);
-   if (IS_GEN6(dev) || IS_GEN7(dev)) {
-   /* Workaround stalls observed on Sandy Bridge GPUs by
-* making the blitter command streamer generate a
-* write to the Hardware Status Page for
-* MI_USER_INTERRUPT.  This appears to serialize the
-* previous seqno write out before the interrupt
-* happens.
-*/
-   I915_WRITE(GEN6_BLITTER_HWSTAM, ~GEN6_BLITTER_USER_INTERRUPT);
-   I915_WRITE(GEN6_BSD_HWSTAM, ~GEN6_BSD_USER_INTERRUPT);
-   }
 
/* XXX hotplug from PCH */
 
-- 
1.7.7.3

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


Re: [Intel-gfx] [PATCH] drm/i915: rip out the HWSTAM missed irq workaround

2012-01-05 Thread Ben Widawsky
On Thu, Jan 05, 2012 at 11:11:53PM +0100, Daniel Vetter wrote:
 With the new ducttape of much finer quality, this seems to be no
 longer necessary.
 
 Tested on my ivb and snb machine with the usual suspects of testcases.
 
 Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_irq.c |   11 ---
  1 files changed, 0 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index b9ba180..af54153 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -1779,17 +1779,6 @@ static void ironlake_irq_preinstall(struct drm_device 
 *dev)
   INIT_WORK(dev_priv-rps_work, gen6_pm_rps_work);
  
   I915_WRITE(HWSTAM, 0xeffe);
 - if (IS_GEN6(dev) || IS_GEN7(dev)) {
 - /* Workaround stalls observed on Sandy Bridge GPUs by
 -  * making the blitter command streamer generate a
 -  * write to the Hardware Status Page for
 -  * MI_USER_INTERRUPT.  This appears to serialize the
 -  * previous seqno write out before the interrupt
 -  * happens.
 -  */
 - I915_WRITE(GEN6_BLITTER_HWSTAM, ~GEN6_BLITTER_USER_INTERRUPT);
 - I915_WRITE(GEN6_BSD_HWSTAM, ~GEN6_BSD_USER_INTERRUPT);
 - }
  
   /* XXX hotplug from PCH */
  


I'd prefer just reverting the old commits instead of this, but that's
just me.

Reviewed-and-Tested-by: Ben Widawsky b...@bwidawsk.net


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