Re: [Intel-gfx] [PATCH 3/3] drm/atomic: Make private objs proper objects

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 11, 2022 at 10:34:34AM +0200, Jani Nikula wrote:
> On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Dec 31, 2021 at 03:23:31PM +0200, Jani Nikula wrote:
> >> On Wed, 12 Jul 2017, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Make the atomic private object stuff less special by introducing proper
> >> > base classes for the object and its state. Drivers can embed these in
> >> > their own appropriate objects, after which these things will work
> >> > exactly like the plane/crtc/connector states during atomic operations.
> >> >
> >> > v2: Reorder to not depend on drm_dynarray (Daniel)
> >> >
> >> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v1
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> 
> >> Stumbled upon an old commit
> >> 
> >> commit a4370c777406c2810e37fafd166ccddecdb2a60c
> >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Date:   Wed Jul 12 18:51:02 2017 +0300
> >> 
> >>     drm/atomic: Make private objs proper objects
> >> 
> >> which is this patch.
> >> 
> >> > @@ -3050,8 +3043,7 @@ struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_a
> >> >  	struct drm_device *dev = mgr->dev;
> >> >  
> >> >  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >> > -	return drm_atomic_get_private_obj_state(state, mgr,
> >> > -						&mst_state_funcs);
> >> > +	return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base));
> >> >  }
> >> >  EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
> >> 
> >> I don't think this combines well with...
> >> 
> >> > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> >> > index 177ab6f86855..d55abb75f29a 100644
> >> > --- a/include/drm/drm_dp_mst_helper.h
> >> > +++ b/include/drm/drm_dp_mst_helper.h
> >> > @@ -404,12 +404,17 @@ struct drm_dp_payload {
> >> >  	int vcpi;
> >> >  };
> >> >  
> >> > +#define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base)
> >> 
> >> ...this in case of error pointers that
> >> drm_atomic_get_private_obj_state() may return.
> >
> > offsetof(base)==0 so should work in practice.
> 
> Returning zeros is fine, but error pointers are another matter.

Why? This is just 'return ptr-0' so shouldn't matter what you
have in that pointer AFAICS.

-- 
Ville Syrjälä
Intel