On Mon, Jul 06, 2015 at 04:15:25PM +0100, Damien Lespiau wrote: > On Mon, Jul 06, 2015 at 04:58:19PM +0200, Daniel Vetter wrote: > > On Mon, Jul 06, 2015 at 01:46:19PM +0100, Damien Lespiau 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. > > > > > > > > Hence never do them, and if it's required we need a special mask. > > > > > > > > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx> > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > > Cc: Nick Hoath <nicholas.hoath@xxxxxxxxx> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > > > > > Eeek. Let's take the problem the other way around: have you verified > > > it's OK to zero all those other fields? > > > > Nope, but it's what we do for other workarounds (e.g. the ones we load > > through the rings except for one case in the cxt switch wa) and on other > > platforms. And in general we've moved away from RMW wherever we can since > > it had too much surprises. > > I don't think that's really fair. Most W/As through the rings are > touching masked registers, just setting/clearing specific bits. > > I just looked at the *_init_clock_gating() function and it's full > of RMW cycles as well. We roughly have no idea of most of the early init > from the firmware and really want to reuse those when we're missing the > info about those bits. > > > It's really just something I spotted while stumbling over a w/a patch for > > hsw that we never merged - I don't like the inconsistency. And it has > > bitten us in the past. > > > > And yes I haven't done the audit here, but the fact that you suggest we > > need one kind proves my point ;-) > > I don't mind the spririt of this, but it requires a massive amount of > lore that is currently done in the firmware. Not at all practical with > the amount of knowledge we have on low level units and early init > sequences and W/As. Yeah I think I checked a biased sample for this case. Specically I ended up looking at GEN6_UCGCTL2 where all pre-gen9 functions don't do a RMW. But reviewing a lot more of the more modern clock gating code we seem to be simply inconsistent all across the place. I guess if someone would be really bored we could go through all modern-ish w/a and check that all platforms apply them in a uniform way, instead of the current MO where we walk the wa db mostly on a per-platform query. I'm just really grumpy about w/a in general since we have only shitty options to validate that we have them all, so the best we can aim for is consistency, which we don't have either. I guess everyone just move on and hope nothing breaks is the right option :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
