| Message ID | 1438869659-12561-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8/6/2015 3:00 PM, Mika Kuoppala wrote: > This register needs to be updated with masked writes. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 99bba8e..29347e7 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -521,7 +521,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) > ring->next_context_status_buffer = write_pointer % 6; > > I915_WRITE(RING_CONTEXT_STATUS_PTR(ring), > - ((u32)ring->next_context_status_buffer & 0x07) << 8); > + _MASKED_FIELD(0x07 << 8, ((u32)ring->next_context_status_buffer & 0x07) << 8)); > } > > static int execlists_context_queue(struct drm_i915_gem_request *request) > -- > 2.1.4 > bspec agrees... but I remember seeing these bits being written without the mask in gen8. Reviewed-by: Michel Thierry <michel.thierry@intel.com>
On Thu, Aug 06, 2015 at 03:25:55PM +0100, Michel Thierry wrote: > On 8/6/2015 3:00 PM, Mika Kuoppala wrote: > >This register needs to be updated with masked writes. > > > >Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > >--- > > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > >index 99bba8e..29347e7 100644 > >--- a/drivers/gpu/drm/i915/intel_lrc.c > >+++ b/drivers/gpu/drm/i915/intel_lrc.c > >@@ -521,7 +521,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) > > ring->next_context_status_buffer = write_pointer % 6; > > > > I915_WRITE(RING_CONTEXT_STATUS_PTR(ring), > >- ((u32)ring->next_context_status_buffer & 0x07) << 8); > >+ _MASKED_FIELD(0x07 << 8, ((u32)ring->next_context_status_buffer & 0x07) << 8)); > > } > > > > static int execlists_context_queue(struct drm_i915_gem_request *request) > >-- > >2.1.4 > > > > bspec agrees... but I remember seeing these bits being written without the > mask in gen8. Impact? Do I need this for -fixes with cc: stable? lrc is shipping as released code, please be a bit more elaborate in your commit messages. Thanks, Daniel > Reviewed-by: Michel Thierry <michel.thierry@intel.com> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/6/2015 5:03 PM, Daniel Vetter wrote: > On Thu, Aug 06, 2015 at 03:25:55PM +0100, Michel Thierry wrote: >> On 8/6/2015 3:00 PM, Mika Kuoppala wrote: >>> This register needs to be updated with masked writes. >>> >>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_lrc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c >>> index 99bba8e..29347e7 100644 >>> --- a/drivers/gpu/drm/i915/intel_lrc.c >>> +++ b/drivers/gpu/drm/i915/intel_lrc.c >>> @@ -521,7 +521,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) >>> ring->next_context_status_buffer = write_pointer % 6; >>> >>> I915_WRITE(RING_CONTEXT_STATUS_PTR(ring), >>> - ((u32)ring->next_context_status_buffer & 0x07) << 8); >>> + _MASKED_FIELD(0x07 << 8, ((u32)ring->next_context_status_buffer & 0x07) << 8)); >>> } >>> >>> static int execlists_context_queue(struct drm_i915_gem_request *request) >>> -- >>> 2.1.4 >>> >> >> bspec agrees... but I remember seeing these bits being written without the >> mask in gen8. > > Impact? Do I need this for -fixes with cc: stable? lrc is shipping as > released code, please be a bit more elaborate in your commit messages. I'll let Mika give more details, but it looks inoffensive to me. The lrc irq handler relies on bits[3:0] only (the read pointer, which is read-only and is updated by the hw automatically), and never reads the write pointer back. It would only impact us when reading the register directly, e.g. while debugging another issue. -Michel > > Thanks, Daniel > >> Reviewed-by: Michel Thierry <michel.thierry@intel.com> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 99bba8e..29347e7 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -521,7 +521,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) ring->next_context_status_buffer = write_pointer % 6; I915_WRITE(RING_CONTEXT_STATUS_PTR(ring), - ((u32)ring->next_context_status_buffer & 0x07) << 8); + _MASKED_FIELD(0x07 << 8, ((u32)ring->next_context_status_buffer & 0x07) << 8)); } static int execlists_context_queue(struct drm_i915_gem_request *request)
This register needs to be updated with masked writes. Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)