[Intel-gfx] [PATCH 04/32] drm/i915: Hide the atomic_read(reset_counter) behind a helper

[Chris Wilson]

On Wed, Dec 16, 2015 at 10:33:32AM +0100, Daniel Vetter wrote:
> Hit send too early ;-)
> 
> On Fri, Dec 11, 2015 at 11:33:00AM +0000, Chris Wilson wrote:
> > @@ -4133,7 +4133,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
> >  
> >  		target = request;
> >  	}
> > -	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
> > +	reset_counter = i915_reset_counter(&dev_priv->gpu_error);
> >  	if (target)
> >  		i915_gem_request_reference(target);
> >  	spin_unlock(&file_priv->mm.lock);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 86664d1b3389..60dff3f89531 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2452,7 +2452,7 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
> >  	 * the reset in-progress bit is only ever set by code outside of this
> >  	 * work we don't need to worry about any other races.
> >  	 */
> > -	if (i915_reset_in_progress(error) && !i915_terminally_wedged(error)) {
> > +	if (i915_reset_in_progress_or_wedged(error) && !i915_terminally_wedged(error)) {
> 
> Well maybe simplify this one here.
> 
> >  		DRM_DEBUG_DRIVER("resetting chip\n");
> >  		kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE,
> >  				   reset_event);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7dd7200d3ba9..cc47c0206294 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3285,10 +3285,12 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +	unsigned reset_counter;
> >  	bool pending;
> >  
> > -	if (i915_reset_in_progress(&dev_priv->gpu_error) ||
> > -	    intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
> > +	reset_counter = i915_reset_counter(&dev_priv->gpu_error);
> > +	if (intel_crtc->reset_counter != reset_counter ||
> > +	    __i915_reset_in_progress_or_wedged(reset_counter))
> >  		return false;
> >  
> >  	spin_lock_irq(&dev->event_lock);
> > @@ -10942,9 +10944,11 @@ static bool page_flip_finished(struct intel_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	unsigned reset_counter;
> >  
> > -	if (i915_reset_in_progress(&dev_priv->gpu_error) ||
> > -	    crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
> > +	reset_counter = i915_reset_counter(&dev_priv->gpu_error);
> > +	if (crtc->reset_counter != reset_counter ||
> > +	    __i915_reset_in_progress_or_wedged(reset_counter))
> >  		return true;
> >  
> >  	/*
> > @@ -11601,7 +11605,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >  		goto cleanup;
> >  
> >  	atomic_inc(&intel_crtc->unpin_work_count);
> > -	intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
> > +	intel_crtc->reset_counter = i915_reset_counter(&dev_priv->gpu_error);
> >  
> >  	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> >  		work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(pipe)) + 1;
> > @@ -13388,10 +13392,10 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> >  		return ret;
> >  
> >  	ret = drm_atomic_helper_prepare_planes(dev, state);
> > -	if (!ret && !async && !i915_reset_in_progress(&dev_priv->gpu_error)) {
> > +	if (!ret && !async && !i915_reset_in_progress_or_wedged(&dev_priv->gpu_error)) {
> 
> I guess we should also drop the _or_wedged here since dead gpu isn't a
> problem for the modeset code? Or is that later on in the series ...

Both of these turn up later in the series. My goal in this patch was to
try a mechinical translation (albeit to a more verbose name). It was
only when I was sure that we had the nuances of the reset handler under
control that I felt safe in making changes to the tests.

So I'm going to take your r-b and run with it!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre