[Intel-gfx] [PATCH] drm/i915: RMW register cycles considered evil

[Daniel Vetter]

On Mon, Jul 06, 2015 at 03:07:16PM +0100, Dave Gordon wrote:
> On 06/07/15 13:50, Ville Syrjälä wrote:
> >On Mon, Jul 06, 2015 at 02:42:02PM +0200, Daniel Vetter wrote:
> >>Especially for workarounds which is stuff that's almost impossible to
> >>verify: The initial state from the firmware on boot-up and after
> >>resume could be different, which will hide bugs when we do an RMW
> >>cycle.
> >
> >If you're really worried about that then we should then explicitly
> >initialize all the registers that might affect stuff.
> >
> >For a bunch of GT registers we could just do a GPU reset at driver
> >init. That that won't help with UCGCTL and such.
> >
> >I'm also worried that if we don't use RMWs for early parts, the hardware
> >folks may still change the default for some ofhte other bits, and then
> >we end up clobbering those.
> 
> In other drivers, I've found a good pattern to be:
> 
> 1.	during driver load, snapshot (just once) anything that the BIOS
> 	may have programmed that we may need later
> 2.	then reset the h/w and reprogram it totally to our preferred
> 	values, which may to a greater or lesser degree be derived from
> 	the saved BIOS settings
> 3.	during unload, reset the h/w again and reprogram it with the
> 	BIOS settings
> 
> 2a.	resume is just like load, except we don't need or want to
> 	capture the BIOS settings first
> 3a.	suspend is like unload, except in some cases the BIOS values
> 	might need to be tweaked when writing them back in order to
> 	ensure the device doesn't generate further activity.
> 
> Step 1 ensures that we don't lose useful settings where the BIOS knows
> better than the driver what the good values are.
> Step 2/2a. ensures that the device starts out in a well-defined state,
> regardless of how comprehensively (or badly) the BIOS has set it up.
> It seems a reasonable compromise between taking advantage of good BIOSes
> while not getting too much hassle from broken ones.

We've had large-scale save/restore hooks and they're another kind of
trouble: It usually means you end up duplicating between how you touch
registers at runtime and the giant resume code. And there's always going
to be some overlap (our hw engineers are creative enough for that) which
means if there's subtle ordering rules those two paths inevitably will
diverge.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch