Re: [Intel-gfx] [PATCH] drm/i915/gt: allow setting generic data pointer

[Andi Shyti <andi@etezian.org>]

Hi Daniele,

> > > > diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c
> > > > index 75255aaacaed..9112a8585caf 100644
> > > > --- a/drivers/gpu/drm/i915/gt/debugfs_gt.c
> > > > +++ b/drivers/gpu/drm/i915/gt/debugfs_gt.c
> > > > @@ -26,15 +26,14 @@ void debugfs_gt_register(struct intel_gt *gt)
> > > >    	debugfs_gt_pm_register(gt, root);
> > > >    }
> > > > -void debugfs_gt_register_files(struct intel_gt *gt,
> > > > -			       struct dentry *root,
> > > > -			       const struct debugfs_gt_file *files,
> > > > -			       unsigned long count)
> > > > +void __intel_gt_debugfs_register_files(struct intel_gt *gt, struct dentry *root,
> > > > +				       const struct debugfs_gt_file *files,
> > > > +				       unsigned long count, void *data)
> > > >    {
> > > >    	while (count--) {
> > > >    		if (!files->eval || files->eval(gt))
> > > 
> > > IMO the files->eval() function should also use the provided data instead of
> > > intel_gt. This will also allow us to drop the intel_gt parameter in this
> > > function, which in turn means we can use this function directly from all the
> > > levels.
> > 
> > do you mean something like this:
> > 
> > -       bool (*eval)(const struct intel_gt *gt);
> > +       bool (*eval)(void *data);
> > 
> > ?
> 
> yes
> 
> > 
> > I am missing the use case, though, what is it that cannot be
> > reached by the gt so that it needs to be more generic?
> 
> It's not a problem of reaching it from gt but the other way around, I don't
> want the caller to have to retrieve a gt variable it don't needs just to
> pass it to this function and then go back to the actual required data from
> gt inside of the eval function. Anything you need for your evaluation should
> be reachable from the struct used as data for the debugfs.
> To make a concrete example, I want to avoid an unneeded guc_to_gt inside
> intel_guc_debugfs_register that would also require a matched guc =
> &gt->uc.guc inside the eval function, passing guc (i.e. the data) straight
> in the eval is cleaner IMO.

Thanks for the feedback, makes sense.

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx