On Wed, May 29, 2013 at 10:09:34AM +0200, Daniel Vetter wrote: > On Wed, May 29, 2013 at 9:54 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > On Wed, May 29, 2013 at 09:28:22AM +0200, Daniel Vetter wrote: > >> If we always force the pipe A to on we can't use the hw state to > >> decide whether it should be on. Hence quirk the quirk. > > > > This is misleading as it is not the hw state that is unreliable, but > > crtc->active that is a misnomer. The quirk makes the hw state > > inconsistent with the bookkeeping. > > Well, crtc->active tracks much more than what we quirk, it means that > the entire display pipe is on (including encoders, planes and all) and > pumping pixels. The quirk only keeps pipe&pll running though. I've > seen two options: > - this one here > - pimping the get_config stuff to figure out we're quirked and e.g. > check whether the plane is running. > > Clamping the tracked state seemed to be the quicker option, and we > already have similar (in spirit) hacks in assert_pipe. > > Also, we shouldn't lose any assert coverage with this: The pipe > enabled state is independently asserted in the enable/disable > functions and the hw state doesn't really check anything if the pipe > is logically off. Completely agree; I came to the same assessment myself wondering if you were covering up some nasty bug. It's just the wording here that makes me think that the hardware readback is broken, when in fact it is just inconsistent with our expectations due to our misleading the state tracking. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html