[Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode

[Daniel Vetter]

On Fri, May 3, 2013 at 2:22 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, May 03, 2013 at 02:55:45PM +0300, Imre Deak wrote:
>> On Fri, 2013-05-03 at 12:44 +0100, Chris Wilson wrote:
>> > On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote:
>> > > Currently the driver's assumed behavior for a modeset with an attached
>> > > FB is that the corresponding connector will be switched to DPMS ON mode
>> > > if it happened to be in DPMS OFF (or another power save mode). This
>> > > wasn't enforced though if only the FB changed, everything else (format,
>> > > connector etc.) remaining the same. In this case we only set the new FB
>> > > base and left the connector in the old power save mode.
>> > >
>> > > Fix this by forcing a full modeset whenever there is an attached FB and
>> > > any affected connector is in a power save mode.
>> > >
>> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
>> >
>> > NAK. Papering over bugs, much?
>>
>> Ok. I posted this based on our IRC discussion where we seemed to agree
>> that DPMS_ON is assumed already; just that it's not done consistently.
>> That is if user space does a modeset now that ends up in a full modeset
>> (lets say a new FB with different format) then the kernel will do a
>> DPMS_ON. But if the new FB happens to be the same format then it won't.
>> I thought this is inconsistent and should be fixed.
>>
>> Another way to make it consistent would be then to remove DPMS ON from
>> the full modeset path..
>
> Right, we have mentioned in the past that the conflation between DPMS
> and modesetting is a more recent bug that is going to cause us even more
> trouble later. I am not sure how we can fix that as UXA already makes
> many bad assumptions.

Afaict the conflagration of dpms on after a modeset has been even in
the old crtc helpers. The only difference is that now we explicitly
update the dpms state tracking to avoid double-encoder enables. That
part allowed us to rip out tons of state tracking from tons of places.

So ever since kms happened, we have inconsistent setcrtc semantics.
Stopping to force dpms on will horribly break existing userspace, so
we can't do this. Making sure that the semantics are more consistent
(especially if we further optimize modeset transitions for fastboot)
seems like the lesser of two evils.

We'll get a 2nd chance to get this right the atomic modeset ioctl.
Until then, I want this duct-taped together, and Imre's patch looks
like a viable approach. Working beats ugly semantics here imo.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch