Re: [PATCH v2 01/20] drm/atomic: add connectors_changed to separate it from mode_changed

Daniel Vetter
2015-07-07T12:03:39Z

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org