[Intel-gfx] [PATCH v3 2/2] drm/i915: kill resource streamer
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Aug 3 08:41:46 UTC 2018
On 19/07/2018 18:05, 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 RS years ago, but nothing ever landed or shipped
> because there was no performance improvement.
>
> This removes it from kernel keeping the uapi defines around for
> compatibility.
>
> 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)
>
> v3: - disable CTX_CTRL_RS_CTX_ENABLE istead of removing it
> - reword commit message after Daniele confirmed no performance
> regression on his machine
> - reword commit message to make clear RS is being removed due to
> never been used
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at 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 | 10 ++++------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
> 8 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 3834bd758a2e..b4b43402cc0c 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 4fb937399440..0b22ca0b24a8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2612,8 +2612,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;
I think slightly nicer than leaving it here as the only trivial flag
rejection test, would be to add the flag to __I915_EXEC_ILLEGAL_FLAGS.
Makes the rejection nice and early before any allocation and do_execbuf
looks tidier.
Regards,
Tvrtko
>
> 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 3a4bb017d676..7b570ba90d9f 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 174479232e94..3726709b8df3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2066,8 +2066,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);
>
> @@ -2585,10 +2584,9 @@ 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_CTRL_RS_CTX_ENABLE) |
> + _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 33faad3197fe..763f56072308 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1982,9 +1982,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 f5ffa6d31e82..8c174e87686b 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -474,7 +474,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;
>
>
More information about the Intel-gfx
mailing list