| Message ID | 1420775953-1100-1-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 08, 2015 at 07:59:10PM -0800, Ben Widawsky wrote: > Implements a required workaround whose implications aren't entirely clear to me > from the description. In particular I do not know if this effects legacy > contexts, execlists, or both. > > I couldn't find a real workaround name, so I made up: > WaHdcCtxNonCoherent I don't think we want to make up w/a names. Might cause someone to conclude that the w/a is no longer needed if they can't find the name in the w/a database or bspec. So maybe just add a small quote from bspec, or leave it without explanation forcing people to check bspec if they want to find out why it's there. I suppose one option would be to add a private namespace for our made up w/a names. But I don't really see a point in making up w/a names if we don't have a some documentation telling people what those names actually mean. So with the made up w/a name removed: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_reg.h | 5 +++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0f32fd1a..dabac96 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5219,9 +5219,10 @@ enum punit_power_well { > > /* GEN8 chicken */ > #define HDC_CHICKEN0 0x7300 > -#define HDC_FORCE_NON_COHERENT (1<<4) > -#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) > #define HDC_FENCE_DEST_SLM_DISABLE (1<<14) > +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) > +#define HDC_FORCE_CTX_NON_COHERENT (1<<5) > +#define HDC_FORCE_NON_COHERENT (1<<4) > > /* WaCatErrorRejectionIssue */ > #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 12a36f0..62318a4 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -790,8 +790,10 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring) > */ > /* WaForceEnableNonCoherent:bdw */ > /* WaHdcDisableFetchWhenMasked:bdw */ > + /* WaHdcCtxNonCoherent:bdw */ > /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ > WA_SET_BIT_MASKED(HDC_CHICKEN0, > + HDC_FORCE_CTX_NON_COHERENT | > HDC_FORCE_NON_COHERENT | > HDC_DONOT_FETCH_MEM_WHEN_MASKED | > (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0)); > -- > 2.2.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Feb 02, 2015 at 02:33:48PM +0200, Ville Syrjälä wrote: > On Thu, Jan 08, 2015 at 07:59:10PM -0800, Ben Widawsky wrote: > > Implements a required workaround whose implications aren't entirely clear to me > > from the description. In particular I do not know if this effects legacy > > contexts, execlists, or both. > > > > I couldn't find a real workaround name, so I made up: > > WaHdcCtxNonCoherent > > I don't think we want to make up w/a names. Might cause someone to > conclude that the w/a is no longer needed if they can't find the > name in the w/a database or bspec. So maybe just add a small quote from > bspec, or leave it without explanation forcing people to check bspec > if they want to find out why it's there. > > I suppose one option would be to add a private namespace for our made > up w/a names. But I don't really see a point in making up w/a names > if we don't have a some documentation telling people what those names > actually mean. > > So with the made up w/a name removed: > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> If you want to believe my version, it's called WaForceContextSaveRestoreNonCoherent http://lists.freedesktop.org/archives/intel-gfx/2015-January/059086.html
On Mon, Feb 02, 2015 at 01:21:19PM +0000, Damien Lespiau wrote: > On Mon, Feb 02, 2015 at 02:33:48PM +0200, Ville Syrjälä wrote: > > On Thu, Jan 08, 2015 at 07:59:10PM -0800, Ben Widawsky wrote: > > > Implements a required workaround whose implications aren't entirely clear to me > > > from the description. In particular I do not know if this effects legacy > > > contexts, execlists, or both. > > > > > > I couldn't find a real workaround name, so I made up: > > > WaHdcCtxNonCoherent > > > > I don't think we want to make up w/a names. Might cause someone to > > conclude that the w/a is no longer needed if they can't find the > > name in the w/a database or bspec. So maybe just add a small quote from > > bspec, or leave it without explanation forcing people to check bspec > > if they want to find out why it's there. > > > > I suppose one option would be to add a private namespace for our made > > up w/a names. But I don't really see a point in making up w/a names > > if we don't have a some documentation telling people what those names > > actually mean. > > > > So with the made up w/a name removed: > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > If you want to believe my version, it's called WaForceContextSaveRestoreNonCoherent > > http://lists.freedesktop.org/archives/intel-gfx/2015-January/059086.html > If you reorder the defines as I did, it's Reviewed-by: Ben Widawsky <ben@bwidawsk.net> It really irks me that the defines are out of place. Or you can send the v2 of my patch :D
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0f32fd1a..dabac96 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5219,9 +5219,10 @@ enum punit_power_well { /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 -#define HDC_FORCE_NON_COHERENT (1<<4) -#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) #define HDC_FENCE_DEST_SLM_DISABLE (1<<14) +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) +#define HDC_FORCE_CTX_NON_COHERENT (1<<5) +#define HDC_FORCE_NON_COHERENT (1<<4) /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 12a36f0..62318a4 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -790,8 +790,10 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring) */ /* WaForceEnableNonCoherent:bdw */ /* WaHdcDisableFetchWhenMasked:bdw */ + /* WaHdcCtxNonCoherent:bdw */ /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ WA_SET_BIT_MASKED(HDC_CHICKEN0, + HDC_FORCE_CTX_NON_COHERENT | HDC_FORCE_NON_COHERENT | HDC_DONOT_FETCH_MEM_WHEN_MASKED | (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0));
Implements a required workaround whose implications aren't entirely clear to me from the description. In particular I do not know if this effects legacy contexts, execlists, or both. I couldn't find a real workaround name, so I made up: WaHdcCtxNonCoherent Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_reg.h | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-)