[Intel-gfx] [PATCH v2] drm/i915: kill resource streamer

Daniele Ceraolo Spurio daniele.ceraolospurio at intel.com
Thu Jul 12 22:58:56 UTC 2018



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 at 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 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           |  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;
>   
> 


More information about the Intel-gfx mailing list