Re: [PATCH 1/7] drm: add atomic fxns

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 23, 2014 at 11:34:00PM +0200, Daniel Vetter wrote:
> On Wed, Jul 23, 2014 at 03:38:14PM -0400, Rob Clark wrote:
> > The 'atomic' mechanism allows for multiple properties to be updated,
> > checked, and commited atomically.  This will be the basis of atomic-
> > modeset and nuclear-pageflip.
> > 
> > The basic flow is:
> > 
> >    state = dev->atomic_begin();
> >    for (... one or more ...)
> >       obj->set_property(obj, state, prop, value);
> >    if (dev->atomic_check(state))
> >       dev->atomic_commit(state);
> >    dev->atomic_end(state);
> > 
> > The split of check and commit steps is to allow for ioctls with a
> > test-only flag (which would skip the commit step).
> > 
> > Signed-off-by: Rob Clark <robdclark@gmail.com>
> 
> [snip]
> > +	if (flags & DRM_MODE_ATOMIC_NOLOCK)
> > +		acquire_flags |= DRM_MODESET_ACQUIRE_NOLOCK;
> > +	if (flags & DRM_MODE_ATOMIC_NONBLOCK)
> > +		acquire_flags |= DRM_MODESET_ACQUIRE_NONBLOCK;
> 
> Just a very quick reply. Can you please remove the code which does the
> NOLOCK/NONBLOCK stuff here? It's not really part of the property
> conversion and I'm still not sold on those concepts.
> 
> At least I want to review them once we add them, maybe at the very end
> where everything else is clear. Afaics nothing in this series actually
> uses this.

Self-correct: NOLOCK is used for the fbdev restore code. Imo we should
drop the modeset_lock_all this one uses and switch to using the atomic
locking scheme like everywhere. That way fbdev modeset changes work
exactly like any other modeset change.

fbdev state itself can be protected with dev->mode_config.mutex, which
wraps around all the ww_mutex dancing.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch