On Fri, 3 May 2013 17:55:31 +0200 Daniel Vetter <daniel at ffwll.ch> wrote: > 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. Agreed; there's definitely redundancy here, but turning the display on at mode set time when it was clearly intended and implied by other usages seems like following the policy of least surprise. -- Jesse Barnes, Intel Open Source Technology Center
