On Mon, Nov 28, 2016 at 02:44:15PM +0000, Chris Wilson wrote:
> On Mon, Nov 28, 2016 at 03:15:12PM +0100, Daniel Vetter wrote:
> > On Mon, Nov 28, 2016 at 08:46:11AM +0000, Chris Wilson wrote:
> > > On Mon, Nov 28, 2016 at 08:48:34AM +0100, Daniel Vetter wrote:
> > > > On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > @@ -1253,7 +1253,7 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
> > > > > DRM_DEBUG_ATOMIC("Set [NOFB] for plane state %p\n",
> > > > > plane_state);
> > > > >
> > > > > - drm_framebuffer_assign(&plane_state->fb, fb);
> > > > > + drm_framebuffer_assign(__mkwrite_drm_framebuffer(&plane_state->fb), fb);
> > > >
> > > > Feels like const gone the wrong way round? Or I'm not entirely clear on
> > > > what this is supposed to achieve. Just dropping the const would still be a
> > > > nice improvement.
> > >
> > > No one is supposed to be writing to it. Adding the const generates a
> > > compiler warning for incorrect code that doesn't do the refcounting -
> > > but doesn't generate a warning for anything simply checking the value.
> > > It is the moral equivalent to calling it _fb to catch all the users.
> >
> > But then shouldn't all users use the drm_plane_set_fb helper (and that
> > maybe gain a comment)? The above escaped the conversion/consolidation,
> > which is why it looked funny to me ...
>
> Yes, all users (of plane->fb = fb) are. The above is for
> plane_state->fb.
Ah, missed that ;-)
> I wasn't sure about adding comments to struct drm_plane, I thought the
> crtc/fb there was just a crutch for !atomic.
>
> But adding
> +
> + /**
> + * @fb:
> + *
> + * Currently active framebuffer. Do not write this directly, use
> + * drm_plane_set_fb()
> + */
> struct drm_framebuffer * const fb;
>
> is not a hardship.
Yeah, that'd be good. We already have a similar comment for
drm_plane_state.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
