On Sat, 14 Jan 2012 01:31:40 +0100, Daniel Vetter <daniel at ffwll.ch> wrote: > On Fri, Jan 13, 2012 at 04:11:43PM -0800, Keith Packard wrote: > > On Sat, 14 Jan 2012 00:52:31 +0100, Daniel Vetter <daniel at ffwll.ch> wrote: > > > acc101d drm/i915: Hold gt_lock across forcewake register reads > > > > > > Imo this is a simple cleanup (reading forcewake-protected registers isn't > > > really a fast-path for us), so material for -next. > > > > The 'optimization' is just a side benefit. The fix is to prevent reads > > From happening without forcewake being set. > > I still fail to see how you can sneak a read in there without forcewake > being asserted. And assuming I haven't understood you the last time around > we've discussed this, you've agreed. Yes, except during reset, where forcewake is cleared even if the lock count is non-zero. Any reads happening while reset is going on will return garbage. None of this is 'required' given the structure of the code today, it just makes it all evident without having to go through yet another long sequence of explanations. I had patches to hold the spinlock across register writes too, as using different locking for reading and writing seems like a bad plan, but I didn't put those in because writes involve spinning to wait for the fifo to drain, and that seemed like a bad thing to do while holding the spinlock. -- 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/20120113/bccadc1d/attachment.pgp>
