| Message ID | 1530586577-20605-2-git-send-email-yakui.zhao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 03, 2018 at 10:56:16AM +0800, Zhao Yakui wrote: > Based on HW spec the fence reg on SNB+ is defined as 64-bit. Just follow > the b-spec to use 64-bit read/write mode. > > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com> Please use git blame to understand why you've just re-introduced a bug that took months to debug. -Daniel > --- > drivers/gpu/drm/i915/i915_gem_fence_reg.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c > index d548ac0..d92fe03 100644 > --- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c > +++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c > @@ -63,6 +63,7 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence, > i915_reg_t fence_reg_lo, fence_reg_hi; > int fence_pitch_shift; > u64 val; > + struct drm_i915_private *dev_priv = fence->i915; > > if (INTEL_GEN(fence->i915) >= 6) { > fence_reg_lo = FENCE_REG_GEN6_LO(fence->id); > @@ -92,9 +93,14 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence, > val |= I965_FENCE_REG_VALID; > } > > - if (!pipelined) { > - struct drm_i915_private *dev_priv = fence->i915; > + if (INTEL_GEN(fence->i915) >= 6) { > + /* Use the 64-bit RW to read/write fence reg on SNB+ */ > + I915_WRITE64_FW(fence_reg_lo, 0); > + I915_READ64(fence_reg_lo); > > + I915_WRITE64_FW(fence_reg_lo, val); > + I915_READ64(fence_reg_lo); > + } else { > /* To w/a incoherency with non-atomic 64-bit register updates, > * we split the 64-bit update into two 32-bit writes. In order > * for a partial fence not to be evaluated between writes, we > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Daniel Vetter (2018-07-03 09:49:29) > On Tue, Jul 03, 2018 at 10:56:16AM +0800, Zhao Yakui wrote: > > Based on HW spec the fence reg on SNB+ is defined as 64-bit. Just follow > > the b-spec to use 64-bit read/write mode. > > > > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com> > > Please use git blame to understand why you've just re-introduced a bug > that took months to debug. And there's even a very nice comment explaining exactly what the HW does and why the double write is required. First rule of IT: turn if off and on again. -Chris
>-----Original Message----- >From: Chris Wilson [mailto:chris@chris-wilson.co.uk] >Sent: Tuesday, July 3, 2018 5:01 PM >To: Daniel Vetter <daniel@ffwll.ch>; Zhao, Yakui <yakui.zhao@intel.com> >Cc: intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: Use 64-bit to Read/Write >fence reg on SNB+ > >Quoting Daniel Vetter (2018-07-03 09:49:29) >> On Tue, Jul 03, 2018 at 10:56:16AM +0800, Zhao Yakui wrote: >> > Based on HW spec the fence reg on SNB+ is defined as 64-bit. Just >> > follow the b-spec to use 64-bit read/write mode. >> > >> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com> >> >> Please use git blame to understand why you've just re-introduced a bug >> that took months to debug. > >And there's even a very nice comment explaining exactly what the HW does >and why the double write is required. > >First rule of IT: turn if off and on again. Hi, Chris/Daniel Thanks for the detailed explanation. I check the history of this issue. It was one commit about five years ago. Maybe the op of fence reg on HW doesn't follow its description very strictly. Not sure whether it is changed on the latest HW. OK. Please ignore this patch as the double write is safer. >-Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c index d548ac0..d92fe03 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c +++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c @@ -63,6 +63,7 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence, i915_reg_t fence_reg_lo, fence_reg_hi; int fence_pitch_shift; u64 val; + struct drm_i915_private *dev_priv = fence->i915; if (INTEL_GEN(fence->i915) >= 6) { fence_reg_lo = FENCE_REG_GEN6_LO(fence->id); @@ -92,9 +93,14 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence, val |= I965_FENCE_REG_VALID; } - if (!pipelined) { - struct drm_i915_private *dev_priv = fence->i915; + if (INTEL_GEN(fence->i915) >= 6) { + /* Use the 64-bit RW to read/write fence reg on SNB+ */ + I915_WRITE64_FW(fence_reg_lo, 0); + I915_READ64(fence_reg_lo); + I915_WRITE64_FW(fence_reg_lo, val); + I915_READ64(fence_reg_lo); + } else { /* To w/a incoherency with non-atomic 64-bit register updates, * we split the 64-bit update into two 32-bit writes. In order * for a partial fence not to be evaluated between writes, we
Based on HW spec the fence reg on SNB+ is defined as 64-bit. Just follow the b-spec to use 64-bit read/write mode. Signed-off-by: Zhao Yakui <yakui.zhao@intel.com> --- drivers/gpu/drm/i915/i915_gem_fence_reg.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)