On Tue, Jul 07, 2015 at 12:05:40PM +0200, Maarten Lankhorst wrote:
> Op 07-07-15 om 10:59 schreef Daniel Vetter:
> > On Tue, Jul 07, 2015 at 09:08:12AM +0200, Maarten Lankhorst wrote:
> >> @@ -373,7 +375,17 @@ drm_atomic_helper_check_modeset(struct drm_device 
> >> *dev,
> >>            if (crtc->state->enable != crtc_state->enable) {
> >>                    DRM_DEBUG_ATOMIC("[CRTC:%d] enable changed\n",
> >>                                     crtc->base.id);
> >> +
> >> +                  /*
> >> +                   * For clarity this assignment is done here, but
> >> +                   * enable == 0 is only true when there are no
> >> +                   * connectors and a NULL mode.
> >> +                   *
> >> +                   * The other way around is true as well. enable != 0
> >> +                   * iff connectors are attached and a mode is set.
> >> +                   */
> >>                    crtc_state->mode_changed = true;
> > I'd drop this one so that connectors_changed and mode_changed are truly
> > orthogonal. Also ->enable implies connectors changed since we do check
> > that there's only connected connectors if the crtc is on. Needs kerneldoc
> > update too ofc.
> 
> They are orthogonal, think of this case:
> 
> 1. crtc previously enabled, connector removed, mode stays same ->
>       connector_changed = true, mode_changed = false
> 
> 2. crtc previously enabled, connectors stay the same, different mode -> 
>       connectors_changed = false, mode_changed = true
> 
> The following is enforced by the checks:
> crtc disabled, implies 0 connectors, no mode.
> crtc enabled implies > 0 connectors, and a mode.
> 
> Hence the connectors_changed and mode_changed here are for documentation 
> purposes only. :)
> 
> So if someone wonders what happens when enable is changed they don't have to 
> dig through
> the entire drm_atomic_helper_check_modeset function and still not be sure if 
> it's coincidence
> or not. 
> 
> You're right about the kerneldoc, I'll fix it.

Right, I guess I should have read your comment properly and disregarded
the kerneldoc ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to