| Message ID | 20200225134709.6153-2-pankaj.laxminarayan.bharadiya@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Introduce i915 based i915_MISSING_CASE macro and us it in i915 | expand |
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
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
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.
> -----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
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, ...);
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(+)