On Thu, May 22, 2014 at 9:26 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Thu, May 22, 2014 at 03:10:37PM -0300, Paulo Zanoni wrote: >> 2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>: >> > All the other checks also check hw state, so checking our software >> > refcounts for the plls looks a bit odd. >> >> As I mentioned before, this contradicts your own previous review of >> the patch that added this code. In addition, you said many times that >> we should do SW checks instead of HW checks when possible. >> >> What should be looking odd here is that we check registers directly >> for the other stuff, instead of looking at some struct :) >> >> > Also this will simplify the >> > conversion over to the shared dpll framework, which itself has massive >> > amounts of checks to make sure that we never leave a display pll >> > enabled when we shouldn't. >> >> What you wrote above is a nice reason to check the new shared DPLL >> structs instead of the registers directly: it has tons of WARNs, so >> it's unlikely the structs will be wrong. > > If I've done this correctly this should happen a few patches later on. If > not I can throw a new patch on top. I'll check this. Ah, actually we don't need it any more, the new stuff is much better ;-) assert_can_disable_lcpll checks crtc->active for all pipes. And the modeset state checker makes sure that the refcounts perfectly match the crtcs, i.e. wrpll_refcount = \sum crtc->active. This is how we check the sw side of things. Adding yet another sw check here feels like too much overkill.y -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
