On Wed, Jan 25, 2017 at 08:47:09PM +0000, Pandiyan, Dhinakaran wrote:
> On Wed, 2017-01-25 at 06:59 +0100, Daniel Vetter wrote:
> > On Tue, Jan 24, 2017 at 03:49:32PM -0800, Dhinakaran Pandiyan wrote:
> > > +#define for_each_private_obj(__state, obj, obj_state, __i, __funcs)      
> > > \
> > > + for ((__i) = 0;                                                 \
> > > +      (__i) < (__state)->num_private_objs &&                     \
> > > +      ((obj) = (__state)->private_objs[__i].obj,                 \
> > > +       (__funcs) = (__state)->private_objs[__i].funcs,           \
> > > +       (obj_state) = (__state)->private_objs[__i].obj_state, 1); \
> > > +       (__i)++)                                                  \
> > > +         for_each_if (__funcs)
> > 
> > You are not filtering for the function table here, which is important to
> > make sure that this can be used to only walk over objects with a given
> > vtable. With that we can then #define specific macros for e.g. mst:
> > 
> > struct drm_private_state_funcs mst_state_funcs;
> > 
> > #define for_each_mst_state(__state, obj, obj_state, __i, __funcs) \
> >     for_each_private_obj(__state, &mst_state_funcs, obj, obj_state, __i, 
> > __funcs)
> > 
> > I'd place the vfunc right after the state, since those are both input
> > parameters to the macro, and specify what exactly we're looping over. To
> > make this work you need something like:
> > 
> > #define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, 
> > __funcs)      \
> >     for ((__i) = 0;                                                 \
> >          (__i) < (__state)->num_private_objs &&                     \
> >          ((obj) = (__state)->private_objs[__i].obj,                 \
> >           (__funcs) = (__state)->private_objs[__i].funcs,           \
> >           (obj_state) = (__state)->private_objs[__i].obj_state, 1); \
> >           (__i)++)                                                  \
> >             for_each_if (__funcs == obj_funcs)
> > 
> > Note the check to compare __funcs == obj_funcs.
> > 
> > With that other subsytem can the filter for their own objects only with
> > e.g.
> > 
> > #define intel_for_each_cdclk_state(__state, obj, obj_state, __i, __funcs) \
> >     for_each_private_obj(__state, &intel_cdclk_state_funcs, obj, obj_state, 
> > __i, __funcs)
> > 
> > Would be good to also then have kerneldoc for this iterator, to explain
> > how to use it.
> > -Daniel
> > 
> 
> I see your point but we can't use this iterator in the swap_state()
> helper if we do that. I have used it to swap states for all objects
> using this version without filtering.
> 
> I guess, I can just code the iterator explicitly for swap_state() and
> re-write the iterator with the filtering.

For swap states I'd use a raw iterator with a __ prefix, which does not
have the vtable check. So yes, you need two I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to