Hi, On 12 May 2015 at 14:16, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Tue, May 12, 2015 at 02:06:39PM +0200, Maarten Lankhorst wrote: >> Op 11-05-15 om 19:11 schreef Daniel Vetter: >> > On Mon, May 11, 2015 at 04:24:40PM +0200, Maarten Lankhorst wrote: >> >> @@ -6079,26 +6059,29 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable) >> >> enum intel_display_power_domain domain; >> >> unsigned long domains; >> >> >> >> + if (enable == intel_crtc->active) >> >> + return; >> >> + >> >> + if (enable && !crtc->state->enable) >> >> + return; >> > I think we only need to check for !state->enable here. Changing dpms while >> > the crtc is fully of is totally legit. And at least -modesetting loves to >> > do just that iirc. >> > >> > I'll will be caught by state->active implying state->enable, but that's >> > hard to read imo. >> > -Daniel >> As discussed on irc it's not. :-) > > Hm there's actually a bug in drm_atomic_helper_connector_dpms I think ... > we seem to unconditionally update crtc_state->active. > > Oh I'm missing that ->enable == false also implies that no connectors are > connected to the crtc, so we can't ever end up setting this to true. So > indeed changing active while enable == false is impossible. Not the only place we seem to rather carelessly do this, mind: https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.1.x/unify-flip-modeset&id=132a1d4086e20cac95094c7fd568f992e4a0cb6d Cheers, Daniel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
