[PATCH] drm/i915: Reword warning for missing cases

[PATCH] drm/i915: Reword warning for missing cases

[Lucas De Marchi]

In some places we end up converting switch statements to a series of
if/else, particularly when introducing helper functions to handle a
group of cases. It's tempting to either leave a wrong warning (since now
we don't have a switch case anymore) or to convert to WARN(1, ...),
losing what MISSING_CASE() provides: source location and id number.

Fix the message to allow reusing MISSING_CASE() - it may not always be
correct (e.g. if you are not checking an id anymore), but it avoids
useless conversions. A quick grep reveals at least a few users in
drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 51dbfe5bb418..8cdc21b92f5f 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -40,7 +40,7 @@
 #undef WARN_ON_ONCE
 #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
 
-#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
+#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
 			     (long)(x), __func__)
 
 #if GCC_VERSION >= 70000
-- 
2.14.3

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

Re: [PATCH] drm/i915: Reword warning for missing cases

[Chris Wilson]

Quoting Lucas De Marchi (2018-03-13 00:03:12)
> In some places we end up converting switch statements to a series of
> if/else, particularly when introducing helper functions to handle a
> group of cases. It's tempting to either leave a wrong warning (since now
> we don't have a switch case anymore) or to convert to WARN(1, ...),
> losing what MISSING_CASE() provides: source location and id number.
> 
> Fix the message to allow reusing MISSING_CASE() - it may not always be
> correct (e.g. if you are not checking an id anymore), but it avoids
> useless conversions. A quick grep reveals at least a few users in
> drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_utils.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> index 51dbfe5bb418..8cdc21b92f5f 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -40,7 +40,7 @@
>  #undef WARN_ON_ONCE
>  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
>  
> -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
>                              (long)(x), __func__)

Whilst here you could make this more informative by:
"Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__

Also these should only be compiled for dev/CI as we have a few pages
worth of them.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Reword warning for missing cases

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases
URL   : https://patchwork.freedesktop.org/series/39821/
State : success

== Summary ==

Series 39821v1 drm/i915: Reword warning for missing cases
https://patchwork.freedesktop.org/api/1.0/series/39821/revisions/1/mbox/

---- Known issues:

Test kms_chamelium:
        Subgroup dp-edid-read:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102505
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:387s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:530s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:301s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:507s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:510s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:498s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:412s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:582s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:427s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:467s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:429s
fi-kbl-7500u     total:288  pass:262  dwarn:1   dfail:0   fail:1   skip:24  time:475s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:644s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:530s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:542s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:431s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:540s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:397s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:509s
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:28  time:520s
fi-glk-j5005 failed to collect. IGT log at Patchwork_8317/fi-glk-j5005/run0.log

da600bbe24412edd1c46591fd21ef81518e49c5d drm-tip: 2018y-03m-12d-22h-06m-53s UTC integration manifest
9e9eb710bd13 drm/i915: Reword warning for missing cases

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8317/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Reword warning for missing cases

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases
URL   : https://patchwork.freedesktop.org/series/39821/
State : success

== Summary ==

---- Known issues:

Test gem_eio:
        Subgroup in-flight-contexts:
                incomplete -> PASS       (shard-apl) fdo#105341 +1
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-nonblocking-fencing:
                fail       -> PASS       (shard-apl) fdo#103207
Test kms_cursor_crc:
        Subgroup cursor-256x256-suspend:
                pass       -> SKIP       (shard-hsw) fdo#103375

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#103207 https://bugs.freedesktop.org/show_bug.cgi?id=103207
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-apl        total:3377 pass:1778 dwarn:1   dfail:0   fail:6   skip:1590 time:12646s
shard-hsw        total:3473 pass:1776 dwarn:1   dfail:0   fail:2   skip:1693 time:11818s
shard-snb        total:3473 pass:1364 dwarn:1   dfail:0   fail:3   skip:2105 time:7217s
Blacklisted hosts:
shard-kbl        total:3375 pass:1898 dwarn:1   dfail:0   fail:9   skip:1465 time:9268s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8317/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Reword warning for missing cases

[Ville Syrjälä]

On Tue, Mar 13, 2018 at 12:17:13AM +0000, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-03-13 00:03:12)
> > In some places we end up converting switch statements to a series of
> > if/else, particularly when introducing helper functions to handle a
> > group of cases. It's tempting to either leave a wrong warning (since now
> > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > losing what MISSING_CASE() provides: source location and id number.
> > 
> > Fix the message to allow reusing MISSING_CASE() - it may not always be
> > correct (e.g. if you are not checking an id anymore), but it avoids
> > useless conversions. A quick grep reveals at least a few users in
> > drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> > 
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_utils.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > index 51dbfe5bb418..8cdc21b92f5f 100644
> > --- a/drivers/gpu/drm/i915/i915_utils.h
> > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > @@ -40,7 +40,7 @@
> >  #undef WARN_ON_ONCE
> >  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
> >  
> > -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> > +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
> >                              (long)(x), __func__)
> 
> Whilst here you could make this more informative by:
> "Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__

The backtrace isn't enough?

> 
> Also these should only be compiled for dev/CI as we have a few pages
> worth of them.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Reword warning for missing cases

[Chris Wilson]

Quoting Ville Syrjälä (2018-03-13 13:01:42)
> On Tue, Mar 13, 2018 at 12:17:13AM +0000, Chris Wilson wrote:
> > Quoting Lucas De Marchi (2018-03-13 00:03:12)
> > > In some places we end up converting switch statements to a series of
> > > if/else, particularly when introducing helper functions to handle a
> > > group of cases. It's tempting to either leave a wrong warning (since now
> > > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > > losing what MISSING_CASE() provides: source location and id number.
> > > 
> > > Fix the message to allow reusing MISSING_CASE() - it may not always be
> > > correct (e.g. if you are not checking an id anymore), but it avoids
> > > useless conversions. A quick grep reveals at least a few users in
> > > drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_utils.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > > index 51dbfe5bb418..8cdc21b92f5f 100644
> > > --- a/drivers/gpu/drm/i915/i915_utils.h
> > > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > > @@ -40,7 +40,7 @@
> > >  #undef WARN_ON_ONCE
> > >  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
> > >  
> > > -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> > > +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
> > >                              (long)(x), __func__)
> > 
> > Whilst here you could make this more informative by:
> > "Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__
> 
> The backtrace isn't enough?

Not if we have more than one in a function. (Why would we do that, you
might ask, and I'd answer if the point is to make this more generic and
versatile, then do so. We already have the value, why not then explain
what that value is.) And give me a single sentence identifying the missing
case makes it much more pleasant.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Reword warning for missing cases

[Ville Syrjälä]

On Tue, Mar 13, 2018 at 01:06:49PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-03-13 13:01:42)
> > On Tue, Mar 13, 2018 at 12:17:13AM +0000, Chris Wilson wrote:
> > > Quoting Lucas De Marchi (2018-03-13 00:03:12)
> > > > In some places we end up converting switch statements to a series of
> > > > if/else, particularly when introducing helper functions to handle a
> > > > group of cases. It's tempting to either leave a wrong warning (since now
> > > > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > > > losing what MISSING_CASE() provides: source location and id number.
> > > > 
> > > > Fix the message to allow reusing MISSING_CASE() - it may not always be
> > > > correct (e.g. if you are not checking an id anymore), but it avoids
> > > > useless conversions. A quick grep reveals at least a few users in
> > > > drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> > > > 
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_utils.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > > > index 51dbfe5bb418..8cdc21b92f5f 100644
> > > > --- a/drivers/gpu/drm/i915/i915_utils.h
> > > > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > > > @@ -40,7 +40,7 @@
> > > >  #undef WARN_ON_ONCE
> > > >  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
> > > >  
> > > > -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> > > > +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
> > > >                              (long)(x), __func__)
> > > 
> > > Whilst here you could make this more informative by:
> > > "Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__
> > 
> > The backtrace isn't enough?
> 
> Not if we have more than one in a function.

I was just commenting on the __func__ part actually. That seems pretty
much redundant to me. The stringify part does seem like a decent idea,
and matches our WARN() trickery pretty well.

> (Why would we do that, you
> might ask, and I'd answer if the point is to make this more generic and
> versatile, then do so. We already have the value, why not then explain
> what that value is.) And give me a single sentence identifying the missing
> case makes it much more pleasant.
> -Chris

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Reword warning for missing cases

[Lucas De Marchi]

On Tue, Mar 13, 2018 at 03:23:54PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 13, 2018 at 01:06:49PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-03-13 13:01:42)
> > > On Tue, Mar 13, 2018 at 12:17:13AM +0000, Chris Wilson wrote:
> > > > Quoting Lucas De Marchi (2018-03-13 00:03:12)
> > > > > In some places we end up converting switch statements to a series of
> > > > > if/else, particularly when introducing helper functions to handle a
> > > > > group of cases. It's tempting to either leave a wrong warning (since now
> > > > > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > > > > losing what MISSING_CASE() provides: source location and id number.
> > > > > 
> > > > > Fix the message to allow reusing MISSING_CASE() - it may not always be
> > > > > correct (e.g. if you are not checking an id anymore), but it avoids
> > > > > useless conversions. A quick grep reveals at least a few users in
> > > > > drivers/gpu/drm/i915/intel_csr.c and drivers/gpu/drm/i915/intel_ddi.c.
> > > > > 
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_utils.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > > > > index 51dbfe5bb418..8cdc21b92f5f 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_utils.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_utils.h
> > > > > @@ -40,7 +40,7 @@
> > > > >  #undef WARN_ON_ONCE
> > > > >  #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
> > > > >  
> > > > > -#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
> > > > > +#define MISSING_CASE(x) WARN(1, "Missing case (%lu) in %s\n", \
> > > > >                              (long)(x), __func__)
> > > > 
> > > > Whilst here you could make this more informative by:
> > > > "Missing case (%s = %lu) in %s\n", __stringify(x), (long)(x), __func__
> > > 
> > > The backtrace isn't enough?
> > 
> > Not if we have more than one in a function.
> 
> I was just commenting on the __func__ part actually. That seems pretty
> much redundant to me. The stringify part does seem like a decent idea,
> and matches our WARN() trickery pretty well.

Humn... indeed. On a quick look I thought it was there so we wouldn't need a addr2line.
But we are not even printing the line number, just the function name.
The warning is from the kernel already does that and more.

What about:

#define MISSING_CASE(x) WARN(1, "Missing case (%s == %lu)\n", \
			     __stringify(x), (unsigned long)(x))

I added a simple hack on probe and I get this:

[ 4535.233717] Missing case (ret == 0)
[ 4535.233868] WARNING: CPU: 1 PID: 795 at drivers/gpu/drm/i915/i915_drv.c:1341 i915_driver_load+0x42/0x10e0 [i915]


Lucas De Marchi

> 
> > (Why would we do that, you
> > might ask, and I'd answer if the point is to make this more generic and
> > versatile, then do so. We already have the value, why not then explain
> > what that value is.) And give me a single sentence identifying the missing
> > case makes it much more pleasant.
> > -Chris
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Reword warning for missing cases

[Lucas De Marchi]

In some places we end up converting switch statements to a series of
if/else, particularly when introducing helper functions to handle a
group of cases. It's tempting to either leave a wrong warning (since now
we don't have a switch case anymore) or to convert to WARN(1, ...),
but we can just provide a better message and avoid the doubt when such
conversions arrise.

Introducing a warning inside i915_driver_load() just for tests we get:

[ 4535.233717] Missing case (ret == 0)
[ 4535.233868] WARNING: CPU: 1 PID: 795 at drivers/gpu/drm/i915/i915_drv.c:1341 i915_driver_load+0x42/0x10e0 [i915]

which is clear enough.

v2: remove __func__ since this is already on the warning.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_utils.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 51dbfe5bb418..0695717522ea 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -40,8 +40,8 @@
 #undef WARN_ON_ONCE
 #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
 
-#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
-			     (long)(x), __func__)
+#define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
+			     __stringify(x), (long)(x))
 
 #if GCC_VERSION >= 70000
 #define add_overflows(A, B) \
-- 
2.14.3

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Reword warning for missing cases (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases (rev2)
URL   : https://patchwork.freedesktop.org/series/39821/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
7cb94d2c3c47 drm/i915: Reword warning for missing cases
-:40: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'x' - possible side-effects?
#40: FILE: drivers/gpu/drm/i915/i915_utils.h:43:
+#define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
+			     __stringify(x), (long)(x))

total: 0 errors, 0 warnings, 1 checks, 10 lines checked

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

✗ Fi.CI.BAT: failure for drm/i915: Reword warning for missing cases (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases (rev2)
URL   : https://patchwork.freedesktop.org/series/39821/
State : failure

== Summary ==

Series 39821v2 drm/i915: Reword warning for missing cases
https://patchwork.freedesktop.org/api/1.0/series/39821/revisions/2/mbox/

---- Possible new issues:

Test gem_exec_parallel:
        Subgroup basic:
                pass       -> INCOMPLETE (fi-snb-2600)

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:439s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:379s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:515s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:504s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:406s
fi-cfl-s2        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:510s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:519s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:425s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:316s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:470s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:437s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:473s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:652s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:435s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:534s
fi-skl-6700hq    total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:544s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:517s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:427s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:571s
fi-snb-2600      total:68   pass:57   dwarn:0   dfail:0   fail:0   skip:10 

260af42eeff094e4768265a6ec8bbcb29b87e9a0 drm-tip: 2018y-03m-19d-17h-15m-08s UTC integration manifest
7cb94d2c3c47 drm/i915: Reword warning for missing cases

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8400/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Reword warning for missing cases

[Chris Wilson]

Quoting Lucas De Marchi (2018-03-19 17:37:20)
> In some places we end up converting switch statements to a series of
> if/else, particularly when introducing helper functions to handle a
> group of cases. It's tempting to either leave a wrong warning (since now
> we don't have a switch case anymore) or to convert to WARN(1, ...),
> but we can just provide a better message and avoid the doubt when such
> conversions arrise.
> 
> Introducing a warning inside i915_driver_load() just for tests we get:
> 
> [ 4535.233717] Missing case (ret == 0)
> [ 4535.233868] WARNING: CPU: 1 PID: 795 at drivers/gpu/drm/i915/i915_drv.c:1341 i915_driver_load+0x42/0x10e0 [i915]
> 
> which is clear enough.
> 
> v2: remove __func__ since this is already on the warning.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Reword warning for missing cases (rev2)

[Lucas De Marchi]

CCing checkpatch maintainers

On Mon, Mar 19, 2018 at 06:36:16PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Reword warning for missing cases (rev2)
> URL   : https://patchwork.freedesktop.org/series/39821/
> State : warning
> 
> == Summary ==
> 
> $ dim checkpatch origin/drm-tip
> 7cb94d2c3c47 drm/i915: Reword warning for missing cases
> -:40: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'x' - possible side-effects?
> #40: FILE: drivers/gpu/drm/i915/i915_utils.h:43:
> +#define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
> +			     __stringify(x), (long)(x))

checkpatch seems wrong here since __stringify() doesn't evaluate x. And
it looks similar to the one fixed by
e942e2c3f7e0 ("checkpatch: fix stringification macro defect")


Lucas De Marchi


> 
> total: 0 errors, 0 warnings, 1 checks, 10 lines checked
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Reword warning for missing cases (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases (rev2)
URL   : https://patchwork.freedesktop.org/series/39821/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
82628817d5f1 drm/i915: Reword warning for missing cases
-:41: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'x' - possible side-effects?
#41: FILE: drivers/gpu/drm/i915/i915_utils.h:43:
+#define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
+			     __stringify(x), (long)(x))

total: 0 errors, 0 warnings, 1 checks, 10 lines checked

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

✓ Fi.CI.BAT: success for drm/i915: Reword warning for missing cases (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases (rev2)
URL   : https://patchwork.freedesktop.org/series/39821/
State : success

== Summary ==

Series 39821v2 drm/i915: Reword warning for missing cases
https://patchwork.freedesktop.org/api/1.0/series/39821/revisions/2/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:435s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:448s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:383s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:535s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:301s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:515s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:516s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:520s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:510s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:406s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:514s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:431s
fi-gdg-551       total:285  pass:177  dwarn:0   dfail:0   fail:0   skip:108 time:319s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:430s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:519s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:653s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:544s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:507s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:437s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:402s
Blacklisted hosts:
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:495s

bac611eda28af14ef7e2549932c440885bfe58bb drm-tip: 2018y-03m-26d-15h-45m-06s UTC integration manifest
82628817d5f1 drm/i915: Reword warning for missing cases

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8494/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Reword warning for missing cases (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Reword warning for missing cases (rev2)
URL   : https://patchwork.freedesktop.org/series/39821/
State : success

== Summary ==

---- Possible new issues:

Test kms_rotation_crc:
        Subgroup cursor-rotation-180:
                fail       -> PASS       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup 2x-dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
        Subgroup 2x-wf_vblank-ts-check:
                fail       -> PASS       (shard-hsw) fdo#100368 +2
        Subgroup flip-vs-expired-vblank:
                pass       -> FAIL       (shard-hsw) fdo#102887 +1
Test kms_plane_multiple:
        Subgroup atomic-pipe-a-tiling-x:
                fail       -> PASS       (shard-snb) fdo#103166
Test kms_rotation_crc:
        Subgroup primary-rotation-180:
                pass       -> FAIL       (shard-snb) fdo#103925

fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925

shard-apl        total:3495 pass:1831 dwarn:1   dfail:0   fail:7   skip:1655 time:12860s
shard-hsw        total:3495 pass:1778 dwarn:1   dfail:0   fail:6   skip:1709 time:11508s
shard-snb        total:3495 pass:1374 dwarn:1   dfail:0   fail:3   skip:2117 time:6948s
Blacklisted hosts:
shard-kbl        total:3495 pass:1955 dwarn:1   dfail:0   fail:9   skip:1530 time:9694s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8494/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Reword warning for missing cases

[Rodrigo Vivi]

On Mon, Mar 19, 2018 at 09:41:32PM +0000, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-03-19 17:37:20)
> > In some places we end up converting switch statements to a series of
> > if/else, particularly when introducing helper functions to handle a
> > group of cases. It's tempting to either leave a wrong warning (since now
> > we don't have a switch case anymore) or to convert to WARN(1, ...),
> > but we can just provide a better message and avoid the doubt when such
> > conversions arrise.
> > 
> > Introducing a warning inside i915_driver_load() just for tests we get:
> > 
> > [ 4535.233717] Missing case (ret == 0)
> > [ 4535.233868] WARNING: CPU: 1 PID: 795 at drivers/gpu/drm/i915/i915_drv.c:1341 i915_driver_load+0x42/0x10e0 [i915]
> > 
> > which is clear enough.
> > 
> > v2: remove __func__ since this is already on the warning.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

pushed to dinq. thanks for patch and reviews.

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