[Intel-gfx] [PATCH v2] drm/i915: Wait for gen3 reset status to be asserted

[Chris Wilson]

Quoting Mika Kuoppala (2018-02-08 11:43:50)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > After we assert the reset request (and wait for 20us), when the device
> > has been fully reset it asserts the reset-status bit. Before we stop
> > requesting the reset and allow the device to return to normal, we should
> > wait for the reset to be completed. (Similar to how we wait for the
> > device to return to normal after deasserting the reset request.)
> >
> > v2: Rename i915_reset_completed() probe to not cause as much confusion.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_uncore.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 0fd59dd6bbb5..e09981a3113c 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1550,24 +1550,31 @@ static void i915_stop_engines(struct drm_i915_private *dev_priv,
> >               gen3_stop_engine(engine);
> >  }
> >  
> > -static bool i915_reset_complete(struct pci_dev *pdev)
> > +static bool i915_in_reset(struct pci_dev *pdev)
> >  {
> >       u8 gdrst;
> >  
> >       pci_read_config_byte(pdev, I915_GDRST, &gdrst);
> > -     return (gdrst & GRDOM_RESET_STATUS) == 0;
> > +     return gdrst & GRDOM_RESET_STATUS;
> >  }
> >  
> >  static int i915_do_reset(struct drm_i915_private *dev_priv, unsigned engine_mask)
> >  {
> >       struct pci_dev *pdev = dev_priv->drm.pdev;
> > +     int err;
> >  
> > -     /* assert reset for at least 20 usec */
> > +     /* Assert reset for at least 20 usec, and wait for acknowledgement. */
> >       pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> >       usleep_range(50, 200);
> > +     err = wait_for(i915_in_reset(pdev), 500);
> > +
> > +     /* Clear the reset request. */
> >       pci_write_config_byte(pdev, I915_GDRST, 0);
> > +     usleep_range(50, 200);
> > +     if (!err)
> > +             err = wait_for(!i915_in_reset(pdev), 500);
> >
> 
> Regardless of if this helps or not with pnv, it makes the
> both phases clear.
> 
> Any thoughts of starting to log the reset attempts
> with timeout, even if the subsequent reset succeeds?
> 
> Patch is,
> Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

Although I don't expect this pair to help CI, it feels justifiable
paranoia. Thanks for the review,
-Chris