On Tue, Apr 11, 2017 at 01:45:01PM -0700, Eric Anholt wrote: > Yannick Fertre <yannick.fertre@xxxxxx> writes: > > +static void ltdc_crtc_disable(struct drm_crtc *crtc) > > +{ > > + struct ltdc_device *ldev = crtc_to_ltdc(crtc); > > + struct drm_pending_vblank_event *event = crtc->state->event; > > + > > + DRM_DEBUG_DRIVER("\n"); > > + > > + if (!crtc->enabled) { > > + DRM_DEBUG_DRIVER("already disabled\n"); > > + return; > > + } > > I think this crtc->enabled is a given for the disable() being called. Yup, one design principle of atomic (compared to the legacy modeset helpers) is to correctly keep track of hw state and not call a hook when not needed. If you don't trust them, conver them to WARN_ON, but otherwise best to remove. But like Eric said, totally fine in a follow-up patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html