| Message ID | 20180712210251.12454-1-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/07/18 14:02, Lucas De Marchi wrote: > After disabling resource streamer on ICL (due to it actually not > existing there), I got feedback that there have been some experimental > patches for mesa to use it, but nothing ever landed nor shipped. > > This is a tentative to remove it from kernel keeping the uapi defines > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > some platforms - Daniele mentioned there's possible performance regression > in gem_latency if this bit is not set. On this patch, I'm just removing > it in order to get proper numbers. I've re-run gem_latency on the same machine I saw the regression on with and without this patch applied. I've tried both with 2 and 10 contexts and the regression I saw with a similar patch a few months back seems to not be happening. The average numbers with the patch applied came out very slightly worse (10 iterations of 60s, ~1% extra latency average) but I'm assuming that's within the noise (what I saw at the time was ~20% latency increase). Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > - don't bother trying to document removed params on uapi header: > applications should know that from the query. > (from Chris) > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > drivers/gpu/drm/i915/intel_device_info.h | 1 - > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > 8 files changed, 6 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3eba3d1ab5b8..10423e2c1176 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, > value = 2; > break; > case I915_PARAM_HAS_RESOURCE_STREAMER: > - value = HAS_RESOURCE_STREAMER(dev_priv); > + value = 0; > break; > case I915_PARAM_HAS_POOLED_EU: > value = HAS_POOLED_EU(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 01dd29837233..dddb886fa06c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission() > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) > - > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 1932bc227942..ca21a08b2be9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > if (!eb.engine) > return -EINVAL; > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > - DRM_DEBUG("RS is only allowed for Haswell and Gen8 - Gen10\n"); > - return -EINVAL; > - } > - if (eb.engine->id != RCS) { > - DRM_DEBUG("RS is not available on %s\n", > - eb.engine->name); > - return -EINVAL; > - } > - > - eb.batch_flags |= I915_DISPATCH_RS; > - } > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > + return -EINVAL; > > if (args->flags & I915_EXEC_FENCE_IN) { > in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index c03ba0fe0845..7af19097dce8 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -360,7 +360,6 @@ static const struct intel_device_info intel_valleyview_info = { > .has_ddi = 1, \ > .has_fpga_dbg = 1, \ > .has_psr = 1, \ > - .has_resource_streamer = 1, \ > .has_dp_mst = 1, \ > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > .has_runtime_pm = 1 > @@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info = { > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > .has_64bit_reloc = 1, > .has_runtime_pm = 1, > - .has_resource_streamer = 1, > .has_rc6 = 1, > .has_logical_ring_contexts = 1, > .has_gmch_display = 1, > @@ -506,7 +504,6 @@ static const struct intel_device_info intel_skylake_gt4_info = { > .has_runtime_pm = 1, \ > .has_pooled_eu = 0, \ > .has_csr = 1, \ > - .has_resource_streamer = 1, \ > .has_rc6 = 1, \ > .has_dp_mst = 1, \ > .has_logical_ring_contexts = 1, \ > @@ -593,7 +590,6 @@ static const struct intel_device_info intel_cannonlake_info = { > GEN(11), \ > .ddb_size = 2048, \ > .has_csr = 0, \ > - .has_resource_streamer = 0, \ > .has_logical_ring_elsq = 1 > > static const struct intel_device_info intel_icelake_11_info = { > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 633f9fbf72ea..6814cc7dfcd3 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -103,7 +103,6 @@ enum intel_platform { > func(has_psr); \ > func(has_rc6); \ > func(has_rc6p); \ > - func(has_resource_streamer); \ > func(has_runtime_pm); \ > func(has_snoop); \ > func(unfenced_needs_alignment); \ > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 35d37af0cb9a..ad10fb7b9668 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request *rq, > > /* FIXME(BDW): Address space and security selectors. */ > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0); > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > *cs++ = lower_32_bits(offset); > *cs++ = upper_32_bits(offset); > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > - (HAS_RESOURCE_STREAMER(dev_priv) ? > - CTX_CTRL_RS_CTX_ENABLE : 0))); > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index f4bd185c9369..e5fcbbc2abbc 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > return PTR_ERR(cs); > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & I915_DISPATCH_SECURE ? > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > - (dispatch_flags & I915_DISPATCH_RS ? > - MI_BATCH_RESOURCE_STREAMER : 0); > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > /* bit0-7 is the length on GEN6+ */ > *cs++ = offset; > intel_ring_advance(rq, cs); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index d1eee08e5f6b..a2c1d7044f2a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -467,7 +467,6 @@ struct intel_engine_cs { > unsigned int dispatch_flags); > #define I915_DISPATCH_SECURE BIT(0) > #define I915_DISPATCH_PINNED BIT(1) > -#define I915_DISPATCH_RS BIT(2) > void (*emit_breadcrumb)(struct i915_request *rq, u32 *cs); > int emit_breadcrumb_sz; > >
On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote: > After disabling resource streamer on ICL (due to it actually not > existing there), I got feedback that there have been some experimental > patches for mesa to use it, but nothing ever landed nor shipped. > > This is a tentative to remove it from kernel keeping the uapi defines > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > some platforms - Daniele mentioned there's possible performance regression > in gem_latency if this bit is not set. On this patch, I'm just removing > it in order to get proper numbers. Since I see below 3 occurrences of - .has_resource_streamer = 1 I feel this commit message lacks of the reason why we are removing this features for the platforms that already had it. > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > - don't bother trying to document removed params on uapi header: > applications should know that from the query. > (from Chris) > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > drivers/gpu/drm/i915/intel_device_info.h | 1 - > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > 8 files changed, 6 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3eba3d1ab5b8..10423e2c1176 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, > value = 2; > break; > case I915_PARAM_HAS_RESOURCE_STREAMER: > - value = HAS_RESOURCE_STREAMER(dev_priv); > + value = 0; > break; > case I915_PARAM_HAS_POOLED_EU: > value = HAS_POOLED_EU(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 01dd29837233..dddb886fa06c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission() > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) > - > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 1932bc227942..ca21a08b2be9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > if (!eb.engine) > return -EINVAL; > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > - DRM_DEBUG("RS is only allowed for Haswell and Gen8 - Gen10\n"); > - return -EINVAL; > - } > - if (eb.engine->id != RCS) { > - DRM_DEBUG("RS is not available on %s\n", > - eb.engine->name); > - return -EINVAL; > - } > - > - eb.batch_flags |= I915_DISPATCH_RS; > - } > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > + return -EINVAL; > > if (args->flags & I915_EXEC_FENCE_IN) { > in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index c03ba0fe0845..7af19097dce8 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -360,7 +360,6 @@ static const struct intel_device_info intel_valleyview_info = { > .has_ddi = 1, \ > .has_fpga_dbg = 1, \ > .has_psr = 1, \ > - .has_resource_streamer = 1, \ > .has_dp_mst = 1, \ > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > .has_runtime_pm = 1 > @@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info = { > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > .has_64bit_reloc = 1, > .has_runtime_pm = 1, > - .has_resource_streamer = 1, > .has_rc6 = 1, > .has_logical_ring_contexts = 1, > .has_gmch_display = 1, > @@ -506,7 +504,6 @@ static const struct intel_device_info intel_skylake_gt4_info = { > .has_runtime_pm = 1, \ > .has_pooled_eu = 0, \ > .has_csr = 1, \ > - .has_resource_streamer = 1, \ > .has_rc6 = 1, \ > .has_dp_mst = 1, \ > .has_logical_ring_contexts = 1, \ > @@ -593,7 +590,6 @@ static const struct intel_device_info intel_cannonlake_info = { > GEN(11), \ > .ddb_size = 2048, \ > .has_csr = 0, \ > - .has_resource_streamer = 0, \ > .has_logical_ring_elsq = 1 > > static const struct intel_device_info intel_icelake_11_info = { > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 633f9fbf72ea..6814cc7dfcd3 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -103,7 +103,6 @@ enum intel_platform { > func(has_psr); \ > func(has_rc6); \ > func(has_rc6p); \ > - func(has_resource_streamer); \ > func(has_runtime_pm); \ > func(has_snoop); \ > func(unfenced_needs_alignment); \ > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 35d37af0cb9a..ad10fb7b9668 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request *rq, > > /* FIXME(BDW): Address space and security selectors. */ > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0); > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > *cs++ = lower_32_bits(offset); > *cs++ = upper_32_bits(offset); > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > - (HAS_RESOURCE_STREAMER(dev_priv) ? > - CTX_CTRL_RS_CTX_ENABLE : 0))); > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index f4bd185c9369..e5fcbbc2abbc 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > return PTR_ERR(cs); > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & I915_DISPATCH_SECURE ? > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > - (dispatch_flags & I915_DISPATCH_RS ? > - MI_BATCH_RESOURCE_STREAMER : 0); > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > /* bit0-7 is the length on GEN6+ */ > *cs++ = offset; > intel_ring_advance(rq, cs); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index d1eee08e5f6b..a2c1d7044f2a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -467,7 +467,6 @@ struct intel_engine_cs { > unsigned int dispatch_flags); > #define I915_DISPATCH_SECURE BIT(0) > #define I915_DISPATCH_PINNED BIT(1) > -#define I915_DISPATCH_RS BIT(2) > void (*emit_breadcrumb)(struct i915_request *rq, u32 *cs); > int emit_breadcrumb_sz; > > -- > 2.17.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote: > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote: > > After disabling resource streamer on ICL (due to it actually not > > existing there), I got feedback that there have been some experimental > > patches for mesa to use it, but nothing ever landed nor shipped. > > > > This is a tentative to remove it from kernel keeping the uapi defines > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > > some platforms - Daniele mentioned there's possible performance regression > > in gem_latency if this bit is not set. On this patch, I'm just removing > > it in order to get proper numbers. > > Since I see below 3 occurrences of > - .has_resource_streamer = 1 > > I feel this commit message lacks of the reason why we are removing > this features for the platforms that already had it. /me confused. Isn't the first paragraph in the commit message exactly that? Lucas De Marchi > > > > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > > - don't bother trying to document removed params on uapi header: > > applications should know that from the query. > > (from Chris) > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 -- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > > drivers/gpu/drm/i915/intel_device_info.h | 1 - > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > > 8 files changed, 6 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 3eba3d1ab5b8..10423e2c1176 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, > > void *data, > > value = 2; > > break; > > case I915_PARAM_HAS_RESOURCE_STREAMER: > > - value = HAS_RESOURCE_STREAMER(dev_priv); > > + value = 0; > > break; > > case I915_PARAM_HAS_POOLED_EU: > > value = HAS_POOLED_EU(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 01dd29837233..dddb886fa06c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis > > sion() > > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)- > > >info.has_resource_streamer) > > - > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 1932bc227942..ca21a08b2be9 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > if (!eb.engine) > > return -EINVAL; > > > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > > - DRM_DEBUG("RS is only allowed for Haswell and > > Gen8 - Gen10\n"); > > - return -EINVAL; > > - } > > - if (eb.engine->id != RCS) { > > - DRM_DEBUG("RS is not available on %s\n", > > - eb.engine->name); > > - return -EINVAL; > > - } > > - > > - eb.batch_flags |= I915_DISPATCH_RS; > > - } > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > > + return -EINVAL; > > > > if (args->flags & I915_EXEC_FENCE_IN) { > > in_fence = sync_file_get_fence(lower_32_bits(args- > > >rsvd2)); > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > b/drivers/gpu/drm/i915/i915_pci.c > > index c03ba0fe0845..7af19097dce8 100644 > > --- a/drivers/gpu/drm/i915/i915_pci.c > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > @@ -360,7 +360,6 @@ static const struct intel_device_info > > intel_valleyview_info = { > > .has_ddi = 1, \ > > .has_fpga_dbg = 1, \ > > .has_psr = 1, \ > > - .has_resource_streamer = 1, \ > > .has_dp_mst = 1, \ > > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > > .has_runtime_pm = 1 > > @@ -433,7 +432,6 @@ static const struct intel_device_info > > intel_cherryview_info = { > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > > .has_64bit_reloc = 1, > > .has_runtime_pm = 1, > > - .has_resource_streamer = 1, > > .has_rc6 = 1, > > .has_logical_ring_contexts = 1, > > .has_gmch_display = 1, > > @@ -506,7 +504,6 @@ static const struct intel_device_info > > intel_skylake_gt4_info = { > > .has_runtime_pm = 1, \ > > .has_pooled_eu = 0, \ > > .has_csr = 1, \ > > - .has_resource_streamer = 1, \ > > .has_rc6 = 1, \ > > .has_dp_mst = 1, \ > > .has_logical_ring_contexts = 1, \ > > @@ -593,7 +590,6 @@ static const struct intel_device_info > > intel_cannonlake_info = { > > GEN(11), \ > > .ddb_size = 2048, \ > > .has_csr = 0, \ > > - .has_resource_streamer = 0, \ > > .has_logical_ring_elsq = 1 > > > > static const struct intel_device_info intel_icelake_11_info = { > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h > > b/drivers/gpu/drm/i915/intel_device_info.h > > index 633f9fbf72ea..6814cc7dfcd3 100644 > > --- a/drivers/gpu/drm/i915/intel_device_info.h > > +++ b/drivers/gpu/drm/i915/intel_device_info.h > > @@ -103,7 +103,6 @@ enum intel_platform { > > func(has_psr); \ > > func(has_rc6); \ > > func(has_rc6p); \ > > - func(has_resource_streamer); \ > > func(has_runtime_pm); \ > > func(has_snoop); \ > > func(unfenced_needs_alignment); \ > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index 35d37af0cb9a..ad10fb7b9668 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request > > *rq, > > > > /* FIXME(BDW): Address space and security selectors. */ > > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : > > 0); > > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > > *cs++ = lower_32_bits(offset); > > *cs++ = upper_32_bits(offset); > > > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > > - (HAS_RESOURCE_STREAMER(dev_priv) ? > > - CTX_CTRL_RS_CTX_ENABLE : 0))); > > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index f4bd185c9369..e5fcbbc2abbc 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > > return PTR_ERR(cs); > > > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & > > I915_DISPATCH_SECURE ? > > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > > - (dispatch_flags & I915_DISPATCH_RS ? > > - MI_BATCH_RESOURCE_STREAMER : 0); > > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > > /* bit0-7 is the length on GEN6+ */ > > *cs++ = offset; > > intel_ring_advance(rq, cs); > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > > b/drivers/gpu/drm/i915/intel_ringbuffer.h > > index d1eee08e5f6b..a2c1d7044f2a 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > > @@ -467,7 +467,6 @@ struct intel_engine_cs { > > unsigned int dispatch_flags); > > #define I915_DISPATCH_SECURE BIT(0) > > #define I915_DISPATCH_PINNED BIT(1) > > -#define I915_DISPATCH_RS BIT(2) > > void (*emit_breadcrumb)(struct i915_request *rq, > > u32 *cs); > > int emit_breadcrumb_sz; > > > > -- > > 2.17.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jul 12, 2018 at 11:28:25PM +0000, De Marchi, Lucas wrote: > On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote: > > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote: > > > After disabling resource streamer on ICL (due to it actually not > > > existing there), I got feedback that there have been some experimental > > > patches for mesa to use it, but nothing ever landed nor shipped. > > > > > > This is a tentative to remove it from kernel keeping the uapi defines > > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > > > some platforms - Daniele mentioned there's possible performance regression > > > in gem_latency if this bit is not set. On this patch, I'm just removing > > > it in order to get proper numbers. > > > > Since I see below 3 occurrences of > > - .has_resource_streamer = 1 > > > > I feel this commit message lacks of the reason why we are removing > > this features for the platforms that already had it. > > /me confused. Isn't the first paragraph in the commit message exactly that? /me confused now... For me the first paragraph tells that after removing from ICL mesa might want to use it on ICL. second paragraph tells that you are removing the support but keeping the uapi. what I couldn't find is the explanation why exactly you are removing this feature. Is this useless? No body using it? It is buggy? It is deprecated? why? > > Lucas De Marchi > > > > > > > > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > > > - don't bother trying to document removed params on uapi header: > > > applications should know that from the query. > > > (from Chris) > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > > drivers/gpu/drm/i915/i915_drv.h | 2 -- > > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > > > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > > > drivers/gpu/drm/i915/intel_device_info.h | 1 - > > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > > > 8 files changed, 6 insertions(+), 30 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 3eba3d1ab5b8..10423e2c1176 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, > > > void *data, > > > value = 2; > > > break; > > > case I915_PARAM_HAS_RESOURCE_STREAMER: > > > - value = HAS_RESOURCE_STREAMER(dev_priv); > > > + value = 0; > > > break; > > > case I915_PARAM_HAS_POOLED_EU: > > > value = HAS_POOLED_EU(dev_priv); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 01dd29837233..dddb886fa06c 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > > > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis > > > sion() > > > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > > > > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)- > > > >info.has_resource_streamer) > > > - > > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > > > > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > index 1932bc227942..ca21a08b2be9 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > > if (!eb.engine) > > > return -EINVAL; > > > > > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > > > - DRM_DEBUG("RS is only allowed for Haswell and > > > Gen8 - Gen10\n"); > > > - return -EINVAL; > > > - } > > > - if (eb.engine->id != RCS) { > > > - DRM_DEBUG("RS is not available on %s\n", > > > - eb.engine->name); > > > - return -EINVAL; > > > - } > > > - > > > - eb.batch_flags |= I915_DISPATCH_RS; > > > - } > > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > > > + return -EINVAL; > > > > > > if (args->flags & I915_EXEC_FENCE_IN) { > > > in_fence = sync_file_get_fence(lower_32_bits(args- > > > >rsvd2)); > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > > b/drivers/gpu/drm/i915/i915_pci.c > > > index c03ba0fe0845..7af19097dce8 100644 > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > @@ -360,7 +360,6 @@ static const struct intel_device_info > > > intel_valleyview_info = { > > > .has_ddi = 1, \ > > > .has_fpga_dbg = 1, \ > > > .has_psr = 1, \ > > > - .has_resource_streamer = 1, \ > > > .has_dp_mst = 1, \ > > > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > > > .has_runtime_pm = 1 > > > @@ -433,7 +432,6 @@ static const struct intel_device_info > > > intel_cherryview_info = { > > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > > > .has_64bit_reloc = 1, > > > .has_runtime_pm = 1, > > > - .has_resource_streamer = 1, > > > .has_rc6 = 1, > > > .has_logical_ring_contexts = 1, > > > .has_gmch_display = 1, > > > @@ -506,7 +504,6 @@ static const struct intel_device_info > > > intel_skylake_gt4_info = { > > > .has_runtime_pm = 1, \ > > > .has_pooled_eu = 0, \ > > > .has_csr = 1, \ > > > - .has_resource_streamer = 1, \ > > > .has_rc6 = 1, \ > > > .has_dp_mst = 1, \ > > > .has_logical_ring_contexts = 1, \ > > > @@ -593,7 +590,6 @@ static const struct intel_device_info > > > intel_cannonlake_info = { > > > GEN(11), \ > > > .ddb_size = 2048, \ > > > .has_csr = 0, \ > > > - .has_resource_streamer = 0, \ > > > .has_logical_ring_elsq = 1 > > > > > > static const struct intel_device_info intel_icelake_11_info = { > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h > > > b/drivers/gpu/drm/i915/intel_device_info.h > > > index 633f9fbf72ea..6814cc7dfcd3 100644 > > > --- a/drivers/gpu/drm/i915/intel_device_info.h > > > +++ b/drivers/gpu/drm/i915/intel_device_info.h > > > @@ -103,7 +103,6 @@ enum intel_platform { > > > func(has_psr); \ > > > func(has_rc6); \ > > > func(has_rc6p); \ > > > - func(has_resource_streamer); \ > > > func(has_runtime_pm); \ > > > func(has_snoop); \ > > > func(unfenced_needs_alignment); \ > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > > b/drivers/gpu/drm/i915/intel_lrc.c > > > index 35d37af0cb9a..ad10fb7b9668 100644 > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request > > > *rq, > > > > > > /* FIXME(BDW): Address space and security selectors. */ > > > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > > > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > > > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : > > > 0); > > > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > > > *cs++ = lower_32_bits(offset); > > > *cs++ = upper_32_bits(offset); > > > > > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > > > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > > > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > > > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > > > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > > > - (HAS_RESOURCE_STREAMER(dev_priv) ? > > > - CTX_CTRL_RS_CTX_ENABLE : 0))); > > > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > > > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > > > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > > > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > index f4bd185c9369..e5fcbbc2abbc 100644 > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > > > return PTR_ERR(cs); > > > > > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & > > > I915_DISPATCH_SECURE ? > > > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > > > - (dispatch_flags & I915_DISPATCH_RS ? > > > - MI_BATCH_RESOURCE_STREAMER : 0); > > > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > > > /* bit0-7 is the length on GEN6+ */ > > > *cs++ = offset; > > > intel_ring_advance(rq, cs); > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > index d1eee08e5f6b..a2c1d7044f2a 100644 > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > @@ -467,7 +467,6 @@ struct intel_engine_cs { > > > unsigned int dispatch_flags); > > > #define I915_DISPATCH_SECURE BIT(0) > > > #define I915_DISPATCH_PINNED BIT(1) > > > -#define I915_DISPATCH_RS BIT(2) > > > void (*emit_breadcrumb)(struct i915_request *rq, > > > u32 *cs); > > > int emit_breadcrumb_sz; > > > > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jul 12, 2018 at 04:48:42PM -0700, Rodrigo Vivi wrote: > On Thu, Jul 12, 2018 at 11:28:25PM +0000, De Marchi, Lucas wrote: > > On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote: > > > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote: > > > > After disabling resource streamer on ICL (due to it actually not > > > > existing there), I got feedback that there have been some experimental > > > > patches for mesa to use it, but nothing ever landed nor shipped. > > > > > > > > This is a tentative to remove it from kernel keeping the uapi defines > > > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > > > > some platforms - Daniele mentioned there's possible performance regression > > > > in gem_latency if this bit is not set. On this patch, I'm just removing > > > > it in order to get proper numbers. > > > > > > Since I see below 3 occurrences of > > > - .has_resource_streamer = 1 > > > > > > I feel this commit message lacks of the reason why we are removing > > > this features for the platforms that already had it. > > > > /me confused. Isn't the first paragraph in the commit message exactly that? > > /me confused now... For me the first paragraph tells that after removing > from ICL mesa might want to use it on ICL. > > second paragraph tells that you are removing the support but keeping the uapi. > > what I couldn't find is the explanation why exactly you are removing this > feature. Is this useless? No body using it? It is buggy? It is deprecated? why? I guess a small change should fix the meaning of the first para: "... there have been some experimental patches for mesa to use it years ago, but nothing ever landed or shipped because there was no performance improvement." These experimental patches long stopped existing :-) Also note that the uapi doesn't stay around, only the #defines (we can't ever reuse them for anything else than RS, so need to keep them reserved). Cheers, Daniel > > > > > Lucas De Marchi > > > > > > > > > > > > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > > > > - don't bother trying to document removed params on uapi header: > > > > applications should know that from the query. > > > > (from Chris) > > > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > > > drivers/gpu/drm/i915/i915_drv.h | 2 -- > > > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > > > > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > > > > drivers/gpu/drm/i915/intel_device_info.h | 1 - > > > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > > > > 8 files changed, 6 insertions(+), 30 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 3eba3d1ab5b8..10423e2c1176 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, > > > > void *data, > > > > value = 2; > > > > break; > > > > case I915_PARAM_HAS_RESOURCE_STREAMER: > > > > - value = HAS_RESOURCE_STREAMER(dev_priv); > > > > + value = 0; > > > > break; > > > > case I915_PARAM_HAS_POOLED_EU: > > > > value = HAS_POOLED_EU(dev_priv); > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > b/drivers/gpu/drm/i915/i915_drv.h > > > > index 01dd29837233..dddb886fa06c 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > > > > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis > > > > sion() > > > > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > > > > > > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)- > > > > >info.has_resource_streamer) > > > > - > > > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > > > > > > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > index 1932bc227942..ca21a08b2be9 100644 > > > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > > > if (!eb.engine) > > > > return -EINVAL; > > > > > > > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > > > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > > > > - DRM_DEBUG("RS is only allowed for Haswell and > > > > Gen8 - Gen10\n"); > > > > - return -EINVAL; > > > > - } > > > > - if (eb.engine->id != RCS) { > > > > - DRM_DEBUG("RS is not available on %s\n", > > > > - eb.engine->name); > > > > - return -EINVAL; > > > > - } > > > > - > > > > - eb.batch_flags |= I915_DISPATCH_RS; > > > > - } > > > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > > > > + return -EINVAL; > > > > > > > > if (args->flags & I915_EXEC_FENCE_IN) { > > > > in_fence = sync_file_get_fence(lower_32_bits(args- > > > > >rsvd2)); > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > > > b/drivers/gpu/drm/i915/i915_pci.c > > > > index c03ba0fe0845..7af19097dce8 100644 > > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > > @@ -360,7 +360,6 @@ static const struct intel_device_info > > > > intel_valleyview_info = { > > > > .has_ddi = 1, \ > > > > .has_fpga_dbg = 1, \ > > > > .has_psr = 1, \ > > > > - .has_resource_streamer = 1, \ > > > > .has_dp_mst = 1, \ > > > > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > > > > .has_runtime_pm = 1 > > > > @@ -433,7 +432,6 @@ static const struct intel_device_info > > > > intel_cherryview_info = { > > > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > > > > .has_64bit_reloc = 1, > > > > .has_runtime_pm = 1, > > > > - .has_resource_streamer = 1, > > > > .has_rc6 = 1, > > > > .has_logical_ring_contexts = 1, > > > > .has_gmch_display = 1, > > > > @@ -506,7 +504,6 @@ static const struct intel_device_info > > > > intel_skylake_gt4_info = { > > > > .has_runtime_pm = 1, \ > > > > .has_pooled_eu = 0, \ > > > > .has_csr = 1, \ > > > > - .has_resource_streamer = 1, \ > > > > .has_rc6 = 1, \ > > > > .has_dp_mst = 1, \ > > > > .has_logical_ring_contexts = 1, \ > > > > @@ -593,7 +590,6 @@ static const struct intel_device_info > > > > intel_cannonlake_info = { > > > > GEN(11), \ > > > > .ddb_size = 2048, \ > > > > .has_csr = 0, \ > > > > - .has_resource_streamer = 0, \ > > > > .has_logical_ring_elsq = 1 > > > > > > > > static const struct intel_device_info intel_icelake_11_info = { > > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h > > > > b/drivers/gpu/drm/i915/intel_device_info.h > > > > index 633f9fbf72ea..6814cc7dfcd3 100644 > > > > --- a/drivers/gpu/drm/i915/intel_device_info.h > > > > +++ b/drivers/gpu/drm/i915/intel_device_info.h > > > > @@ -103,7 +103,6 @@ enum intel_platform { > > > > func(has_psr); \ > > > > func(has_rc6); \ > > > > func(has_rc6p); \ > > > > - func(has_resource_streamer); \ > > > > func(has_runtime_pm); \ > > > > func(has_snoop); \ > > > > func(unfenced_needs_alignment); \ > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > > > b/drivers/gpu/drm/i915/intel_lrc.c > > > > index 35d37af0cb9a..ad10fb7b9668 100644 > > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request > > > > *rq, > > > > > > > > /* FIXME(BDW): Address space and security selectors. */ > > > > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > > > > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > > > > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : > > > > 0); > > > > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > > > > *cs++ = lower_32_bits(offset); > > > > *cs++ = upper_32_bits(offset); > > > > > > > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > > > > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > > > > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > > > > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > > > > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > > > > - (HAS_RESOURCE_STREAMER(dev_priv) ? > > > > - CTX_CTRL_RS_CTX_ENABLE : 0))); > > > > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > > > > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > > > > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > > > > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > index f4bd185c9369..e5fcbbc2abbc 100644 > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > > > > return PTR_ERR(cs); > > > > > > > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & > > > > I915_DISPATCH_SECURE ? > > > > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > > > > - (dispatch_flags & I915_DISPATCH_RS ? > > > > - MI_BATCH_RESOURCE_STREAMER : 0); > > > > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > > > > /* bit0-7 is the length on GEN6+ */ > > > > *cs++ = offset; > > > > intel_ring_advance(rq, cs); > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > index d1eee08e5f6b..a2c1d7044f2a 100644 > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > @@ -467,7 +467,6 @@ struct intel_engine_cs { > > > > unsigned int dispatch_flags); > > > > #define I915_DISPATCH_SECURE BIT(0) > > > > #define I915_DISPATCH_PINNED BIT(1) > > > > -#define I915_DISPATCH_RS BIT(2) > > > > void (*emit_breadcrumb)(struct i915_request *rq, > > > > u32 *cs); > > > > int emit_breadcrumb_sz; > > > > > > > > -- > > > > 2.17.1 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jul 13, 2018 at 10:46:26AM +0200, Daniel Vetter wrote: > On Thu, Jul 12, 2018 at 04:48:42PM -0700, Rodrigo Vivi wrote: > > On Thu, Jul 12, 2018 at 11:28:25PM +0000, De Marchi, Lucas wrote: > > > On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote: > > > > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote: > > > > > After disabling resource streamer on ICL (due to it actually not > > > > > existing there), I got feedback that there have been some experimental > > > > > patches for mesa to use it, but nothing ever landed nor shipped. > > > > > > > > > > This is a tentative to remove it from kernel keeping the uapi defines > > > > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on > > > > > some platforms - Daniele mentioned there's possible performance regression > > > > > in gem_latency if this bit is not set. On this patch, I'm just removing > > > > > it in order to get proper numbers. > > > > > > > > Since I see below 3 occurrences of > > > > - .has_resource_streamer = 1 > > > > > > > > I feel this commit message lacks of the reason why we are removing > > > > this features for the platforms that already had it. > > > > > > /me confused. Isn't the first paragraph in the commit message exactly that? > > > > /me confused now... For me the first paragraph tells that after removing > > from ICL mesa might want to use it on ICL. nah, the *hw* block doesn't exist on ICL, a patch to use it there would be garbage, not experimental :) > > > > second paragraph tells that you are removing the support but keeping the uapi. by "use it" I was referring to the feature itself. > > > > what I couldn't find is the explanation why exactly you are removing this > > feature. Is this useless? No body using it? It is buggy? It is deprecated? why? > > I guess a small change should fix the meaning of the first para: > > "... there have been some experimental patches for mesa to use it years > ago, but nothing ever landed or shipped because there was no performance > improvement." yep, it's better. And s/it/RS/ would help clear out the confusion on my badly worded paragraph. > > These experimental patches long stopped existing :-) > > Also note that the uapi doesn't stay around, only the #defines (we can't > ever reuse them for anything else than RS, so need to keep them reserved). I think I got it right on this one saying "keeping the uapi *defines* around for compatibility. Lucas De Marchi > > Cheers, Daniel > > > > > > > > > Lucas De Marchi > > > > > > > > > > > > > > > > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH > > > > > - don't bother trying to document removed params on uapi header: > > > > > applications should know that from the query. > > > > > (from Chris) > > > > > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_drv.c | 2 +- > > > > > drivers/gpu/drm/i915/i915_drv.h | 2 -- > > > > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- > > > > > drivers/gpu/drm/i915/i915_pci.c | 4 ---- > > > > > drivers/gpu/drm/i915/intel_device_info.h | 1 - > > > > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- > > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > > > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > > > > > 8 files changed, 6 insertions(+), 30 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > > index 3eba3d1ab5b8..10423e2c1176 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, > > > > > void *data, > > > > > value = 2; > > > > > break; > > > > > case I915_PARAM_HAS_RESOURCE_STREAMER: > > > > > - value = HAS_RESOURCE_STREAMER(dev_priv); > > > > > + value = 0; > > > > > break; > > > > > case I915_PARAM_HAS_POOLED_EU: > > > > > value = HAS_POOLED_EU(dev_priv); > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > > b/drivers/gpu/drm/i915/i915_drv.h > > > > > index 01dd29837233..dddb886fa06c 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) > > > > > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis > > > > > sion() > > > > > #define USES_HUC(dev_priv) intel_uc_is_using_huc() > > > > > > > > > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)- > > > > > >info.has_resource_streamer) > > > > > - > > > > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > > > > > > > > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80 > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > > index 1932bc227942..ca21a08b2be9 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > > > > if (!eb.engine) > > > > > return -EINVAL; > > > > > > > > > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > > > > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) { > > > > > - DRM_DEBUG("RS is only allowed for Haswell and > > > > > Gen8 - Gen10\n"); > > > > > - return -EINVAL; > > > > > - } > > > > > - if (eb.engine->id != RCS) { > > > > > - DRM_DEBUG("RS is not available on %s\n", > > > > > - eb.engine->name); > > > > > - return -EINVAL; > > > > > - } > > > > > - > > > > > - eb.batch_flags |= I915_DISPATCH_RS; > > > > > - } > > > > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER) > > > > > + return -EINVAL; > > > > > > > > > > if (args->flags & I915_EXEC_FENCE_IN) { > > > > > in_fence = sync_file_get_fence(lower_32_bits(args- > > > > > >rsvd2)); > > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > > > > b/drivers/gpu/drm/i915/i915_pci.c > > > > > index c03ba0fe0845..7af19097dce8 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > > > @@ -360,7 +360,6 @@ static const struct intel_device_info > > > > > intel_valleyview_info = { > > > > > .has_ddi = 1, \ > > > > > .has_fpga_dbg = 1, \ > > > > > .has_psr = 1, \ > > > > > - .has_resource_streamer = 1, \ > > > > > .has_dp_mst = 1, \ > > > > > .has_rc6p = 0 /* RC6p removed-by HSW */, \ > > > > > .has_runtime_pm = 1 > > > > > @@ -433,7 +432,6 @@ static const struct intel_device_info > > > > > intel_cherryview_info = { > > > > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > > > > > .has_64bit_reloc = 1, > > > > > .has_runtime_pm = 1, > > > > > - .has_resource_streamer = 1, > > > > > .has_rc6 = 1, > > > > > .has_logical_ring_contexts = 1, > > > > > .has_gmch_display = 1, > > > > > @@ -506,7 +504,6 @@ static const struct intel_device_info > > > > > intel_skylake_gt4_info = { > > > > > .has_runtime_pm = 1, \ > > > > > .has_pooled_eu = 0, \ > > > > > .has_csr = 1, \ > > > > > - .has_resource_streamer = 1, \ > > > > > .has_rc6 = 1, \ > > > > > .has_dp_mst = 1, \ > > > > > .has_logical_ring_contexts = 1, \ > > > > > @@ -593,7 +590,6 @@ static const struct intel_device_info > > > > > intel_cannonlake_info = { > > > > > GEN(11), \ > > > > > .ddb_size = 2048, \ > > > > > .has_csr = 0, \ > > > > > - .has_resource_streamer = 0, \ > > > > > .has_logical_ring_elsq = 1 > > > > > > > > > > static const struct intel_device_info intel_icelake_11_info = { > > > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h > > > > > b/drivers/gpu/drm/i915/intel_device_info.h > > > > > index 633f9fbf72ea..6814cc7dfcd3 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_device_info.h > > > > > +++ b/drivers/gpu/drm/i915/intel_device_info.h > > > > > @@ -103,7 +103,6 @@ enum intel_platform { > > > > > func(has_psr); \ > > > > > func(has_rc6); \ > > > > > func(has_rc6p); \ > > > > > - func(has_resource_streamer); \ > > > > > func(has_runtime_pm); \ > > > > > func(has_snoop); \ > > > > > func(unfenced_needs_alignment); \ > > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > > > > b/drivers/gpu/drm/i915/intel_lrc.c > > > > > index 35d37af0cb9a..ad10fb7b9668 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > > > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request > > > > > *rq, > > > > > > > > > > /* FIXME(BDW): Address space and security selectors. */ > > > > > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > > > > > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > > > > > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : > > > > > 0); > > > > > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); > > > > > *cs++ = lower_32_bits(offset); > > > > > *cs++ = upper_32_bits(offset); > > > > > > > > > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, > > > > > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), > > > > > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | > > > > > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | > > > > > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | > > > > > - (HAS_RESOURCE_STREAMER(dev_priv) ? > > > > > - CTX_CTRL_RS_CTX_ENABLE : 0))); > > > > > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); > > > > > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); > > > > > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); > > > > > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); > > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > > index f4bd185c9369..e5fcbbc2abbc 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > > > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, > > > > > return PTR_ERR(cs); > > > > > > > > > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & > > > > > I915_DISPATCH_SECURE ? > > > > > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | > > > > > - (dispatch_flags & I915_DISPATCH_RS ? > > > > > - MI_BATCH_RESOURCE_STREAMER : 0); > > > > > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); > > > > > /* bit0-7 is the length on GEN6+ */ > > > > > *cs++ = offset; > > > > > intel_ring_advance(rq, cs); > > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > > b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > > index d1eee08e5f6b..a2c1d7044f2a 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > > > > > @@ -467,7 +467,6 @@ struct intel_engine_cs { > > > > > unsigned int dispatch_flags); > > > > > #define I915_DISPATCH_SECURE BIT(0) > > > > > #define I915_DISPATCH_PINNED BIT(1) > > > > > -#define I915_DISPATCH_RS BIT(2) > > > > > void (*emit_breadcrumb)(struct i915_request *rq, > > > > > u32 *cs); > > > > > int emit_breadcrumb_sz; > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3eba3d1ab5b8..10423e2c1176 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, value = 2; break; case I915_PARAM_HAS_RESOURCE_STREAMER: - value = HAS_RESOURCE_STREAMER(dev_priv); + value = 0; break; case I915_PARAM_HAS_POOLED_EU: value = HAS_POOLED_EU(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 01dd29837233..dddb886fa06c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv) #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission() #define USES_HUC(dev_priv) intel_uc_is_using_huc() -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) - #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) #define INTEL_PCH_DEVICE_ID_MASK 0xff80 diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 1932bc227942..ca21a08b2be9 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (!eb.engine) return -EINVAL; - if (args->flags & I915_EXEC_RESOURCE_STREAMER) { - if (!HAS_RESOURCE_STREAMER(eb.i915)) { - DRM_DEBUG("RS is only allowed for Haswell and Gen8 - Gen10\n"); - return -EINVAL; - } - if (eb.engine->id != RCS) { - DRM_DEBUG("RS is not available on %s\n", - eb.engine->name); - return -EINVAL; - } - - eb.batch_flags |= I915_DISPATCH_RS; - } + if (args->flags & I915_EXEC_RESOURCE_STREAMER) + return -EINVAL; if (args->flags & I915_EXEC_FENCE_IN) { in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index c03ba0fe0845..7af19097dce8 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -360,7 +360,6 @@ static const struct intel_device_info intel_valleyview_info = { .has_ddi = 1, \ .has_fpga_dbg = 1, \ .has_psr = 1, \ - .has_resource_streamer = 1, \ .has_dp_mst = 1, \ .has_rc6p = 0 /* RC6p removed-by HSW */, \ .has_runtime_pm = 1 @@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, .has_64bit_reloc = 1, .has_runtime_pm = 1, - .has_resource_streamer = 1, .has_rc6 = 1, .has_logical_ring_contexts = 1, .has_gmch_display = 1, @@ -506,7 +504,6 @@ static const struct intel_device_info intel_skylake_gt4_info = { .has_runtime_pm = 1, \ .has_pooled_eu = 0, \ .has_csr = 1, \ - .has_resource_streamer = 1, \ .has_rc6 = 1, \ .has_dp_mst = 1, \ .has_logical_ring_contexts = 1, \ @@ -593,7 +590,6 @@ static const struct intel_device_info intel_cannonlake_info = { GEN(11), \ .ddb_size = 2048, \ .has_csr = 0, \ - .has_resource_streamer = 0, \ .has_logical_ring_elsq = 1 static const struct intel_device_info intel_icelake_11_info = { diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 633f9fbf72ea..6814cc7dfcd3 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -103,7 +103,6 @@ enum intel_platform { func(has_psr); \ func(has_rc6); \ func(has_rc6p); \ - func(has_resource_streamer); \ func(has_runtime_pm); \ func(has_snoop); \ func(unfenced_needs_alignment); \ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 35d37af0cb9a..ad10fb7b9668 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request *rq, /* FIXME(BDW): Address space and security selectors. */ *cs++ = MI_BATCH_BUFFER_START_GEN8 | - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0); + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)); *cs++ = lower_32_bits(offset); *cs++ = upper_32_bits(offset); @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs, CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine), _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) | - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | - (HAS_RESOURCE_STREAMER(dev_priv) ? - CTX_CTRL_RS_CTX_ENABLE : 0))); + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH)); CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0); CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0); CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index f4bd185c9369..e5fcbbc2abbc 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq, return PTR_ERR(cs); *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags & I915_DISPATCH_SECURE ? - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) | - (dispatch_flags & I915_DISPATCH_RS ? - MI_BATCH_RESOURCE_STREAMER : 0); + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW); /* bit0-7 is the length on GEN6+ */ *cs++ = offset; intel_ring_advance(rq, cs); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index d1eee08e5f6b..a2c1d7044f2a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -467,7 +467,6 @@ struct intel_engine_cs { unsigned int dispatch_flags); #define I915_DISPATCH_SECURE BIT(0) #define I915_DISPATCH_PINNED BIT(1) -#define I915_DISPATCH_RS BIT(2) void (*emit_breadcrumb)(struct i915_request *rq, u32 *cs); int emit_breadcrumb_sz;
After disabling resource streamer on ICL (due to it actually not existing there), I got feedback that there have been some experimental patches for mesa to use it, but nothing ever landed nor shipped. This is a tentative to remove it from kernel keeping the uapi defines around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on some platforms - Daniele mentioned there's possible performance regression in gem_latency if this bit is not set. On this patch, I'm just removing it in order to get proper numbers. v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH - don't bother trying to document removed params on uapi header: applications should know that from the query. (from Chris) Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++------------- drivers/gpu/drm/i915/i915_pci.c | 4 ---- drivers/gpu/drm/i915/intel_device_info.h | 1 - drivers/gpu/drm/i915/intel_lrc.c | 7 ++----- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 8 files changed, 6 insertions(+), 30 deletions(-)