On Tue, Nov 25, 2014 at 12:45:46PM +0100, Thierry Reding wrote:
> On Tue, Nov 25, 2014 at 12:22:34PM +0100, Daniel Vetter wrote:
> > On Tue, Nov 25, 2014 at 12:09:46PM +0100, Thierry Reding wrote:
[...]
> > > +/*
> > > + * drm_plane_disabled - check whether a plane is being disabled
> > > + * @plane: plane object
> > > + * @old_state: previous atomic state
> > > + *
> > > + * Checks the atomic state of a plane to determine whether it's being 
> > > disabled
> > > + * or not. This also WARNs if it detects an invalid state (both CRTC and 
> > > FB
> > > + * need to either both be NULL or both be non-NULL).
> > > + *
> > > + * RETURNS:
> > > + * True if the plane is being disabled, false otherwise.
> > > + */
> > > +static inline bool drm_plane_disabled(struct drm_plane *plane,
> > > +                               struct drm_plane_state *old_state)
> > > +{
[...]
> > > + return (!old_state || old_state->crtc) && !plane->state->crtc;
> > 
> > The !old_state check here confused me a bit, until I've realized that you
> > use this for transitional helpers too. What about adding
> > 
> >     /* Cope with NULL old_state for transitional helpers. */
> > 
> > right above?
> 
> Sounds good.

When I now thought about the reason again it took me a while to
reconstruct, so I figured I'd be extra verbose and used this:

        /*
         * When using the transitional helpers, old_state may be NULL. If so,
         * we know nothing about the current state and have to assume that it
         * might be enabled.
         */

Does that sound accurate and sufficient to you?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141125/bbd487ee/attachment.sig>

Reply via email to