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 <[email protected]>
---
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
On Tue, 13 Mar 2018, Salvatore Mesoraca <[email protected]> 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
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 <[email protected]>
> ---
> 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
2018-03-14 13:17 GMT+01:00 Jani Nikula <[email protected]>:
> 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
2018-03-14 13:27 GMT+01:00 Joonas Lahtinen <[email protected]>:
> 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