* [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
* [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
* ✗ 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
* 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 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
* 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
* ✗ 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
* 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, intel-gfx
On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote:
> On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote:
> > After disabling resource streamer on ICL (due to it actually not
> > existing there), I got feedback that there have been some experimental
> > patches for mesa to use it, but nothing ever landed nor shipped.
> >
> > This is a tentative to remove it from kernel keeping the uapi defines
> > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
> > some platforms - Daniele mentioned there's possible performance regression
> > in gem_latency if this bit is not set. On this patch, I'm just removing
> > it in order to get proper numbers.
>
> Since I see below 3 occurrences of
> - .has_resource_streamer = 1
>
> I feel this commit message lacks of the reason why we are removing
> this features for the platforms that already had it.
/me confused. Isn't the first paragraph in the commit message exactly that?
Lucas De Marchi
>
> >
> > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
> > - don't bother trying to document removed params on uapi header:
> > applications should know that from the query.
> > (from Chris)
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > drivers/gpu/drm/i915/i915_drv.h | 2 --
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-------------
> > drivers/gpu/drm/i915/i915_pci.c | 4 ----
> > drivers/gpu/drm/i915/intel_device_info.h | 1 -
> > drivers/gpu/drm/i915/intel_lrc.c | 7 ++-----
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
> > 8 files changed, 6 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 3eba3d1ab5b8..10423e2c1176 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> > void *data,
> > value = 2;
> > break;
> > case I915_PARAM_HAS_RESOURCE_STREAMER:
> > - value = HAS_RESOURCE_STREAMER(dev_priv);
> > + value = 0;
> > break;
> > case I915_PARAM_HAS_POOLED_EU:
> > value = HAS_POOLED_EU(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 01dd29837233..dddb886fa06c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis
> > sion()
> > #define USES_HUC(dev_priv) intel_uc_is_using_huc()
> >
> > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)-
> > >info.has_resource_streamer)
> > -
> > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
> >
> > #define INTEL_PCH_DEVICE_ID_MASK 0xff80
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 1932bc227942..ca21a08b2be9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> > if (!eb.engine)
> > return -EINVAL;
> >
> > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> > - if (!HAS_RESOURCE_STREAMER(eb.i915)) {
> > - DRM_DEBUG("RS is only allowed for Haswell and
> > Gen8 - Gen10\n");
> > - return -EINVAL;
> > - }
> > - if (eb.engine->id != RCS) {
> > - DRM_DEBUG("RS is not available on %s\n",
> > - eb.engine->name);
> > - return -EINVAL;
> > - }
> > -
> > - eb.batch_flags |= I915_DISPATCH_RS;
> > - }
> > + if (args->flags & I915_EXEC_RESOURCE_STREAMER)
> > + return -EINVAL;
> >
> > if (args->flags & I915_EXEC_FENCE_IN) {
> > in_fence = sync_file_get_fence(lower_32_bits(args-
> > >rsvd2));
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index c03ba0fe0845..7af19097dce8 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -360,7 +360,6 @@ static const struct intel_device_info
> > intel_valleyview_info = {
> > .has_ddi = 1, \
> > .has_fpga_dbg = 1, \
> > .has_psr = 1, \
> > - .has_resource_streamer = 1, \
> > .has_dp_mst = 1, \
> > .has_rc6p = 0 /* RC6p removed-by HSW */, \
> > .has_runtime_pm = 1
> > @@ -433,7 +432,6 @@ static const struct intel_device_info
> > intel_cherryview_info = {
> > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> > .has_64bit_reloc = 1,
> > .has_runtime_pm = 1,
> > - .has_resource_streamer = 1,
> > .has_rc6 = 1,
> > .has_logical_ring_contexts = 1,
> > .has_gmch_display = 1,
> > @@ -506,7 +504,6 @@ static const struct intel_device_info
> > intel_skylake_gt4_info = {
> > .has_runtime_pm = 1, \
> > .has_pooled_eu = 0, \
> > .has_csr = 1, \
> > - .has_resource_streamer = 1, \
> > .has_rc6 = 1, \
> > .has_dp_mst = 1, \
> > .has_logical_ring_contexts = 1, \
> > @@ -593,7 +590,6 @@ static const struct intel_device_info
> > intel_cannonlake_info = {
> > GEN(11), \
> > .ddb_size = 2048, \
> > .has_csr = 0, \
> > - .has_resource_streamer = 0, \
> > .has_logical_ring_elsq = 1
> >
> > static const struct intel_device_info intel_icelake_11_info = {
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index 633f9fbf72ea..6814cc7dfcd3 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -103,7 +103,6 @@ enum intel_platform {
> > func(has_psr); \
> > func(has_rc6); \
> > func(has_rc6p); \
> > - func(has_resource_streamer); \
> > func(has_runtime_pm); \
> > func(has_snoop); \
> > func(unfenced_needs_alignment); \
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 35d37af0cb9a..ad10fb7b9668 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request
> > *rq,
> >
> > /* FIXME(BDW): Address space and security selectors. */
> > *cs++ = MI_BATCH_BUFFER_START_GEN8 |
> > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) |
> > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER :
> > 0);
> > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8));
> > *cs++ = lower_32_bits(offset);
> > *cs++ = upper_32_bits(offset);
> >
> > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs,
> > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine),
> > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
> > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) |
> > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH |
> > - (HAS_RESOURCE_STREAMER(dev_priv) ?
> > - CTX_CTRL_RS_CTX_ENABLE : 0)));
> > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH));
> > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
> > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0);
> > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index f4bd185c9369..e5fcbbc2abbc 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq,
> > return PTR_ERR(cs);
> >
> > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags &
> > I915_DISPATCH_SECURE ?
> > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) |
> > - (dispatch_flags & I915_DISPATCH_RS ?
> > - MI_BATCH_RESOURCE_STREAMER : 0);
> > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW);
> > /* bit0-7 is the length on GEN6+ */
> > *cs++ = offset;
> > intel_ring_advance(rq, cs);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index d1eee08e5f6b..a2c1d7044f2a 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -467,7 +467,6 @@ struct intel_engine_cs {
> > unsigned int dispatch_flags);
> > #define I915_DISPATCH_SECURE BIT(0)
> > #define I915_DISPATCH_PINNED BIT(1)
> > -#define I915_DISPATCH_RS BIT(2)
> > void (*emit_breadcrumb)(struct i915_request *rq,
> > u32 *cs);
> > int emit_breadcrumb_sz;
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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, intel-gfx
On Thu, Jul 12, 2018 at 11:28:25PM +0000, De Marchi, Lucas wrote:
> On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote:
> > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote:
> > > After disabling resource streamer on ICL (due to it actually not
> > > existing there), I got feedback that there have been some experimental
> > > patches for mesa to use it, but nothing ever landed nor shipped.
> > >
> > > This is a tentative to remove it from kernel keeping the uapi defines
> > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
> > > some platforms - Daniele mentioned there's possible performance regression
> > > in gem_latency if this bit is not set. On this patch, I'm just removing
> > > it in order to get proper numbers.
> >
> > Since I see below 3 occurrences of
> > - .has_resource_streamer = 1
> >
> > I feel this commit message lacks of the reason why we are removing
> > this features for the platforms that already had it.
>
> /me confused. Isn't the first paragraph in the commit message exactly that?
/me confused now... For me the first paragraph tells that after removing
from ICL mesa might want to use it on ICL.
second paragraph tells that you are removing the support but keeping the uapi.
what I couldn't find is the explanation why exactly you are removing this
feature. Is this useless? No body using it? It is buggy? It is deprecated? why?
>
> Lucas De Marchi
>
> >
> > >
> > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
> > > - don't bother trying to document removed params on uapi header:
> > > applications should know that from the query.
> > > (from Chris)
> > >
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > > drivers/gpu/drm/i915/i915_drv.h | 2 --
> > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-------------
> > > drivers/gpu/drm/i915/i915_pci.c | 4 ----
> > > drivers/gpu/drm/i915/intel_device_info.h | 1 -
> > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++-----
> > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
> > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
> > > 8 files changed, 6 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 3eba3d1ab5b8..10423e2c1176 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> > > void *data,
> > > value = 2;
> > > break;
> > > case I915_PARAM_HAS_RESOURCE_STREAMER:
> > > - value = HAS_RESOURCE_STREAMER(dev_priv);
> > > + value = 0;
> > > break;
> > > case I915_PARAM_HAS_POOLED_EU:
> > > value = HAS_POOLED_EU(dev_priv);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 01dd29837233..dddb886fa06c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis
> > > sion()
> > > #define USES_HUC(dev_priv) intel_uc_is_using_huc()
> > >
> > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)-
> > > >info.has_resource_streamer)
> > > -
> > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
> > >
> > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index 1932bc227942..ca21a08b2be9 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> > > if (!eb.engine)
> > > return -EINVAL;
> > >
> > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) {
> > > - DRM_DEBUG("RS is only allowed for Haswell and
> > > Gen8 - Gen10\n");
> > > - return -EINVAL;
> > > - }
> > > - if (eb.engine->id != RCS) {
> > > - DRM_DEBUG("RS is not available on %s\n",
> > > - eb.engine->name);
> > > - return -EINVAL;
> > > - }
> > > -
> > > - eb.batch_flags |= I915_DISPATCH_RS;
> > > - }
> > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER)
> > > + return -EINVAL;
> > >
> > > if (args->flags & I915_EXEC_FENCE_IN) {
> > > in_fence = sync_file_get_fence(lower_32_bits(args-
> > > >rsvd2));
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > b/drivers/gpu/drm/i915/i915_pci.c
> > > index c03ba0fe0845..7af19097dce8 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -360,7 +360,6 @@ static const struct intel_device_info
> > > intel_valleyview_info = {
> > > .has_ddi = 1, \
> > > .has_fpga_dbg = 1, \
> > > .has_psr = 1, \
> > > - .has_resource_streamer = 1, \
> > > .has_dp_mst = 1, \
> > > .has_rc6p = 0 /* RC6p removed-by HSW */, \
> > > .has_runtime_pm = 1
> > > @@ -433,7 +432,6 @@ static const struct intel_device_info
> > > intel_cherryview_info = {
> > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> > > .has_64bit_reloc = 1,
> > > .has_runtime_pm = 1,
> > > - .has_resource_streamer = 1,
> > > .has_rc6 = 1,
> > > .has_logical_ring_contexts = 1,
> > > .has_gmch_display = 1,
> > > @@ -506,7 +504,6 @@ static const struct intel_device_info
> > > intel_skylake_gt4_info = {
> > > .has_runtime_pm = 1, \
> > > .has_pooled_eu = 0, \
> > > .has_csr = 1, \
> > > - .has_resource_streamer = 1, \
> > > .has_rc6 = 1, \
> > > .has_dp_mst = 1, \
> > > .has_logical_ring_contexts = 1, \
> > > @@ -593,7 +590,6 @@ static const struct intel_device_info
> > > intel_cannonlake_info = {
> > > GEN(11), \
> > > .ddb_size = 2048, \
> > > .has_csr = 0, \
> > > - .has_resource_streamer = 0, \
> > > .has_logical_ring_elsq = 1
> > >
> > > static const struct intel_device_info intel_icelake_11_info = {
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > > b/drivers/gpu/drm/i915/intel_device_info.h
> > > index 633f9fbf72ea..6814cc7dfcd3 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > @@ -103,7 +103,6 @@ enum intel_platform {
> > > func(has_psr); \
> > > func(has_rc6); \
> > > func(has_rc6p); \
> > > - func(has_resource_streamer); \
> > > func(has_runtime_pm); \
> > > func(has_snoop); \
> > > func(unfenced_needs_alignment); \
> > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > index 35d37af0cb9a..ad10fb7b9668 100644
> > > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > @@ -2127,8 +2127,7 @@ static int gen8_emit_bb_start(struct i915_request
> > > *rq,
> > >
> > > /* FIXME(BDW): Address space and security selectors. */
> > > *cs++ = MI_BATCH_BUFFER_START_GEN8 |
> > > - (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) |
> > > - (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER :
> > > 0);
> > > + (flags & I915_DISPATCH_SECURE ? 0 : BIT(8));
> > > *cs++ = lower_32_bits(offset);
> > > *cs++ = upper_32_bits(offset);
> > >
> > > @@ -2647,9 +2646,7 @@ static void execlists_init_reg_state(u32 *regs,
> > > CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine),
> > > _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
> > > CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) |
> > > - _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH |
> > > - (HAS_RESOURCE_STREAMER(dev_priv) ?
> > > - CTX_CTRL_RS_CTX_ENABLE : 0)));
> > > + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH));
> > > CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
> > > CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0);
> > > CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0);
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index f4bd185c9369..e5fcbbc2abbc 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -1977,9 +1977,7 @@ hsw_emit_bb_start(struct i915_request *rq,
> > > return PTR_ERR(cs);
> > >
> > > *cs++ = MI_BATCH_BUFFER_START | (dispatch_flags &
> > > I915_DISPATCH_SECURE ?
> > > - 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) |
> > > - (dispatch_flags & I915_DISPATCH_RS ?
> > > - MI_BATCH_RESOURCE_STREAMER : 0);
> > > + 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW);
> > > /* bit0-7 is the length on GEN6+ */
> > > *cs++ = offset;
> > > intel_ring_advance(rq, cs);
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > index d1eee08e5f6b..a2c1d7044f2a 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > @@ -467,7 +467,6 @@ struct intel_engine_cs {
> > > unsigned int dispatch_flags);
> > > #define I915_DISPATCH_SECURE BIT(0)
> > > #define I915_DISPATCH_PINNED BIT(1)
> > > -#define I915_DISPATCH_RS BIT(2)
> > > void (*emit_breadcrumb)(struct i915_request *rq,
> > > u32 *cs);
> > > int emit_breadcrumb_sz;
> > >
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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, intel-gfx, 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 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
* 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, intel-gfx, 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
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.