[Intel-gfx] [PATCH 39/66] drm/i915: Check hw state in assert_can_disable_lcpll

[Daniel Vetter]

On Thu, May 22, 2014 at 9:26 PM, Daniel Vetter <daniel at ffwll.ch> 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 at ffwll.ch>:
>> > 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