| Message ID | 1467110253-16046-3-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 28, 2016 at 01:37:31PM +0300, Imre Deak wrote: > Since wait_for_atomic doesn't re-check the wait-for condition after > expiry of the timeout it can fail when called from non-atomic context > even if the condition is set correctly before the expiry. Fix this by > using the non-atomic wait_for instead. Same point as patch 1. If the bug is using the wrong function, ... The only question is whether you do have an atomic context, in which case we need to reinspect the notion of "there is only at most 3 non-preemptible instructions between the COND and the timeout"... -Chris
On ti, 2016-06-28 at 11:50 +0100, Chris Wilson wrote: > On Tue, Jun 28, 2016 at 01:37:31PM +0300, Imre Deak wrote: > > Since wait_for_atomic doesn't re-check the wait-for condition after > > expiry of the timeout it can fail when called from non-atomic > > context > > even if the condition is set correctly before the expiry. Fix this > > by > > using the non-atomic wait_for instead. > > Same point as patch 1. If the bug is using the wrong function, ... > The only question is whether you do have an atomic context, in which > case we need to reinspect the notion of "there is only at most 3 > non-preemptible instructions between the COND and the timeout"... These functions are only called from non-atomic context. --Imre
On 28/06/16 11:37, Imre Deak wrote: > Since wait_for_atomic doesn't re-check the wait-for condition after > expiry of the timeout it can fail when called from non-atomic context > even if the condition is set correctly before the expiry. Fix this by > using the non-atomic wait_for instead. > > Fixes: 0351b93992aa ("drm/i915: Do not lie about atomic timeout granularity") > CC: Chris Wilson <chris@chris-wilson.co.uk> > CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c3b5dc8..0312472 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8537,16 +8537,16 @@ static void lpt_reset_fdi_mphy(struct drm_i915_private *dev_priv) > tmp |= FDI_MPHY_IOSFSB_RESET_CTL; > I915_WRITE(SOUTH_CHICKEN2, tmp); > > - if (wait_for_atomic_us(I915_READ(SOUTH_CHICKEN2) & > - FDI_MPHY_IOSFSB_RESET_STATUS, 100)) > + if (wait_for_us(I915_READ(SOUTH_CHICKEN2) & > + FDI_MPHY_IOSFSB_RESET_STATUS, 100)) > DRM_ERROR("FDI mPHY reset assert timeout\n"); > > tmp = I915_READ(SOUTH_CHICKEN2); > tmp &= ~FDI_MPHY_IOSFSB_RESET_CTL; > I915_WRITE(SOUTH_CHICKEN2, tmp); > > - if (wait_for_atomic_us((I915_READ(SOUTH_CHICKEN2) & > - FDI_MPHY_IOSFSB_RESET_STATUS) == 0, 100)) > + if (wait_for_us((I915_READ(SOUTH_CHICKEN2) & > + FDI_MPHY_IOSFSB_RESET_STATUS) == 0, 100)) > DRM_ERROR("FDI mPHY reset de-assert timeout\n"); > } This one was easy to find non-atomic context is correct. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c3b5dc8..0312472 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8537,16 +8537,16 @@ static void lpt_reset_fdi_mphy(struct drm_i915_private *dev_priv) tmp |= FDI_MPHY_IOSFSB_RESET_CTL; I915_WRITE(SOUTH_CHICKEN2, tmp); - if (wait_for_atomic_us(I915_READ(SOUTH_CHICKEN2) & - FDI_MPHY_IOSFSB_RESET_STATUS, 100)) + if (wait_for_us(I915_READ(SOUTH_CHICKEN2) & + FDI_MPHY_IOSFSB_RESET_STATUS, 100)) DRM_ERROR("FDI mPHY reset assert timeout\n"); tmp = I915_READ(SOUTH_CHICKEN2); tmp &= ~FDI_MPHY_IOSFSB_RESET_CTL; I915_WRITE(SOUTH_CHICKEN2, tmp); - if (wait_for_atomic_us((I915_READ(SOUTH_CHICKEN2) & - FDI_MPHY_IOSFSB_RESET_STATUS) == 0, 100)) + if (wait_for_us((I915_READ(SOUTH_CHICKEN2) & + FDI_MPHY_IOSFSB_RESET_STATUS) == 0, 100)) DRM_ERROR("FDI mPHY reset de-assert timeout\n"); }
Since wait_for_atomic doesn't re-check the wait-for condition after expiry of the timeout it can fail when called from non-atomic context even if the condition is set correctly before the expiry. Fix this by using the non-atomic wait_for instead. Fixes: 0351b93992aa ("drm/i915: Do not lie about atomic timeout granularity") CC: Chris Wilson <chris@chris-wilson.co.uk> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)