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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
