[PATCH] drm/atomic: Set current atomic state in drm_private_state
Alexandru-Cosmin Gheorghe
Alexandru-Cosmin.Gheorghe at arm.com
Thu May 31 09:56:14 UTC 2018
Hi,
Thank you for the review.
On Wed, May 30, 2018 at 11:22:27PM +0300, Ville Syrjälä wrote:
> On Wed, May 30, 2018 at 06:30:52PM +0100, Alexandru Gheorghe wrote:
> > drm_private_state has a back pointer to the drm_atomic_state,
> > however that was not initialized in drm_atomic_get_private_obj_state
> > after duplication, as it is the case for other drm atomic getters
> >
> > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42..249aaf8 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1108,6 +1108,7 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
> > state->private_objs[index].old_state = obj->state;
> > state->private_objs[index].new_state = obj_state;
> > state->private_objs[index].ptr = obj;
> > + obj_state->state = state;
>
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> I guess no one ever used that pointer. Should we add some WARNs
> to drm_atomic_helper_swap_state() to make sure these are correct?
That would've caught this, but I'm not sure if it's not to late for a
warning, we just got lucky nobody ever tried to access that pointer yet.
>
> I think in general life might be nicer if we didn't even have these
> pointers at all. As it stands it's pretty easy to accidentally
> use them when you're not supposed to (eg. after swap_state() try
> to use the new_state->state). But there's tons of code that would
> need to be touched to eliminate them so not a very pleasant project.
I agree easy to accidentally use them after swap, but I suppose is to
late to eliminate them or if we should.
>
> >
> > state->num_private_objs = num_objs;
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
--
Cheers,
Alex G
More information about the dri-devel
mailing list