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

[Andi Shyti <andi@etezian.org>]

Hi Chris,

> > > Quoting Andi Shyti (2020-03-06 23:03:44)
> > > > -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 dentry *root,
> > > > +                                    const struct debugfs_gt_file *files,
> > > > +                                    unsigned long count, void *data)
> > > >  {
> > > >         while (count--) {
> > > > -               if (!files->eval || files->eval(gt))
> > > > +               if (!files->eval || files->eval(data))
> > > >                         debugfs_create_file(files->name,
> > > > -                                           0444, root, gt,
> > > > +                                           0444, root, data,
> > > >                                             files->fops);
> > > >  
> > > 
> > > And now we are not a intel_gt routine, you'll want to move again :)
> > > i915_debugfs_utils.c ? :)
> > 
> > Actually, this is what it came to and this was the first
> > discussion I had with Daniele and that's also why I was loyal to
> > th "_gt_" wrappers until the end. But I had to agree that this
> > was becoming more a limitation.
> > 
> > The biggest difference left, which by the way is the real
> > distinguishing factor other than the *gt pointer, is that we
> > create files under gt directory, instead of having the root
> > imposed by the drm (even though the caller can eventually choose
> > different roots).
> > 
> > We could perhaps store the root pointer in the intel_gt
> > structure so that this function stays de facto an intel_gt
> > routine and the caller doesn't need to care where the files will
> > be generated. This is what we planned to do with sysfs as well.
> > 
> > What do you think?
> 
> I thought we were passing along the root. If not I think we should, more
> of a debugfs constructor context?

What do you mean with debugfs constructor context? Is it a
gt->debugfs_root pointer like the gt->sysfs_root?

> The main thing of course is not to overengineer and do the minimal
> necessary for the immediate users we have. We can always extend and
> refactor for a third user, etc, etc.
> 
> So if this works for gt + children, go for it and worry about tomorrow,
> tomorrow. Trusting our good practice for "a stitch in time saves nine".

this came after Daniele's guc patches where he preferred to
define his own functions instead of using this one that is meant
to be used in that situation.

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