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

[Keith Packard <keithp@keithp.com>]

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

On Tue, 10 Jan 2012 00:39:52 +0100, Daniel Vetter <daniel@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.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 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