Re: [PATCH 05/10] drm/i915: add assert_rpm_wakelock_held helper

[Imre Deak <imre.deak@intel.com>]

On ke, 2015-12-16 at 15:39 +0200, Joonas Lahtinen wrote:
> On ke, 2015-12-16 at 13:02 +0000, Chris Wilson wrote:
> > On Wed, Dec 16, 2015 at 02:54:43PM +0200, Imre Deak wrote:
> > > On ke, 2015-12-16 at 12:11 +0000, Chris Wilson wrote:
> > > > On Tue, Dec 15, 2015 at 08:10:33PM +0200, Imre Deak wrote:
> > > > > +static inline void
> > > > > +assert_rpm_device_not_suspended(struct drm_i915_private
> > > > > *dev_priv)
> > > > > +{
> > > > > +	WARN_ONCE(dev_priv->pm.suspended,
> > > > > +		  "Device suspended during HW access\n");
> > > > > +}
> > > > 
> > > > On irc, Joonas expressed a wish to see all errors during an igt
> > > > run,
> > > > i.e. something like
> > > > 
> > > > static inline void
> > > > assert_rpm_device_not_suspended(struct drm_i915_private
> > > > *dev_priv)
> > > > {
> > > > 	WARN(dev_priv->pm.suspended &&
> > > > 	     atomic_inc_return(&dev_priv->pm.errors) < 0,
> > > > 		  "Device suspended during HW access\n");
> > > > }
> > [snip]
> > 
> > > Sounds good, we could use this also for other PM related error
> > > reporting.
> > > 
> > > Are you ok to do this as a follow-up?
> > 
> > Definitely. We haven't changed any behaviour so far, so this is a
> > new
> > feature.
> 
> I'd prefer to currently add it as WARN instead of WARN_ONCE, and then
> reduce the message amount with follow-up. This way we'll get more
> useful CI results immediately.

And more annoyed upstream users.. Really adding that knob is a good
idea and it doesn't take long to implement it, but we could make
progress if we did it as a follow-up.

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