On Thu, 5 Jan 2012 12:29:08 +0100, Daniel Vetter <daniel at ffwll.ch> wrote: > - The reset code (running from a workqueue) does hold sturct mutex. It's > the hangcheck and error state capture code running from softirq/timer > context causing issues. Right, I mis-wrote; I meant the hangcheck timer (which I always think of as part of the reset code). > - Forcewake works like a reference counted resources. As long as all > _get/_put calls are properly balanced, I don't see how somebody could > sneak in in between (when we don't hold the spinlock) and cause havoc. > For paranaoia we might want to drop a WARN_ON in the _put call to check > whether it ever drops below 0. But all current users are clearly > balanced, so I didn't bother with it. Right, I was just confused somehow. Still seems weird to me to drop a spinlock, execute a single instruction, and then immediately re-acquire it, along with bumping forcewake_count twice. > - My missed IRQ w/a actually relies on this by grabbing a forcewake ref in > get_irq and dropping it again in put_irq. In between there's usually a > schedule(). This is essentially the same as the user-level forcewake and would be handled in the same way -- keep forcewake_count, but use it only for long-term values. > - I've pondered with Chris whether we should do your proposed optimization > but we then noticed that the gem code doesn't actually read from any > forcewake protected registers in normal execution (I don't consider > running the hangcheck timer normal ;-). With my missed irq w/a that now > changes, so we might need to reconsider this. But imo that's material > for a separate patch. Yeah, all sounds reasonable. That separate patch can actually use per-chip functions to read/write from the chip so we can also avoid checking the forcewake stuff on register reads for older generation hardware. Make it work, then make it work faster. -- keith.packard at intel.com -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 827 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120105/66604d6b/attachment.pgp>
