Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v2] drm/i915/gt: allow setting generic data pointer
@ 2020-03-06 20:06 Andi Shyti
  2020-03-06 20:51 ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 3+ messages in thread
From: Andi Shyti @ 2020-03-06 20:06 UTC (permalink / raw)
  To: Intel GFX

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,
+				       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));
 }
-- 
2.25.0

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: allow setting generic data pointer
  2020-03-06 20:06 [Intel-gfx] [PATCH v2] drm/i915/gt: allow setting generic data pointer Andi Shyti
@ 2020-03-06 20:51 ` Daniele Ceraolo Spurio
  2020-03-06 21:51   ` Andi Shyti
  0 siblings, 1 reply; 3+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-06 20:51 UTC (permalink / raw)
  To: Andi Shyti, Intel GFX



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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: allow setting generic data pointer
  2020-03-06 20:51 ` Daniele Ceraolo Spurio
@ 2020-03-06 21:51   ` Andi Shyti
  0 siblings, 0 replies; 3+ messages in thread
From: Andi Shyti @ 2020-03-06 21:51 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: Intel GFX

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-06 21:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-06 20:06 [Intel-gfx] [PATCH v2] drm/i915/gt: allow setting generic data pointer Andi Shyti
2020-03-06 20:51 ` Daniele Ceraolo Spurio
2020-03-06 21:51   ` Andi Shyti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox