[Intel-gfx,01/10] drm/i915: Add i915 device based MISSING_CASE macro
diff mbox series

Message ID 20200225134709.6153-2-pankaj.laxminarayan.bharadiya@intel.com
State New
Headers show
Series
  • drm/i915: Introduce i915 based i915_MISSING_CASE macro and us it in i915
Related show

Commit Message

Pankaj Bharadiya Feb. 25, 2020, 1:47 p.m. UTC
Now that we have struct drm_device based drm_WARN, introduce struct
drm_i915_private based i915_MISSING_CASE macro which uses drm_WARN so
that device specific information will also get printed in backtrace.

i915_MISSING_CASE macro should be preferred over MISSING_CASE,
wherever possible.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
 drivers/gpu/drm/i915/i915_utils.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Chris Wilson Feb. 25, 2020, 2:01 p.m. UTC | #1
Quoting Pankaj Bharadiya (2020-02-25 13:47:00)
> Now that we have struct drm_device based drm_WARN, introduce struct
> drm_i915_private based i915_MISSING_CASE macro which uses drm_WARN so
> that device specific information will also get printed in backtrace.
> 
> i915_MISSING_CASE macro should be preferred over MISSING_CASE,
> wherever possible.

Whatever for? MISSING_CASE() itself should be a complete picture for the
forgotten code.
-Chris
Pankaj Bharadiya Feb. 27, 2020, 6:33 a.m. UTC | #2
Hi Chris,

> -----Original Message-----
> From: Chris Wilson <chris@chris-wilson.co.uk>
> Sent: 25 February 2020 19:32
> To: David Airlie <airlied@linux.ie>; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; daniel@ffwll.ch; dri-devel@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com
> Cc: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Subject: Re: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based
> MISSING_CASE macro
> 
> Quoting Pankaj Bharadiya (2020-02-25 13:47:00)
> > Now that we have struct drm_device based drm_WARN, introduce struct
> > drm_i915_private based i915_MISSING_CASE macro which uses
> drm_WARN so
> > that device specific information will also get printed in backtrace.
> >
> > i915_MISSING_CASE macro should be preferred over MISSING_CASE,
> > wherever possible.
> 
> Whatever for? MISSING_CASE() itself should be a complete picture for the
> forgotten code.

Are you saying, no need to have a new device specific macro?

We want convert all the calls of WARN* with device specific drm_WARN* 
in i915, hence I introduced new i915_MISSING_CASE macro.

Jani, Will you please share your opinion on this?

Thanks,
Pankaj

> -Chris
Jani Nikula Feb. 27, 2020, 8:29 a.m. UTC | #3
On Thu, 27 Feb 2020, "Laxminarayan Bharadiya, Pankaj"	<pankaj.laxminarayan.bharadiya@intel.com> wrote:
> Hi Chris,
>
>> -----Original Message-----
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>> Sent: 25 February 2020 19:32
>> To: David Airlie <airlied@linux.ie>; Joonas Lahtinen
>> <joonas.lahtinen@linux.intel.com>; Laxminarayan Bharadiya, Pankaj
>> <pankaj.laxminarayan.bharadiya@intel.com>; Vivi, Rodrigo
>> <rodrigo.vivi@intel.com>; daniel@ffwll.ch; dri-devel@lists.freedesktop.org;
>> intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com
>> Cc: Laxminarayan Bharadiya, Pankaj
>> <pankaj.laxminarayan.bharadiya@intel.com>
>> Subject: Re: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based
>> MISSING_CASE macro
>> 
>> Quoting Pankaj Bharadiya (2020-02-25 13:47:00)
>> > Now that we have struct drm_device based drm_WARN, introduce struct
>> > drm_i915_private based i915_MISSING_CASE macro which uses
>> drm_WARN so
>> > that device specific information will also get printed in backtrace.
>> >
>> > i915_MISSING_CASE macro should be preferred over MISSING_CASE,
>> > wherever possible.
>> 
>> Whatever for? MISSING_CASE() itself should be a complete picture for the
>> forgotten code.
>
> Are you saying, no need to have a new device specific macro?
>
> We want convert all the calls of WARN* with device specific drm_WARN* 
> in i915, hence I introduced new i915_MISSING_CASE macro.
>
> Jani, Will you please share your opinion on this?

In general, many or most WARNs are device specific, and the device
information is useful. However MISSING_CASE is about the *code*. That
was the intent anyway. Perhaps there are cases where the device
information might be useful, but for most cases probably not.

BR,
Jani.
Pankaj Bharadiya Feb. 28, 2020, 5:02 a.m. UTC | #4
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 27 February 2020 14:00
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; Chris Wilson <chris@chris-
> wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; David Airlie
> <airlied@linux.ie>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi,
> Rodrigo <rodrigo.vivi@intel.com>; daniel@ffwll.ch
> Subject: RE: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based
> MISSING_CASE macro
> 
> On Thu, 27 Feb 2020, "Laxminarayan Bharadiya, Pankaj"
> 	<pankaj.laxminarayan.bharadiya@intel.com> wrote:
> > Hi Chris,
> >
> >> -----Original Message-----
> >> From: Chris Wilson <chris@chris-wilson.co.uk>
> >> Sent: 25 February 2020 19:32
> >> To: David Airlie <airlied@linux.ie>; Joonas Lahtinen
> >> <joonas.lahtinen@linux.intel.com>; Laxminarayan Bharadiya, Pankaj
> >> <pankaj.laxminarayan.bharadiya@intel.com>; Vivi, Rodrigo
> >> <rodrigo.vivi@intel.com>; daniel@ffwll.ch;
> >> dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> >> jani.nikula@linux.intel.com
> >> Cc: Laxminarayan Bharadiya, Pankaj
> >> <pankaj.laxminarayan.bharadiya@intel.com>
> >> Subject: Re: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based
> >> MISSING_CASE macro
> >>
> >> Quoting Pankaj Bharadiya (2020-02-25 13:47:00)
> >> > Now that we have struct drm_device based drm_WARN, introduce struct
> >> > drm_i915_private based i915_MISSING_CASE macro which uses
> >> drm_WARN so
> >> > that device specific information will also get printed in backtrace.
> >> >
> >> > i915_MISSING_CASE macro should be preferred over MISSING_CASE,
> >> > wherever possible.
> >>
> >> Whatever for? MISSING_CASE() itself should be a complete picture for
> >> the forgotten code.
> >
> > Are you saying, no need to have a new device specific macro?
> >
> > We want convert all the calls of WARN* with device specific drm_WARN*
> > in i915, hence I introduced new i915_MISSING_CASE macro.
> >
> > Jani, Will you please share your opinion on this?
> 
> In general, many or most WARNs are device specific, and the device information
> is useful. However MISSING_CASE is about the *code*. That was the intent
> anyway. Perhaps there are cases where the device information might be useful,
> but for most cases probably not.

Thanks for clarification. Please ignore this patch series then.

Thanks,
Pankaj 
> 
> BR,
> Jani.
> 
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index b0ade76bec90..f8db1eb9c1cc 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -52,6 +52,10 @@  struct timer_list;
 #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
 			     __stringify(x), (long)(x))
 
+#define i915_MISSING_CASE(i915, x) drm_WARN(&(i915)->drm, 1, \
+					    "Missing case (%s == %ld)\n", \
+					    __stringify(x), (long)(x))
+
 void __printf(3, 4)
 __i915_printk(struct drm_i915_private *dev_priv, const char *level,
 	      const char *fmt, ...);