| Message ID | 20200306200636.52051-1-andi@etezian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/gt: allow setting generic data pointer | expand |
On 3/6/20 12:06 PM, Andi Shyti wrote: > From: Andi Shyti <andi.shyti@intel.com> > > When registering debugfs files the intel gt debugfs library > forces a 'struct *gt' private data on the caller. > > There might be different needs, therefore make it generic by > adding one more argument to the "debugfs_register_files()" > function which gets the generic void private data as argument. > > Still keep it simple by defining a wrapper where struct *gt is > the chosen private data to be stored. > > I take the chance to rename the functions by using "intel_gt_" as > prefix instead of "debugfs_". > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > --- > Changelog: > v2: > - the eval function is made generic as suggested by Daniele. > > drivers/gpu/drm/i915/gt/debugfs_engines.c | 2 +- > drivers/gpu/drm/i915/gt/debugfs_gt.c | 11 +++++------ > drivers/gpu/drm/i915/gt/debugfs_gt.h | 12 +++++++----- > drivers/gpu/drm/i915/gt/debugfs_gt_pm.c | 14 +++++++++----- > 4 files changed, 22 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/debugfs_engines.c b/drivers/gpu/drm/i915/gt/debugfs_engines.c > index 6a5e9ab20b94..3434df10d58c 100644 > --- a/drivers/gpu/drm/i915/gt/debugfs_engines.c > +++ b/drivers/gpu/drm/i915/gt/debugfs_engines.c > @@ -32,5 +32,5 @@ void debugfs_engines_register(struct intel_gt *gt, struct dentry *root) > { "engines", &engines_fops }, > }; > > - debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files)); > + intel_gt_debugfs_register_file(gt, root, files, ARRAY_SIZE(files)); > } > diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c > index 75255aaacaed..24099fb157be 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, The gt variable is now unused in this function, so you can get rid of it, which means you can also drop the macro wrapper you have defined below. > + 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); > > files++; > diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.h b/drivers/gpu/drm/i915/gt/debugfs_gt.h > index 4ea0f06cda8f..f498fe75c79a 100644 > --- a/drivers/gpu/drm/i915/gt/debugfs_gt.h > +++ b/drivers/gpu/drm/i915/gt/debugfs_gt.h > @@ -28,12 +28,14 @@ void debugfs_gt_register(struct intel_gt *gt); > struct debugfs_gt_file { > const char *name; > const struct file_operations *fops; > - bool (*eval)(const struct intel_gt *gt); > + bool (*eval)(void *data); > }; > > -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); > + > +#define intel_gt_debugfs_register_file(g, r, f, c) \ > + __intel_gt_debugfs_register_files(g, r, f, c, g) > > #endif /* DEBUGFS_GT_H */ > diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c > index 059c9e5c002e..a233b97a9294 100644 > --- a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c > +++ b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c > @@ -506,9 +506,11 @@ static int llc_show(struct seq_file *m, void *data) > return 0; > } > > -static bool llc_eval(const struct intel_gt *gt) > +static bool llc_eval(void *data) > { > - return HAS_LLC(gt->i915); > + struct intel_gt *gt = data; > + > + return !gt ? false : HAS_LLC(gt->i915); Is there a case where gt can be NULL? BTW, you can also have this condition as: return gt && HAS_LLC(gt->i915); Daniele > } > > DEFINE_GT_DEBUGFS_ATTRIBUTE(llc); > @@ -580,9 +582,11 @@ static int rps_boost_show(struct seq_file *m, void *data) > return 0; > } > > -static bool rps_eval(const struct intel_gt *gt) > +static bool rps_eval(void *data) > { > - return HAS_RPS(gt->i915); > + struct intel_gt *gt = data; > + > + return !gt ? false : HAS_RPS(gt->i915); > } > > DEFINE_GT_DEBUGFS_ATTRIBUTE(rps_boost); > @@ -597,5 +601,5 @@ void debugfs_gt_pm_register(struct intel_gt *gt, struct dentry *root) > { "rps_boost", &rps_boost_fops, rps_eval }, > }; > > - debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files)); > + intel_gt_debugfs_register_file(gt, root, files, ARRAY_SIZE(files)); > } >
Hi Daniele, > > diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c > > index 75255aaacaed..24099fb157be 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, > > The gt variable is now unused in this function, so you can get rid of it, > which means you can also drop the macro wrapper you have defined below. uh, yes, right! I forgot to remove it :) > > -static bool llc_eval(const struct intel_gt *gt) > > +static bool llc_eval(void *data) > > { > > - return HAS_LLC(gt->i915); > > + struct intel_gt *gt = data; > > + > > + return !gt ? false : HAS_LLC(gt->i915); > > Is there a case where gt can be NULL? > BTW, you can also have this condition as: > > return gt && HAS_LLC(gt->i915); Thanks, Andi
diff --git a/drivers/gpu/drm/i915/gt/debugfs_engines.c b/drivers/gpu/drm/i915/gt/debugfs_engines.c index 6a5e9ab20b94..3434df10d58c 100644 --- a/drivers/gpu/drm/i915/gt/debugfs_engines.c +++ b/drivers/gpu/drm/i915/gt/debugfs_engines.c @@ -32,5 +32,5 @@ void debugfs_engines_register(struct intel_gt *gt, struct dentry *root) { "engines", &engines_fops }, }; - debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files)); + intel_gt_debugfs_register_file(gt, root, files, ARRAY_SIZE(files)); } diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c index 75255aaacaed..24099fb157be 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)) + if (!files->eval || files->eval(data)) debugfs_create_file(files->name, - 0444, root, gt, + 0444, root, data, files->fops); files++; diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.h b/drivers/gpu/drm/i915/gt/debugfs_gt.h index 4ea0f06cda8f..f498fe75c79a 100644 --- a/drivers/gpu/drm/i915/gt/debugfs_gt.h +++ b/drivers/gpu/drm/i915/gt/debugfs_gt.h @@ -28,12 +28,14 @@ void debugfs_gt_register(struct intel_gt *gt); struct debugfs_gt_file { const char *name; const struct file_operations *fops; - bool (*eval)(const struct intel_gt *gt); + bool (*eval)(void *data); }; -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); + +#define intel_gt_debugfs_register_file(g, r, f, c) \ + __intel_gt_debugfs_register_files(g, r, f, c, g) #endif /* DEBUGFS_GT_H */ diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c index 059c9e5c002e..a233b97a9294 100644 --- a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c @@ -506,9 +506,11 @@ static int llc_show(struct seq_file *m, void *data) return 0; } -static bool llc_eval(const struct intel_gt *gt) +static bool llc_eval(void *data) { - return HAS_LLC(gt->i915); + struct intel_gt *gt = data; + + return !gt ? false : HAS_LLC(gt->i915); } DEFINE_GT_DEBUGFS_ATTRIBUTE(llc); @@ -580,9 +582,11 @@ static int rps_boost_show(struct seq_file *m, void *data) return 0; } -static bool rps_eval(const struct intel_gt *gt) +static bool rps_eval(void *data) { - return HAS_RPS(gt->i915); + struct intel_gt *gt = data; + + return !gt ? false : HAS_RPS(gt->i915); } DEFINE_GT_DEBUGFS_ATTRIBUTE(rps_boost); @@ -597,5 +601,5 @@ void debugfs_gt_pm_register(struct intel_gt *gt, struct dentry *root) { "rps_boost", &rps_boost_fops, rps_eval }, }; - debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files)); + intel_gt_debugfs_register_file(gt, root, files, ARRAY_SIZE(files)); }