| Message ID | 1367580129-6681-1-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 3, 2013 at 1:22 PM, Imre Deak <imre.deak@intel.com> 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@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339 Safe for the first just a few wild guesses. Can you please spam bugzilla with this patch (there's tons more spurious kms_flip fail iirc, also a few of the kms_flip jitter reports should be fixed by now). Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Fri, May 3, 2013 at 1:27 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, May 3, 2013 at 1:22 PM, Imre Deak <imre.deak@intel.com> 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@intel.com> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339 > > Safe for the first just a few wild guesses. Can you please spam > bugzilla with this patch (there's tons more spurious kms_flip fail > iirc, also a few of the kms_flip jitter reports should be fixed by > now). Also, and i-g-t to specifically exercise this would be nice ... Maybe easiest on top of kms_flip, using dpms off, but then a set_base as an implicit dpms on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Fri, 2013-05-03 at 13:32 +0200, Daniel Vetter wrote: > On Fri, May 3, 2013 at 1:27 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, May 3, 2013 at 1:22 PM, Imre Deak <imre.deak@intel.com> 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@intel.com> > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61642 Chris marked this as a dup for another bug with an other root cause. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59834 Yes, this is tested against the patch already. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59339 Ok I'll point them to the fix. > > Safe for the first just a few wild guesses. Can you please spam > > bugzilla with this patch (there's tons more spurious kms_flip fail > > iirc, also a few of the kms_flip jitter reports should be fixed by > > now). > > Also, and i-g-t to specifically exercise this would be nice ... Maybe > easiest on top of kms_flip, using dpms off, but then a set_base as an > implicit dpms on. Ok, will put together something. --Imre > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
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@intel.com> NAK. Papering over bugs, much? -Chris
On Fri, 03 May 2013, Imre Deak <imre.deak@intel.com> 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@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2939524..0aee2ba 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct drm_device *dev, > } > } > > +static bool > +crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors, > + int num_connectors) > +{ > + int i; > + > + for (i = 0; i < num_connectors; i++) > + if (connectors[i].encoder && > + connectors[i].encoder->crtc == crtc && > + connectors[i].dpms != DRM_MODE_DPMS_ON) > + return true; > + > + return false; > +} > + > static void > intel_set_config_compute_mode_changes(struct drm_mode_set *set, > struct intel_set_config *config) > @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, > } else if (set->fb->pixel_format != > set->crtc->fb->pixel_format) { > config->mode_changed = true; > - } else > + } else if (crtc_connector_off(set->crtc, *set->connectors, > + set->num_connectors)) { Bikeshed, crtc_connector_off() sounds like disabling something, while is_crtc_connector_off() would sound like checking the state. Jani. > + config->mode_changed = true; > + } else { > config->fb_changed = true; > + } > } > > if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y)) > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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.. --Imre
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@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. -Chris
On Fri, 2013-05-03 at 13:22 +0100, Chris Wilson 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@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. Ok, so I assume the right approach for user space is to do an explicit DPMS_ON after modesetting. --Imre
On Fri, May 3, 2013 at 2:22 PM, Chris Wilson <chris@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@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
On Fri, 3 May 2013 17:55:31 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, May 3, 2013 at 2:22 PM, Chris Wilson <chris@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@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.
On Fri, May 03, 2013 at 05:55:31PM +0200, Daniel Vetter wrote: > On Fri, May 3, 2013 at 2:22 PM, Chris Wilson <chris@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@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. Not quite. It was added afterwards, I know for it is one of my mistakes that I belatedly recognised as trying to workaround a bug in UXA. And we then layered on further bugs to try and patch up the glaring holes then introduced. > 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. I think the patch is quite ugly as we do not attempt to avoid the unnecessary modeset step and so penalise working userspace code. Now, if you were first to make intel_modeset_affected_pipes() smarter, such that the explicit fb set-base optimisation was no longer ever necessary, things would be more interesting. -Chris
On Fri, May 3, 2013 at 6:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Not quite. It was added afterwards, I know for it is one of my mistakes > that I belatedly recognised as trying to workaround a bug in UXA. And we > then layered on further bugs to try and patch up the glaring holes then > introduced. I've done some digging around and found some cool stuff, but no change which introduce the original bug that we've never consulted dpms state in drm_crtc_helper_set_mode but just blindly enabled encoders. Most of the commits I've dug out try to paper over that mismatch by updating dpms state, too. E.g. bf9dc102 which was then fixed up a bit in 811aaa55 but I don't see any mention of someone explicitly kill dpms checks. Of course for DP support we've started to add dpms state checks to our dp encoder (since double-enable kills DP). Then duplicated the drm_connector->dpms into our own intel_dp->dpms state since the crtc helpers liked to frob around with dpms in a rather ill-defined way. But that's just hilarity and flailing around on top, nothing that I could see fundamentally changed that a full setcrtc implicitly enabled the encoders/crtc hw. And since we have our own modeset code that all disappeared (since a full modeset simply force-updates the dpms state to reflect reality). But nowhere have a found a commit to support your claim, just lots of flailing around due to the ill-defined semantics of this all. Care to do some more digging? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
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@intel.com> [..] > @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, > } else if (set->fb->pixel_format != > set->crtc->fb->pixel_format) { > config->mode_changed = true; > - } else > + } else if (crtc_connector_off(set->crtc, *set->connectors, > + set->num_connectors)) { > + config->mode_changed = true; > + } else { > config->fb_changed = true; > + } > } > > if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y)) On https://bugs.freedesktop.org/show_bug.cgi?id=64178 I had a similar problem for which I found a very similar 'workaround': Assuming the Xserver is displaying a mode a on output A, doing a: xrandr --output A --off xrandr --output A --mode a will not light up the screen. This all sounds quite similar, to the issues describe by this patch, however the patch as is will not fix this issue: With this setup the fb does not change. The crtc_connector_off() test however only runs if set->crtc->fb != set->fb is true. Thus the connector_off() test needs to happen before: - if (set->crtc->fb != set->fb) { + if (set->connectors != NULL && + connector_off(set->crtc, *set->connectors, + set->num_connectors)) { + config->mode_changed = true; + } else if (set->crtc->fb != set->fb) { Mind the test for set->connectors != NULL as now the connectors list may be empty. Cheers, Egbert.
On Fri, May 3, 2013 at 6:50 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, May 3, 2013 at 6:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> Not quite. It was added afterwards, I know for it is one of my mistakes >> that I belatedly recognised as trying to workaround a bug in UXA. And we >> then layered on further bugs to try and patch up the glaring holes then >> introduced. > > I've done some digging around and found some cool stuff, but no change > which introduce the original bug that we've never consulted dpms state > in drm_crtc_helper_set_mode but just blindly enabled encoders. Most of > the commits I've dug out try to paper over that mismatch by updating > dpms state, too. E.g. bf9dc102 which was then fixed up a bit in > 811aaa55 but I don't see any mention of someone explicitly kill dpms > checks. > > Of course for DP support we've started to add dpms state checks to our > dp encoder (since double-enable kills DP). Then duplicated the > drm_connector->dpms into our own intel_dp->dpms state since the crtc > helpers liked to frob around with dpms in a rather ill-defined way. > But that's just hilarity and flailing around on top, nothing that I > could see fundamentally changed that a full setcrtc implicitly enabled > the encoders/crtc hw. And since we have our own modeset code that all > disappeared (since a full modeset simply force-updates the dpms state > to reflect reality). > > But nowhere have a found a commit to support your claim, just lots of > flailing around due to the ill-defined semantics of this all. Care to > do some more digging? Ping for history digging ... or an ack on the patch ;-) Alternative ideas are welcome too ofc, but under the assumption that setcrtc always kinda enabled encoders/connectors implicitly (neglecting a few funny corner cases every screwed up in different ways) this hack here seems the most sensible&consistent option we have. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2939524..0aee2ba 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct drm_device *dev, } } +static bool +crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors, + int num_connectors) +{ + int i; + + for (i = 0; i < num_connectors; i++) + if (connectors[i].encoder && + connectors[i].encoder->crtc == crtc && + connectors[i].dpms != DRM_MODE_DPMS_ON) + return true; + + return false; +} + static void intel_set_config_compute_mode_changes(struct drm_mode_set *set, struct intel_set_config *config) @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, } else if (set->fb->pixel_format != set->crtc->fb->pixel_format) { config->mode_changed = true; - } else + } else if (crtc_connector_off(set->crtc, *set->connectors, + set->num_connectors)) { + config->mode_changed = true; + } else { config->fb_changed = true; + } } if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
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@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)