[PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Salvatore Mesoraca]

Avoid 3 VLAs[1] by using real constant expressions instead of variables.
The compiler should be able to optimize the original code and avoid using
any actual VLAs. Anyway this change is useful because it will avoid a false
positives with -Wvla, it might also help the compiler generating better
code.

[1] https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e968aea..bf0a8e3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4259,19 +4259,20 @@ static ssize_t cur_wm_latency_write(struct file *file, const char __user *ubuf,
 			i915_cache_sharing_get, i915_cache_sharing_set,
 			"%llu\n");
 
+#define CHERRYVIEW_SS_MAX 2
+
 static void cherryview_sseu_device_status(struct drm_i915_private *dev_priv,
 					  struct sseu_dev_info *sseu)
 {
-	int ss_max = 2;
 	int ss;
-	u32 sig1[ss_max], sig2[ss_max];
+	u32 sig1[CHERRYVIEW_SS_MAX], sig2[CHERRYVIEW_SS_MAX];
 
 	sig1[0] = I915_READ(CHV_POWER_SS0_SIG1);
 	sig1[1] = I915_READ(CHV_POWER_SS1_SIG1);
 	sig2[0] = I915_READ(CHV_POWER_SS0_SIG2);
 	sig2[1] = I915_READ(CHV_POWER_SS1_SIG2);
 
-	for (ss = 0; ss < ss_max; ss++) {
+	for (ss = 0; ss < CHERRYVIEW_SS_MAX; ss++) {
 		unsigned int eu_cnt;
 
 		if (sig1[ss] & CHV_SS_PG_ENABLE)
@@ -4290,15 +4291,17 @@ static void cherryview_sseu_device_status(struct drm_i915_private *dev_priv,
 	}
 }
 
+#define GEN10_S_MAX 6
+#define GEN10_SS_MAX 4
+
 static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
 				     struct sseu_dev_info *sseu)
 {
 	const struct intel_device_info *info = INTEL_INFO(dev_priv);
-	int s_max = 6, ss_max = 4;
 	int s, ss;
-	u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
+	u32 s_reg[GEN10_S_MAX], eu_reg[2 * GEN10_S_MAX], eu_mask[2];
 
-	for (s = 0; s < s_max; s++) {
+	for (s = 0; s < GEN10_S_MAX; s++) {
 		/*
 		 * FIXME: Valid SS Mask respects the spec and read
 		 * only valid bits for those registers, excluding reserverd
@@ -4320,7 +4323,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
 		     GEN9_PGCTL_SSB_EU210_ACK |
 		     GEN9_PGCTL_SSB_EU311_ACK;
 
-	for (s = 0; s < s_max; s++) {
+	for (s = 0; s < GEN10_S_MAX; s++) {
 		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
 			/* skip disabled slice */
 			continue;
@@ -4328,7 +4331,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
 		sseu->slice_mask |= BIT(s);
 		sseu->subslice_mask = info->sseu.subslice_mask;
 
-		for (ss = 0; ss < ss_max; ss++) {
+		for (ss = 0; ss < GEN10_SS_MAX; ss++) {
 			unsigned int eu_cnt;
 
 			if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
@@ -4345,12 +4348,15 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
 	}
 }
 
+#define GEN9_S_MAX 3
+#define GEN9_SS_MAX 4
+
 static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 				    struct sseu_dev_info *sseu)
 {
-	int s_max = 3, ss_max = 4;
+	int s_max = GEN9_S_MAX, ss_max = GEN9_SS_MAX;
 	int s, ss;
-	u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
+	u32 s_reg[GEN9_S_MAX], eu_reg[2*GEN9_S_MAX], eu_mask[2];
 
 	/* BXT has a single slice and at most 3 subslices. */
 	if (IS_GEN9_LP(dev_priv)) {
-- 
1.9.1

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Jani Nikula]

On Tue, 13 Mar 2018, Salvatore Mesoraca <s.mesoraca16@gmail.com> wrote:
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.

Thanks for your patch. However, Chris beat you to it with:

7aa0b14ede64 ("drm/i915: Remove variable length arrays from sseu debugfs
printers")

as well as adding -Wvla to our subdir-ccflags-y to prevent more from
cropping up:

c5c2b11894f4 ("drm/i915: Warn against variable length arrays")


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Jani Nikula]

On Tue, 13 Mar 2018, Salvatore Mesoraca <s.mesoraca16@gmail.com> wrote:
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.

Thanks for your patch. However, Chris beat you to it with:

7aa0b14ede64 ("drm/i915: Remove variable length arrays from sseu debugfs
printers")

as well as adding -Wvla to our subdir-ccflags-y to prevent more from
cropping up:

c5c2b11894f4 ("drm/i915: Warn against variable length arrays")


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Jani Nikula]

On Tue, 13 Mar 2018, Salvatore Mesoraca <s.mesoraca16@gmail.com> wrote:
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.

Thanks for your patch. However, Chris beat you to it with:

7aa0b14ede64 ("drm/i915: Remove variable length arrays from sseu debugfs
printers")

as well as adding -Wvla to our subdir-ccflags-y to prevent more from
cropping up:

c5c2b11894f4 ("drm/i915: Warn against variable length arrays")


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Joonas Lahtinen]

Quoting Salvatore Mesoraca (2018-03-13 21:51:28)
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.
> 
> [1] https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e968aea..bf0a8e3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4259,19 +4259,20 @@ static ssize_t cur_wm_latency_write(struct file *file, const char __user *ubuf,
>                         i915_cache_sharing_get, i915_cache_sharing_set,
>                         "%llu\n");
>  
> +#define CHERRYVIEW_SS_MAX 2

CHV_SS_MAX should be good enough. Make these function scoped (so #define
at the beginning and #undef at the end of function).

Do use ARRAY_SIZE() instead of repeating.

Regards, Joonas

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Joonas Lahtinen]

Quoting Salvatore Mesoraca (2018-03-13 21:51:28)
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.
> 
> [1] https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e968aea..bf0a8e3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4259,19 +4259,20 @@ static ssize_t cur_wm_latency_write(struct file *file, const char __user *ubuf,
>                         i915_cache_sharing_get, i915_cache_sharing_set,
>                         "%llu\n");
>  
> +#define CHERRYVIEW_SS_MAX 2

CHV_SS_MAX should be good enough. Make these function scoped (so #define
at the beginning and #undef at the end of function).

Do use ARRAY_SIZE() instead of repeating.

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

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Joonas Lahtinen]

Quoting Salvatore Mesoraca (2018-03-13 21:51:28)
> Avoid 3 VLAs[1] by using real constant expressions instead of variables.
> The compiler should be able to optimize the original code and avoid using
> any actual VLAs. Anyway this change is useful because it will avoid a false
> positives with -Wvla, it might also help the compiler generating better
> code.
> 
> [1] https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e968aea..bf0a8e3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4259,19 +4259,20 @@ static ssize_t cur_wm_latency_write(struct file *file, const char __user *ubuf,
>                         i915_cache_sharing_get, i915_cache_sharing_set,
>                         "%llu\n");
>  
> +#define CHERRYVIEW_SS_MAX 2

CHV_SS_MAX should be good enough. Make these function scoped (so #define
at the beginning and #undef at the end of function).

Do use ARRAY_SIZE() instead of repeating.

Regards, Joonas

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Salvatore Mesoraca]

2018-03-14 13:17 GMT+01:00 Jani Nikula <jani.nikula@linux.intel.com>:
> Thanks for your patch. However, Chris beat you to it with:
>
> 7aa0b14ede64 ("drm/i915: Remove variable length arrays from sseu debugfs
> printers")

I didn't notice it :)

> as well as adding -Wvla to our subdir-ccflags-y to prevent more from
> cropping up:
>
> c5c2b11894f4 ("drm/i915: Warn against variable length arrays")

Great!
Best regards,

Salvatore

Re: [PATCH] drm/i915: drop various VLAs in i915_debugfs.c

[Salvatore Mesoraca]

2018-03-14 13:27 GMT+01:00 Joonas Lahtinen <joonas.lahtinen@linux.intel.com>:
> CHV_SS_MAX should be good enough. Make these function scoped (so #define
> at the beginning and #undef at the end of function).
>
> Do use ARRAY_SIZE() instead of repeating.

Thank you very much for your suggestions.
Unfortunately, it seems that someone else already fixed this issue, so
I'll just drop this patch.
Best regards,

Salvatore

✗ Fi.CI.BAT: failure for drm/i915: drop various VLAs in i915_debugfs.c

[Patchwork]

== Series Details ==

Series: drm/i915: drop various VLAs in i915_debugfs.c
URL   : https://patchwork.freedesktop.org/series/39985/
State : failure

== Summary ==

Applying: drm/i915: drop various VLAs in i915_debugfs.c
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_debugfs.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_debugfs.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_debugfs.c
Patch failed at 0001 drm/i915: drop various VLAs in i915_debugfs.c
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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