* [PATCH 0/2] Kill resource streamer
@ 2018-07-10 0:06 Lucas De Marchi
2018-07-10 0:06 ` [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Lucas De Marchi
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: Lucas De Marchi @ 2018-07-10 0:06 UTC (permalink / raw)
To: intel-gfx; +Cc: daniel.vetter
First patch is makes sense for ICL since it doesn't have resource
streamer. It has been a feature that was never properly tested/used so
also kill it on second patch - this is a tentative/rfc: see commit
message.
Lucas De Marchi (2):
drm/i915/icl: move has_resource_streamer to GEN11_FEATURES
drm/i915: kill resource streamer
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 | 8 ++------
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
include/uapi/drm/i915_drm.h | 1 +
tools/include/uapi/drm/i915_drm.h | 1 +
10 files changed, 8 insertions(+), 31 deletions(-)
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi @ 2018-07-10 0:06 ` Lucas De Marchi 2018-07-10 16:52 ` Daniele Ceraolo Spurio 2018-07-10 0:06 ` [PATCH 2/2] drm/i915: kill resource streamer Lucas De Marchi ` (5 subsequent siblings) 6 siblings, 1 reply; 20+ messages in thread From: Lucas De Marchi @ 2018-07-10 0:06 UTC (permalink / raw) To: intel-gfx; +Cc: daniel.vetter Resource streamer has been removed on GEN11 so move it to the FEATURES macro. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_pci.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3f0c612d42e7..1932bc227942 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2223,7 +2223,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (args->flags & I915_EXEC_RESOURCE_STREAMER) { if (!HAS_RESOURCE_STREAMER(eb.i915)) { - DRM_DEBUG("RS is only allowed for Haswell, Gen8 and above\n"); + DRM_DEBUG("RS is only allowed for Haswell and Gen8 - Gen10\n"); return -EINVAL; } if (eb.engine->id != RCS) { diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 55543f1b0236..c03ba0fe0845 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -593,13 +593,13 @@ 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 = { GEN11_FEATURES, PLATFORM(INTEL_ICELAKE), .is_alpha_support = 1, - .has_resource_streamer = 0, .ring_mask = RENDER_RING | BLT_RING | VEBOX_RING | BSD_RING | BSD3_RING, }; -- 2.17.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES 2018-07-10 0:06 ` [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Lucas De Marchi @ 2018-07-10 16:52 ` Daniele Ceraolo Spurio 0 siblings, 0 replies; 20+ messages in thread From: Daniele Ceraolo Spurio @ 2018-07-10 16:52 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx; +Cc: daniel.vetter On 09/07/18 17:06, Lucas De Marchi wrote: > Resource streamer has been removed on GEN11 so move it to the FEATURES > macro. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/i915_pci.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 3f0c612d42e7..1932bc227942 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2223,7 +2223,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > if (args->flags & I915_EXEC_RESOURCE_STREAMER) { > if (!HAS_RESOURCE_STREAMER(eb.i915)) { > - DRM_DEBUG("RS is only allowed for Haswell, Gen8 and above\n"); > + DRM_DEBUG("RS is only allowed for Haswell and Gen8 - Gen10\n"); > return -EINVAL; > } > if (eb.engine->id != RCS) { > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 55543f1b0236..c03ba0fe0845 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -593,13 +593,13 @@ 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 = { > GEN11_FEATURES, > PLATFORM(INTEL_ICELAKE), > .is_alpha_support = 1, > - .has_resource_streamer = 0, > .ring_mask = RENDER_RING | BLT_RING | VEBOX_RING | BSD_RING | BSD3_RING, > }; > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: kill resource streamer 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi 2018-07-10 0:06 ` [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Lucas De Marchi @ 2018-07-10 0:06 ` Lucas De Marchi 2018-07-10 9:51 ` Chris Wilson 2018-07-13 8:51 ` [PATCH 2/2] " Daniel Vetter 2018-07-10 0:20 ` ✗ Fi.CI.SPARSE: warning for Kill " Patchwork ` (4 subsequent siblings) 6 siblings, 2 replies; 20+ messages in thread From: Lucas De Marchi @ 2018-07-10 0:06 UTC (permalink / raw) To: intel-gfx; +Cc: daniel.vetter 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. 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 | 8 ++------ drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - include/uapi/drm/i915_drm.h | 1 + tools/include/uapi/drm/i915_drm.h | 1 + 10 files changed, 8 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 0db3c83cce29..6715e37ee66b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -370,7 +370,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 bec0a2796c37..bd69e4b120a8 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..a5eec97a40fe 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) + DRM_DEBUG("RS is removed from all Gens: ignoring.\n"); 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 ab89dabc2965..843a940bcc58 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2138,8 +2138,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); @@ -2657,10 +2656,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))); + CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)); 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 700f94c371b3..a76b974c398d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1979,9 +1979,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 ce6cc2a6cf7a..efec5b14ddb0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -472,7 +472,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; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7f5634ce8e88..179e0477dbc6 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_SUBSLICE_TOTAL 33 #define I915_PARAM_EU_TOTAL 34 #define I915_PARAM_HAS_GPU_RESET 35 +/* Deprecated: do not use */ #define I915_PARAM_HAS_RESOURCE_STREAMER 36 #define I915_PARAM_HAS_EXEC_SOFTPIN 37 #define I915_PARAM_HAS_POOLED_EU 38 diff --git a/tools/include/uapi/drm/i915_drm.h b/tools/include/uapi/drm/i915_drm.h index 7f5634ce8e88..179e0477dbc6 100644 --- a/tools/include/uapi/drm/i915_drm.h +++ b/tools/include/uapi/drm/i915_drm.h @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_SUBSLICE_TOTAL 33 #define I915_PARAM_EU_TOTAL 34 #define I915_PARAM_HAS_GPU_RESET 35 +/* Deprecated: do not use */ #define I915_PARAM_HAS_RESOURCE_STREAMER 36 #define I915_PARAM_HAS_EXEC_SOFTPIN 37 #define I915_PARAM_HAS_POOLED_EU 38 -- 2.17.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: kill resource streamer 2018-07-10 0:06 ` [PATCH 2/2] drm/i915: kill resource streamer Lucas De Marchi @ 2018-07-10 9:51 ` Chris Wilson 2018-07-11 17:29 ` Lucas De Marchi 2018-07-13 8:51 ` [PATCH 2/2] " Daniel Vetter 1 sibling, 1 reply; 20+ messages in thread From: Chris Wilson @ 2018-07-10 9:51 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx; +Cc: daniel.vetter Quoting Lucas De Marchi (2018-07-10 01:06:58) > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 1932bc227942..a5eec97a40fe 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) > + DRM_DEBUG("RS is removed from all Gens: ignoring.\n"); return -EINVAL; If userspace set up its batch on the assumption that RS is enabled, it will fail. > @@ -2657,10 +2656,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 | Bring back INHIBIT_SYNC_CTX_SWITCH. > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 7f5634ce8e88..179e0477dbc6 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_SUBSLICE_TOTAL 33 > #define I915_PARAM_EU_TOTAL 34 > #define I915_PARAM_HAS_GPU_RESET 35 > +/* Deprecated: do not use */ Don't bother. As written it implies all subsequent param are deprecated. Userspace knows simply by the query return. > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > #define I915_PARAM_HAS_POOLED_EU 38 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: kill resource streamer 2018-07-10 9:51 ` Chris Wilson @ 2018-07-11 17:29 ` Lucas De Marchi 2018-07-12 21:02 ` [PATCH v2] " Lucas De Marchi 0 siblings, 1 reply; 20+ messages in thread From: Lucas De Marchi @ 2018-07-11 17:29 UTC (permalink / raw) To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Lucas De Marchi On Tue, Jul 10, 2018 at 2:51 AM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Lucas De Marchi (2018-07-10 01:06:58) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 1932bc227942..a5eec97a40fe 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) > > + DRM_DEBUG("RS is removed from all Gens: ignoring.\n"); > > return -EINVAL; > > If userspace set up its batch on the assumption that RS is enabled, it > will fail. ok, > > > @@ -2657,10 +2656,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 | > > Bring back INHIBIT_SYNC_CTX_SWITCH. ugh, thanks for catching that. > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index 7f5634ce8e88..179e0477dbc6 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > > #define I915_PARAM_SUBSLICE_TOTAL 33 > > #define I915_PARAM_EU_TOTAL 34 > > #define I915_PARAM_HAS_GPU_RESET 35 > > +/* Deprecated: do not use */ > > Don't bother. As written it implies all subsequent param are deprecated. > Userspace knows simply by the query return. ok. I'll respin a new version. Lucas De Marchi > > > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > > #define I915_PARAM_HAS_POOLED_EU 38 > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] drm/i915: kill resource streamer 2018-07-11 17:29 ` Lucas De Marchi @ 2018-07-12 21:02 ` Lucas De Marchi 2018-07-12 22:58 ` Daniele Ceraolo Spurio 2018-07-12 23:06 ` Rodrigo Vivi 0 siblings, 2 replies; 20+ messages in thread From: Lucas De Marchi @ 2018-07-12 21:02 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter 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(-) 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-12 21:02 ` [PATCH v2] " Lucas De Marchi @ 2018-07-12 22:58 ` Daniele Ceraolo Spurio 2018-07-12 23:06 ` Rodrigo Vivi 1 sibling, 0 replies; 20+ messages in thread From: Daniele Ceraolo Spurio @ 2018-07-12 22:58 UTC (permalink / raw) To: Lucas De Marchi, intel-gfx; +Cc: Daniel Vetter 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; > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-12 21:02 ` [PATCH v2] " Lucas De Marchi 2018-07-12 22:58 ` Daniele Ceraolo Spurio @ 2018-07-12 23:06 ` Rodrigo Vivi 2018-07-12 23:28 ` De Marchi, Lucas 1 sibling, 1 reply; 20+ messages in thread From: Rodrigo Vivi @ 2018-07-12 23:06 UTC (permalink / raw) To: Lucas De Marchi; +Cc: Daniel Vetter, intel-gfx 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-12 23:06 ` Rodrigo Vivi @ 2018-07-12 23:28 ` De Marchi, Lucas 2018-07-12 23:48 ` Rodrigo Vivi 0 siblings, 1 reply; 20+ messages in thread From: De Marchi, Lucas @ 2018-07-12 23:28 UTC (permalink / raw) To: Vivi, Rodrigo; +Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-12 23:28 ` De Marchi, Lucas @ 2018-07-12 23:48 ` Rodrigo Vivi 2018-07-13 8:46 ` Daniel Vetter 0 siblings, 1 reply; 20+ messages in thread From: Rodrigo Vivi @ 2018-07-12 23:48 UTC (permalink / raw) To: De Marchi, Lucas; +Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-12 23:48 ` Rodrigo Vivi @ 2018-07-13 8:46 ` Daniel Vetter 2018-07-13 14:17 ` Lucas De Marchi 0 siblings, 1 reply; 20+ messages in thread From: Daniel Vetter @ 2018-07-13 8:46 UTC (permalink / raw) To: Rodrigo Vivi Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, De Marchi, Lucas 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: kill resource streamer 2018-07-13 8:46 ` Daniel Vetter @ 2018-07-13 14:17 ` Lucas De Marchi 0 siblings, 0 replies; 20+ messages in thread From: Lucas De Marchi @ 2018-07-13 14:17 UTC (permalink / raw) To: Daniel Vetter Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, Rodrigo Vivi 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: kill resource streamer 2018-07-10 0:06 ` [PATCH 2/2] drm/i915: kill resource streamer Lucas De Marchi 2018-07-10 9:51 ` Chris Wilson @ 2018-07-13 8:51 ` Daniel Vetter 2018-07-13 14:11 ` Lucas De Marchi 1 sibling, 1 reply; 20+ messages in thread From: Daniel Vetter @ 2018-07-13 8:51 UTC (permalink / raw) To: Lucas De Marchi; +Cc: daniel.vetter, intel-gfx On Mon, Jul 09, 2018 at 05:06:58PM -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. > > 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 | 8 ++------ > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > include/uapi/drm/i915_drm.h | 1 + > tools/include/uapi/drm/i915_drm.h | 1 + > 10 files changed, 8 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 0db3c83cce29..6715e37ee66b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -370,7 +370,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 bec0a2796c37..bd69e4b120a8 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..a5eec97a40fe 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) > + DRM_DEBUG("RS is removed from all Gens: ignoring.\n"); Still missing the return -EINVAL here. Needs an igt update I think. > > 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 ab89dabc2965..843a940bcc58 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2138,8 +2138,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); > > @@ -2657,10 +2656,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))); I'd hard-code this to disable the RS instead of removing it outright. With these 2 nits addressed and the commit message augmented as I propose in my other reply: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)); > 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 700f94c371b3..a76b974c398d 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1979,9 +1979,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 ce6cc2a6cf7a..efec5b14ddb0 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -472,7 +472,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; > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 7f5634ce8e88..179e0477dbc6 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_SUBSLICE_TOTAL 33 > #define I915_PARAM_EU_TOTAL 34 > #define I915_PARAM_HAS_GPU_RESET 35 > +/* Deprecated: do not use */ > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > #define I915_PARAM_HAS_POOLED_EU 38 > diff --git a/tools/include/uapi/drm/i915_drm.h b/tools/include/uapi/drm/i915_drm.h > index 7f5634ce8e88..179e0477dbc6 100644 > --- a/tools/include/uapi/drm/i915_drm.h > +++ b/tools/include/uapi/drm/i915_drm.h > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_SUBSLICE_TOTAL 33 > #define I915_PARAM_EU_TOTAL 34 > #define I915_PARAM_HAS_GPU_RESET 35 > +/* Deprecated: do not use */ > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > #define I915_PARAM_HAS_POOLED_EU 38 > -- > 2.17.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: kill resource streamer 2018-07-13 8:51 ` [PATCH 2/2] " Daniel Vetter @ 2018-07-13 14:11 ` Lucas De Marchi 0 siblings, 0 replies; 20+ messages in thread From: Lucas De Marchi @ 2018-07-13 14:11 UTC (permalink / raw) To: Daniel Vetter; +Cc: daniel.vetter, intel-gfx On Fri, Jul 13, 2018 at 10:51:16AM +0200, Daniel Vetter wrote: > On Mon, Jul 09, 2018 at 05:06:58PM -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. > > > > 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 | 8 ++------ > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - > > include/uapi/drm/i915_drm.h | 1 + > > tools/include/uapi/drm/i915_drm.h | 1 + > > 10 files changed, 8 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 0db3c83cce29..6715e37ee66b 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -370,7 +370,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 bec0a2796c37..bd69e4b120a8 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..a5eec97a40fe 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) > > + DRM_DEBUG("RS is removed from all Gens: ignoring.\n"); > > Still missing the return -EINVAL here. Needs an igt update I think. that's because you are replying on v1 rather than v2. See the updated patch down the thread. > > > > > 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 ab89dabc2965..843a940bcc58 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -2138,8 +2138,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); > > > > @@ -2657,10 +2656,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))); > > I'd hard-code this to disable the RS instead of removing it outright. _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT | CTX_CTRL_RS_CTX_ENABLE) | _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH); right? > > With these 2 nits addressed and the commit message augmented as I propose > in my other reply: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> thanks Lucas De Marchi > > > > + CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)); > > 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 700f94c371b3..a76b974c398d 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1979,9 +1979,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 ce6cc2a6cf7a..efec5b14ddb0 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > > @@ -472,7 +472,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; > > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index 7f5634ce8e88..179e0477dbc6 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > > #define I915_PARAM_SUBSLICE_TOTAL 33 > > #define I915_PARAM_EU_TOTAL 34 > > #define I915_PARAM_HAS_GPU_RESET 35 > > +/* Deprecated: do not use */ > > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > > #define I915_PARAM_HAS_POOLED_EU 38 > > diff --git a/tools/include/uapi/drm/i915_drm.h b/tools/include/uapi/drm/i915_drm.h > > index 7f5634ce8e88..179e0477dbc6 100644 > > --- a/tools/include/uapi/drm/i915_drm.h > > +++ b/tools/include/uapi/drm/i915_drm.h > > @@ -449,6 +449,7 @@ typedef struct drm_i915_irq_wait { > > #define I915_PARAM_SUBSLICE_TOTAL 33 > > #define I915_PARAM_EU_TOTAL 34 > > #define I915_PARAM_HAS_GPU_RESET 35 > > +/* Deprecated: do not use */ > > #define I915_PARAM_HAS_RESOURCE_STREAMER 36 > > #define I915_PARAM_HAS_EXEC_SOFTPIN 37 > > #define I915_PARAM_HAS_POOLED_EU 38 > > -- > > 2.17.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Kill resource streamer 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi 2018-07-10 0:06 ` [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Lucas De Marchi 2018-07-10 0:06 ` [PATCH 2/2] drm/i915: kill resource streamer Lucas De Marchi @ 2018-07-10 0:20 ` Patchwork 2018-07-10 0:35 ` ✓ Fi.CI.BAT: success " Patchwork ` (3 subsequent siblings) 6 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2018-07-10 0:20 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: Kill resource streamer URL : https://patchwork.freedesktop.org/series/46224/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Okay! Commit: drm/i915: kill resource streamer -drivers/gpu/drm/i915/selftests/../i915_drv.h:3652:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3650:16: warning: expression using sizeof(void) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for Kill resource streamer 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi ` (2 preceding siblings ...) 2018-07-10 0:20 ` ✗ Fi.CI.SPARSE: warning for Kill " Patchwork @ 2018-07-10 0:35 ` Patchwork 2018-07-10 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork ` (2 subsequent siblings) 6 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2018-07-10 0:35 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: Kill resource streamer URL : https://patchwork.freedesktop.org/series/46224/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4458 -> Patchwork_9599 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46224/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9599 that come from known issues: === IGT changes === ==== Possible fixes ==== igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS igt@kms_flip@basic-flip-vs-dpms: fi-skl-6700hq: DMESG-WARN (fdo#105998) -> PASS fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 == Participating hosts (45 -> 41) == Additional (1): fi-skl-guc Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4458 -> Patchwork_9599 CI_DRM_4458: dbed4c29cc0cd0225aae60c57ac2f4a738bb5a17 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9599: 526eac8de54b23e00a6293cb7ff185a2b1530156 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 526eac8de54b drm/i915: kill resource streamer a10cc1118b3e drm/i915/icl: move has_resource_streamer to GEN11_FEATURES == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9599/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.IGT: failure for Kill resource streamer 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi ` (3 preceding siblings ...) 2018-07-10 0:35 ` ✓ Fi.CI.BAT: success " Patchwork @ 2018-07-10 9:35 ` Patchwork 2018-07-12 21:39 ` ✗ Fi.CI.SPARSE: warning for Kill resource streamer (rev2) Patchwork 2018-07-12 21:57 ` ✓ Fi.CI.BAT: success " Patchwork 6 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2018-07-10 9:35 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: Kill resource streamer URL : https://patchwork.freedesktop.org/series/46224/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4458_full -> Patchwork_9599_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9599_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9599_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9599_full: === IGT changes === ==== Possible regressions ==== igt@gem_exec_params@rs-invalid-gen: shard-snb: PASS -> FAIL igt@gem_exec_params@rs-invalid-on-vebox-ring: shard-hsw: PASS -> FAIL +2 igt@perf_pmu@event-wait-rcs0: shard-glk: PASS -> FAIL +11 igt@perf_pmu@semaphore-wait-idle-bcs0: shard-kbl: PASS -> FAIL +13 igt@perf_pmu@semaphore-wait-vcs0: shard-apl: PASS -> FAIL +11 ==== Warnings ==== igt@gem_exec_schedule@deep-vebox: shard-kbl: SKIP -> PASS +2 igt@gem_mocs_settings@mocs-rc6-bsd1: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9599_full that come from known issues: === IGT changes === ==== Issues hit ==== igt@drv_suspend@shrink: shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133) igt@kms_vblank@pipe-c-ts-continuation-suspend: shard-hsw: PASS -> FAIL (fdo#104894) igt@perf@blocking: shard-hsw: PASS -> FAIL (fdo#102252) ==== Possible fixes ==== igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@kms_flip@2x-dpms-vs-vblank-race-interruptible: shard-glk: FAIL (fdo#103060) -> PASS igt@kms_flip_tiling@flip-y-tiled: shard-glk: FAIL (fdo#107161) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894 fdo#107161 https://bugs.freedesktop.org/show_bug.cgi?id=107161 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4458 -> Patchwork_9599 CI_DRM_4458: dbed4c29cc0cd0225aae60c57ac2f4a738bb5a17 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9599: 526eac8de54b23e00a6293cb7ff185a2b1530156 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9599/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Kill resource streamer (rev2) 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi ` (4 preceding siblings ...) 2018-07-10 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork @ 2018-07-12 21:39 ` Patchwork 2018-07-12 21:57 ` ✓ Fi.CI.BAT: success " Patchwork 6 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2018-07-12 21:39 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: Kill resource streamer (rev2) URL : https://patchwork.freedesktop.org/series/46224/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Okay! Commit: drm/i915: kill resource streamer -drivers/gpu/drm/i915/selftests/../i915_drv.h:3655:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for Kill resource streamer (rev2) 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi ` (5 preceding siblings ...) 2018-07-12 21:39 ` ✗ Fi.CI.SPARSE: warning for Kill resource streamer (rev2) Patchwork @ 2018-07-12 21:57 ` Patchwork 6 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2018-07-12 21:57 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx == Series Details == Series: Kill resource streamer (rev2) URL : https://patchwork.freedesktop.org/series/46224/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9640 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46224/revisions/2/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9640: === IGT changes === ==== Possible regressions ==== igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: {fi-cfl-8109u}: PASS -> INCOMPLETE == Known issues == Here are the changes found in Patchwork_9640 that come from known issues: === IGT changes === ==== Possible fixes ==== igt@drv_module_reload@basic-reload: fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2 igt@kms_chamelium@hdmi-hpd-fast: fi-kbl-7500u: FAIL (fdo#103841, fdo#102672) -> SKIP igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS ==== Warnings ==== igt@gem_exec_suspend@basic-s4-devices: {fi-kbl-8809g}: INCOMPLETE (fdo#107139) -> DMESG-WARN (fdo#107139) {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 == Participating hosts (46 -> 42) == Additional (1): fi-byt-j1900 Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4479 -> Patchwork_9640 CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9640: 2997ade936fa14aa75840c30d62237891853f586 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2997ade936fa drm/i915: kill resource streamer a668fe24a019 drm/i915/icl: move has_resource_streamer to GEN11_FEATURES == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9640/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-07-13 14:17 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-10 0:06 [PATCH 0/2] Kill resource streamer Lucas De Marchi 2018-07-10 0:06 ` [PATCH 1/2] drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Lucas De Marchi 2018-07-10 16:52 ` Daniele Ceraolo Spurio 2018-07-10 0:06 ` [PATCH 2/2] drm/i915: kill resource streamer Lucas De Marchi 2018-07-10 9:51 ` Chris Wilson 2018-07-11 17:29 ` Lucas De Marchi 2018-07-12 21:02 ` [PATCH v2] " Lucas De Marchi 2018-07-12 22:58 ` Daniele Ceraolo Spurio 2018-07-12 23:06 ` Rodrigo Vivi 2018-07-12 23:28 ` De Marchi, Lucas 2018-07-12 23:48 ` Rodrigo Vivi 2018-07-13 8:46 ` Daniel Vetter 2018-07-13 14:17 ` Lucas De Marchi 2018-07-13 8:51 ` [PATCH 2/2] " Daniel Vetter 2018-07-13 14:11 ` Lucas De Marchi 2018-07-10 0:20 ` ✗ Fi.CI.SPARSE: warning for Kill " Patchwork 2018-07-10 0:35 ` ✓ Fi.CI.BAT: success " Patchwork 2018-07-10 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork 2018-07-12 21:39 ` ✗ Fi.CI.SPARSE: warning for Kill resource streamer (rev2) Patchwork 2018-07-12 21:57 ` ✓ Fi.CI.BAT: success " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.