From 2ca9faa551c4c97610bb0209e20c7231a93712ff Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 5 Apr 2017 16:30:54 +0100 Subject: drm/i915: Assert the engine is idle before overwiting the HWS When we update the global seqno (on the engine timeline), we modify HW state (both registers and mapped pages). As we do this, we should be sure that the HW is idle and we are not causing a conflict. The caller is supposed to wait_for_idle before calling us to update the seqno, so let's assert they have and the engine is indeed idle. Signed-off-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/20170405153055.28123-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 854e8e0c836b..92f871cf3265 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -223,6 +223,8 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) { struct drm_i915_private *dev_priv = engine->i915; + GEM_BUG_ON(!intel_engine_is_idle(engine)); + /* Our semaphore implementation is strictly monotonic (i.e. we proceed * so long as the semaphore value in the register/page is greater * than the sync value), so whenever we reset the seqno, @@ -260,6 +262,8 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) * there are any waiters for that seqno. */ intel_engine_wakeup(engine); + + GEM_BUG_ON(intel_engine_get_seqno(engine) != seqno); } static void intel_engine_init_timeline(struct intel_engine_cs *engine) -- cgit v1.2.3 From 0908180b9a1cf8c5410d0c1151894fae710744dc Mon Sep 17 00:00:00 2001 From: Daniele Ceraolo Spurio Date: Mon, 10 Apr 2017 07:34:29 -0700 Subject: drm/i915: Classify the engines in class + instance In such a way that vcs and vcs2 are just two different instances (0 and 1) of the same engine class (VIDEO_DECODE_CLASS). v2: Align the instance types (Tvrtko) v3: Don't use enums for bspec-defined stuff (Michal) Cc: Paulo Zanoni Cc: Rodrigo Vivi Cc: Chris Wilson Signed-off-by: Daniele Ceraolo Spurio Reviewed-by: Tvrtko Ursulin Reviewed-by: Michal Wajdeczko Signed-off-by: Oscar Mateo Link: http://patchwork.freedesktop.org/patch/msgid/1491834873-9345-2-git-send-email-oscar.mateo@intel.com Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++ drivers/gpu/drm/i915/intel_engine_cs.c | 14 ++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++++ 3 files changed, 26 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 11b12f412492..4c72adae368b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -85,6 +85,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VECS_HW 3 #define VCS2_HW 4 +/* Engine class */ + +#define RENDER_CLASS 0 +#define VIDEO_DECODE_CLASS 1 +#define VIDEO_ENHANCEMENT_CLASS 2 +#define COPY_ENGINE_CLASS 3 +#define OTHER_CLASS 4 + /* PCI config space */ #define MCHBAR_I915 0x44 diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 92f871cf3265..bb229274c30d 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -30,6 +30,8 @@ static const struct engine_info { const char *name; unsigned int exec_id; unsigned int hw_id; + u8 class; + u8 instance; u32 mmio_base; unsigned irq_shift; int (*init_legacy)(struct intel_engine_cs *engine); @@ -39,6 +41,8 @@ static const struct engine_info { .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, + .class = RENDER_CLASS, + .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, .init_execlists = logical_render_ring_init, @@ -48,6 +52,8 @@ static const struct engine_info { .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, + .class = COPY_ENGINE_CLASS, + .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -57,6 +63,8 @@ static const struct engine_info { .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, + .class = VIDEO_DECODE_CLASS, + .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -66,6 +74,8 @@ static const struct engine_info { .name = "vcs2", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, + .class = VIDEO_DECODE_CLASS, + .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -75,6 +85,8 @@ static const struct engine_info { .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, + .class = VIDEO_ENHANCEMENT_CLASS, + .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, @@ -101,6 +113,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv, engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; engine->irq_shift = info->irq_shift; + engine->class = info->class; + engine->instance = info->instance; /* Nothing to do here, execute in order of dependencies */ engine->schedule = NULL; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index cbe61d3f31da..f54fe7d063a8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -193,6 +193,10 @@ struct intel_engine_cs { enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; + + u8 class; + u8 instance; + unsigned int guc_id; u32 mmio_base; unsigned int irq_shift; -- cgit v1.2.3 From 5ff36d36a36d55c8bd4bc937b6a9450b72853c5f Mon Sep 17 00:00:00 2001 From: Oscar Mateo Date: Mon, 10 Apr 2017 07:34:30 -0700 Subject: drm/i915: Use the same vfunc for BSD2 ring init If we needed to do something different for the init functions, we could always look at the engine instance to make the distinction. But, in any case, the two functions are virtually identical already (please notice that BSD2_RING is only used from gen8 onwards). With this, the init functions depends excusively on the engine class (a fact that we will use soon). v2: Commit message Cc: Paulo Zanoni Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Signed-off-by: Oscar Mateo Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/1491834873-9345-3-git-send-email-oscar.mateo@intel.com Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 -------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 3 files changed, 1 insertion(+), 16 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index bb229274c30d..a7ffa4c66d1c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -79,7 +79,7 @@ static const struct engine_info { .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd2_ring_buffer, + .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { .name = "vecs", diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 97d5fcca8805..833740bf49c9 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2175,20 +2175,6 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine) return intel_init_ring_buffer(engine); } -/** - * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3) - */ -int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine) -{ - struct drm_i915_private *dev_priv = engine->i915; - - intel_ring_default_vfuncs(dev_priv, engine); - - engine->emit_flush = gen6_bsd_ring_flush; - - return intel_init_ring_buffer(engine); -} - int intel_init_blt_ring_buffer(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f54fe7d063a8..8b53ddb190c5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -555,7 +555,6 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine); int intel_init_render_ring_buffer(struct intel_engine_cs *engine); int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine); -int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine); int intel_init_blt_ring_buffer(struct intel_engine_cs *engine); int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine); -- cgit v1.2.3 From 6e516148f35401029ac920e323cce34266c22e6d Mon Sep 17 00:00:00 2001 From: Oscar Mateo Date: Mon, 10 Apr 2017 07:34:31 -0700 Subject: drm/i915: Generate the engine name based on the instance number Not really needed, but makes the next change a little bit more compact. v2: - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris) - Make sure the mock engine name is null-terminated (Tvrtko, Chris) v3: Because I'm stupid (Chris) v4: Verify engine name wasn't truncated (Michal) v5: - Kill the warning in mock engine (Chris) Cc: Paulo Zanoni Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Reviewed-by: Tvrtko Ursulin Reviewed-by: Michal Wajdeczko Signed-off-by: Oscar Mateo Link: http://patchwork.freedesktop.org/patch/msgid/1491834873-9345-4-git-send-email-oscar.mateo@intel.com Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a7ffa4c66d1c..5e5cda02ee19 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -71,7 +71,7 @@ static const struct engine_info { .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs2", + .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, @@ -108,7 +108,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv, engine->id = id; engine->i915 = dev_priv; - engine->name = info->name; + WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", + info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 8b53ddb190c5..fd8994b5688c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -187,9 +187,11 @@ enum intel_engine_id { VECS }; +#define INTEL_ENGINE_CS_MAX_NAME 8 + struct intel_engine_cs { struct drm_i915_private *i915; - const char *name; + char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; unsigned int exec_id; unsigned int hw_id; diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b89050e0de48..b8e53bdc3cc4 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, /* minimal engine setup for requests */ engine->base.i915 = i915; - engine->base.name = name; + snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); engine->base.id = id++; engine->base.status_page.page_addr = (void *)(engine + 1); -- cgit v1.2.3 From b8400f01fcbbacec274d89a1c41340d0409529bb Mon Sep 17 00:00:00 2001 From: Oscar Mateo Date: Mon, 10 Apr 2017 07:34:32 -0700 Subject: drm/i915: Split the engine info table in two levels, using class + instance There are some properties that logically belong to the engine class, and some that belong to the engine instance. Make it explicit. v2: Commit message (Tvrtko) v3: - Rebased - Exec/uabi id should be per instance (Chris) v4: - Rebased - Avoid re-ordering fields for smaller diff (Tvrtko) - Bug on oob access to the class array (Michal) v5: Bug on the right thing (Michal) v6: Rebased Cc: Tvrtko Ursulin Cc: Paulo Zanoni Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Reviewed-by: Michal Wajdeczko Signed-off-by: Oscar Mateo Link: http://patchwork.freedesktop.org/patch/msgid/1491834873-9345-5-git-send-email-oscar.mateo@intel.com Reviewed-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_engine_cs.c | 66 ++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 23 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 5e5cda02ee19..058ecc020b28 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -26,71 +26,84 @@ #include "intel_ringbuffer.h" #include "intel_lrc.h" -static const struct engine_info { +struct engine_class_info { const char *name; - unsigned int exec_id; + int (*init_legacy)(struct intel_engine_cs *engine); + int (*init_execlists)(struct intel_engine_cs *engine); +}; + +static const struct engine_class_info intel_engine_classes[] = { + [RENDER_CLASS] = { + .name = "rcs", + .init_execlists = logical_render_ring_init, + .init_legacy = intel_init_render_ring_buffer, + }, + [COPY_ENGINE_CLASS] = { + .name = "bcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_blt_ring_buffer, + }, + [VIDEO_DECODE_CLASS] = { + .name = "vcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_bsd_ring_buffer, + }, + [VIDEO_ENHANCEMENT_CLASS] = { + .name = "vecs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_vebox_ring_buffer, + }, +}; + +struct engine_info { unsigned int hw_id; + unsigned int exec_id; u8 class; u8 instance; u32 mmio_base; unsigned irq_shift; - int (*init_legacy)(struct intel_engine_cs *engine); - int (*init_execlists)(struct intel_engine_cs *engine); -} intel_engines[] = { +}; + +static const struct engine_info intel_engines[] = { [RCS] = { - .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, .class = RENDER_CLASS, .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, - .init_execlists = logical_render_ring_init, - .init_legacy = intel_init_render_ring_buffer, }, [BCS] = { - .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, .class = COPY_ENGINE_CLASS, .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_blt_ring_buffer, }, [VCS] = { - .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { - .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, .class = VIDEO_ENHANCEMENT_CLASS, .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_vebox_ring_buffer, }, }; @@ -99,8 +112,12 @@ intel_engine_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id) { const struct engine_info *info = &intel_engines[id]; + const struct engine_class_info *class_info; struct intel_engine_cs *engine; + GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); + class_info = &intel_engine_classes[info->class]; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -109,7 +126,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv, engine->id = id; engine->i915 = dev_priv; WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", - info->name, info->instance) >= sizeof(engine->name)); + class_info->name, info->instance) >= + sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; @@ -190,12 +208,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv) int err = 0; for_each_engine(engine, dev_priv, id) { + const struct engine_class_info *class_info = + &intel_engine_classes[engine->class]; int (*init)(struct intel_engine_cs *engine); if (i915.enable_execlists) - init = intel_engines[id].init_execlists; + init = class_info->init_execlists; else - init = intel_engines[id].init_legacy; + init = class_info->init_legacy; if (!init) { kfree(engine); dev_priv->engine[id] = NULL; -- cgit v1.2.3 From 1d39f28170cb95e8b9eb9833d1c17528f400f9c4 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 11 Apr 2017 13:43:06 +0100 Subject: drm/i915: Rename intel_engine_cs.exec_id to uabi_id We want to refer to the index of the engine consistently throughout the userspace ABI. We already have such an index through the execbuffer engine specifier, that needs to be able to refer to each engine specifically, so rename it the index to uabi_id to reflect its generality beyond execbuf. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Link: http://patchwork.freedesktop.org/patch/msgid/20170411124306.15448-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_cmd_parser.c | 8 ++++---- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 14 +++++++------- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 7af100f84410..2a1a3347495a 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1166,8 +1166,8 @@ static bool check_cmd(const struct intel_engine_cs *engine, find_reg(engine, is_master, reg_addr); if (!reg) { - DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (exec_id=%d)\n", - reg_addr, *cmd, engine->exec_id); + DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (%s)\n", + reg_addr, *cmd, engine->name); return false; } @@ -1222,11 +1222,11 @@ static bool check_cmd(const struct intel_engine_cs *engine, desc->bits[i].mask; if (dword != desc->bits[i].expected) { - DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (exec_id=%d)\n", + DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (%s)\n", *cmd, desc->bits[i].mask, desc->bits[i].expected, - dword, engine->exec_id); + dword, engine->name); return false; } } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b210acc3d0b4..cb8c6a94ba4e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3996,7 +3996,7 @@ __busy_set_if_active(const struct dma_fence *fence, if (i915_gem_request_completed(rq)) return 0; - return flag(rq->engine->exec_id); + return flag(rq->engine->uabi_id); } static __always_inline unsigned int diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 058ecc020b28..71e89a93fe18 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -57,7 +57,7 @@ static const struct engine_class_info intel_engine_classes[] = { struct engine_info { unsigned int hw_id; - unsigned int exec_id; + unsigned int uabi_id; u8 class; u8 instance; u32 mmio_base; @@ -67,7 +67,7 @@ struct engine_info { static const struct engine_info intel_engines[] = { [RCS] = { .hw_id = RCS_HW, - .exec_id = I915_EXEC_RENDER, + .uabi_id = I915_EXEC_RENDER, .class = RENDER_CLASS, .instance = 0, .mmio_base = RENDER_RING_BASE, @@ -75,7 +75,7 @@ static const struct engine_info intel_engines[] = { }, [BCS] = { .hw_id = BCS_HW, - .exec_id = I915_EXEC_BLT, + .uabi_id = I915_EXEC_BLT, .class = COPY_ENGINE_CLASS, .instance = 0, .mmio_base = BLT_RING_BASE, @@ -83,7 +83,7 @@ static const struct engine_info intel_engines[] = { }, [VCS] = { .hw_id = VCS_HW, - .exec_id = I915_EXEC_BSD, + .uabi_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, @@ -91,7 +91,7 @@ static const struct engine_info intel_engines[] = { }, [VCS2] = { .hw_id = VCS2_HW, - .exec_id = I915_EXEC_BSD, + .uabi_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, @@ -99,7 +99,7 @@ static const struct engine_info intel_engines[] = { }, [VECS] = { .hw_id = VECS_HW, - .exec_id = I915_EXEC_VEBOX, + .uabi_id = I915_EXEC_VEBOX, .class = VIDEO_ENHANCEMENT_CLASS, .instance = 0, .mmio_base = VEBOX_RING_BASE, @@ -128,7 +128,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv, WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", class_info->name, info->instance) >= sizeof(engine->name)); - engine->exec_id = info->exec_id; + engine->uabi_id = info->uabi_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; engine->irq_shift = info->irq_shift; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index fd8994b5688c..00d36aa4e26d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -193,7 +193,7 @@ struct intel_engine_cs { struct drm_i915_private *i915; char name[INTEL_ENGINE_CS_MAX_NAME]; enum intel_engine_id id; - unsigned int exec_id; + unsigned int uabi_id; unsigned int hw_id; u8 class; -- cgit v1.2.3 From 5f9be05432cb4c323967f6d71ce0ecc024a775c7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 11 Apr 2017 17:56:58 +0100 Subject: drm/i915: Bail if we do not setup the RCS engine In places, we assume that RCS exists. This has been true forever, but let us catch this failure during bringup by adding an explicit check that we do have an RCS engine. v2: Make use of HAS_ENGINE (Tvrtko) Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170411165658.23828-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_engine_cs.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 71e89a93fe18..15970f1b09d2 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -153,10 +153,10 @@ intel_engine_setup(struct drm_i915_private *dev_priv, int intel_engines_init_early(struct drm_i915_private *dev_priv) { struct intel_device_info *device_info = mkwrite_device_info(dev_priv); - unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask; - unsigned int mask = 0; + const unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask; struct intel_engine_cs *engine; enum intel_engine_id id; + unsigned int mask = 0; unsigned int i; int err; @@ -183,6 +183,12 @@ int intel_engines_init_early(struct drm_i915_private *dev_priv) if (WARN_ON(mask != ring_mask)) device_info->ring_mask = mask; + /* We always presume we have at least RCS available for later probing */ + if (WARN_ON(!HAS_ENGINE(dev_priv, RCS))) { + err = -ENODEV; + goto cleanup; + } + device_info->num_rings = hweight32(mask); return 0; -- cgit v1.2.3 From a8e9a419c337a655e23b4bab422e85e47ee86c92 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 11 Apr 2017 20:00:42 +0100 Subject: drm/i915: Lie and treat all engines as idle if wedged Similar to commit 8490ae207f1d ("drm/i915: Suppress busy status for engines if wedged") we also want to report intel_engine_is_idle() as true as well as the main intel_engines_are_idle(), as we now check that the engines are idle when overwriting the HWS page. This is not true whilst we are setting the device as wedged, at least according to our bookkeeping, so we have to lie to ourselves! [ 383.588601] [drm:i915_reset [i915]] *ERROR* Failed to reset chip: -110 [ 383.588685] ------------[ cut here ]------------ [ 383.588755] WARNING: CPU: 0 PID: 12 at drivers/gpu/drm/i915/intel_engine_cs.c:226 intel_engine_init_global_seqno+0x222/0x290 [i915] [ 383.588757] WARN_ON(!intel_engine_is_idle(engine)) [ 383.588759] Modules linked in: ctr ccm snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core arc4 iwldvm mac80211 snd_pcm snd_hwdep snd_seq_midi snd_seq_midi_event rfcomm bnep snd_rawmidi intel_powerclamp coretemp dm_multipath iwlwifi crct10dif_pclmul snd_seq crc32_pclmul ghash_clmulni_intel btusb aesni_intel btrtl btbcm aes_x86_64 crypto_simd cryptd btintel snd_timer glue_helper bluetooth intel_ips snd_seq_device cfg80211 snd soundcore binfmt_misc mei_me mei dm_mirror dm_region_hash dm_log i915 intel_gtt i2c_algo_bit drm_kms_helper cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea prime_numbers ahci libahci drm e1000e [ 383.588851] CPU: 0 PID: 12 Comm: migration/0 Not tainted 4.11.0-rc5+ #207 [ 383.588853] Hardware name: LENOVO 514328U/514328U, BIOS 6QET44WW (1.14 ) 04/20/2010 [ 383.588855] Call Trace: [ 383.588866] dump_stack+0x63/0x90 [ 383.588871] __warn+0xc7/0xf0 [ 383.588876] warn_slowpath_fmt+0x4a/0x50 [ 383.588883] ? set_next_entity+0x821/0x910 [ 383.588943] intel_engine_init_global_seqno+0x222/0x290 [i915] [ 383.588998] __i915_gem_set_wedged_BKL+0xa4/0x190 [i915] [ 383.589003] ? __switch_to+0x215/0x390 [ 383.589008] multi_cpu_stop+0xbb/0xe0 [ 383.589012] ? cpu_stop_queue_work+0x90/0x90 [ 383.589016] cpu_stopper_thread+0x82/0x110 [ 383.589021] smpboot_thread_fn+0x137/0x190 [ 383.589026] kthread+0xf7/0x130 [ 383.589030] ? sort_range+0x20/0x20 [ 383.589034] ? kthread_park+0x90/0x90 [ 383.589040] ret_from_fork+0x2c/0x40 Fixes: 2ca9faa551c4 ("drm/i915: Assert the engine is idle before overwiting the HWS") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Reviewed-by: Joonas Lahtinen Link: http://patchwork.freedesktop.org/patch/msgid/20170411190042.25662-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 15970f1b09d2..ee87ca7420de 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1131,6 +1131,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; + /* More white lies, if wedged, hw state is inconsistent */ + if (i915_terminally_wedged(&dev_priv->gpu_error)) + return true; + /* Any inflight/incomplete requests? */ if (!i915_seqno_passed(intel_engine_get_seqno(engine), intel_engine_last_submit(engine))) -- cgit v1.2.3 From 8968a3649a7c09a0c7f92001b0a599b93a4d9685 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 12 Apr 2017 00:44:26 +0100 Subject: drm/i915: Pretend the engine is always idle when mocking If we have a mock engine and it has no more requests in flight, report it as idle as there is no hardware to contradict us! Otherwise we attempt to query the hw that doesn't exist and find that the hw hasn't set its idle bit and we get upset. Signed-off-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/20170411234427.14841-2-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index ee87ca7420de..402769d9d840 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1140,6 +1140,9 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) intel_engine_last_submit(engine))) return false; + if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock)) + return true; + /* Interrupt/tasklet pending? */ if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) return false; -- cgit v1.2.3 From 546cdbc75b6a1cba6445896141c736b0a3070afc Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Apr 2017 09:31:13 +0100 Subject: drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno() The hangcheck runs independently to the main flow of seqno through the driver. However, we have an odd coupling of the seqno reset that is unwelcome, and if poked at just the right rate can cause spurious hangs (e.g. gem_exec_whisper) on an apparently idle engine. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/20170421083113.21321-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 402769d9d840..82a274b336c5 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -265,6 +265,7 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) struct drm_i915_private *dev_priv = engine->i915; GEM_BUG_ON(!intel_engine_is_idle(engine)); + GEM_BUG_ON(i915_gem_active_isset(&engine->timeline->last_request)); /* Our semaphore implementation is strictly monotonic (i.e. we proceed * so long as the semaphore value in the register/page is greater @@ -296,9 +297,6 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno); clear_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted); - GEM_BUG_ON(i915_gem_active_isset(&engine->timeline->last_request)); - engine->hangcheck.seqno = seqno; - /* After manually advancing the seqno, fake the interrupt in case * there are any waiters for that seqno. */ -- cgit v1.2.3 From 63ffbcdadcf2b5dde2cd6db6715fc94e77cd43b6 Mon Sep 17 00:00:00 2001 From: Joonas Lahtinen Date: Fri, 28 Apr 2017 10:53:36 +0300 Subject: drm/i915: Sanitize engine context sizes Pre-calculate engine context size based on engine class and device generation and store it in the engine instance. v2: - Squash and get rid of hw_context_size (Chris) v3: - Move after MMIO init for probing on Gen7 and 8 (Chris) - Retained rounding (Tvrtko) v4: - Rebase for deferred legacy context allocation Signed-off-by: Joonas Lahtinen Cc: Paulo Zanoni Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Cc: Tvrtko Ursulin Cc: Oscar Mateo Cc: Zhenyu Wang Cc: intel-gvt-dev@lists.freedesktop.org Acked-by: Tvrtko Ursulin Cc: Tvrtko Ursulin Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/gvt/scheduler.c | 6 +- drivers/gpu/drm/i915/i915_drv.c | 15 +++-- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_gem_context.c | 56 ++----------------- drivers/gpu/drm/i915/i915_guc_submission.c | 3 +- drivers/gpu/drm/i915/i915_reg.h | 10 ---- drivers/gpu/drm/i915/intel_engine_cs.c | 90 +++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_lrc.c | 54 +----------------- drivers/gpu/drm/i915/intel_lrc.h | 2 - drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 7 ++- 11 files changed, 112 insertions(+), 138 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index a77db2332e68..ac538dcfff61 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -69,8 +69,7 @@ static int populate_shadow_context(struct intel_vgpu_workload *workload) gvt_dbg_sched("ring id %d workload lrca %x", ring_id, workload->ctx_desc.lrca); - context_page_num = intel_lr_context_size( - gvt->dev_priv->engine[ring_id]); + context_page_num = gvt->dev_priv->engine[ring_id]->context_size; context_page_num = context_page_num >> PAGE_SHIFT; @@ -333,8 +332,7 @@ static void update_guest_context(struct intel_vgpu_workload *workload) gvt_dbg_sched("ring id %d workload lrca %x\n", ring_id, workload->ctx_desc.lrca); - context_page_num = intel_lr_context_size( - gvt->dev_priv->engine[ring_id]); + context_page_num = gvt->dev_priv->engine[ring_id]->context_size; context_page_num = context_page_num >> PAGE_SHIFT; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c7d68e789642..2d3c426465d3 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -835,10 +835,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, intel_uc_init_early(dev_priv); i915_memcpy_init_early(dev_priv); - ret = intel_engines_init_early(dev_priv); - if (ret) - return ret; - ret = i915_workqueues_init(dev_priv); if (ret < 0) goto err_engines; @@ -948,14 +944,21 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv) ret = i915_mmio_setup(dev_priv); if (ret < 0) - goto put_bridge; + goto err_bridge; intel_uncore_init(dev_priv); + + ret = intel_engines_init_mmio(dev_priv); + if (ret) + goto err_uncore; + i915_gem_init_mmio(dev_priv); return 0; -put_bridge: +err_uncore: + intel_uncore_fini(dev_priv); +err_bridge: pci_dev_put(dev_priv->bridge_dev); return ret; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d1f7c48e4ae3..e68edf1305cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2359,7 +2359,6 @@ struct drm_i915_private { */ struct mutex av_mutex; - uint32_t hw_context_size; struct list_head context_list; u32 fdi_rx_config; @@ -3023,7 +3022,7 @@ extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); -int intel_engines_init_early(struct drm_i915_private *dev_priv); +int intel_engines_init_mmio(struct drm_i915_private *dev_priv); int intel_engines_init(struct drm_i915_private *dev_priv); /* intel_hotplug.c */ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index d46a69d3d390..31a73c39239f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -92,33 +92,6 @@ #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1 -static int get_context_size(struct drm_i915_private *dev_priv) -{ - int ret; - u32 reg; - - switch (INTEL_GEN(dev_priv)) { - case 6: - reg = I915_READ(CXT_SIZE); - ret = GEN6_CXT_TOTAL_SIZE(reg) * 64; - break; - case 7: - reg = I915_READ(GEN7_CXT_SIZE); - if (IS_HASWELL(dev_priv)) - ret = HSW_CXT_TOTAL_SIZE; - else - ret = GEN7_CXT_TOTAL_SIZE(reg) * 64; - break; - case 8: - ret = GEN8_CXT_TOTAL_SIZE; - break; - default: - BUG(); - } - - return ret; -} - void i915_gem_context_free(struct kref *ctx_ref) { struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref); @@ -384,21 +357,6 @@ int i915_gem_context_init(struct drm_i915_private *dev_priv) BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX); ida_init(&dev_priv->context_hw_ida); - if (i915.enable_execlists) { - /* NB: intentionally left blank. We will allocate our own - * backing objects as we need them, thank you very much */ - dev_priv->hw_context_size = 0; - } else if (HAS_HW_CONTEXTS(dev_priv)) { - dev_priv->hw_context_size = - round_up(get_context_size(dev_priv), - I915_GTT_PAGE_SIZE); - if (dev_priv->hw_context_size > (1<<20)) { - DRM_DEBUG_DRIVER("Disabling HW Contexts; invalid size %d\n", - dev_priv->hw_context_size); - dev_priv->hw_context_size = 0; - } - } - ctx = i915_gem_create_context(dev_priv, NULL); if (IS_ERR(ctx)) { DRM_ERROR("Failed to create default global context (error %ld)\n", @@ -418,8 +376,8 @@ int i915_gem_context_init(struct drm_i915_private *dev_priv) GEM_BUG_ON(!i915_gem_context_is_kernel(ctx)); DRM_DEBUG_DRIVER("%s context support initialized\n", - i915.enable_execlists ? "LR" : - dev_priv->hw_context_size ? "HW" : "fake"); + dev_priv->engine[RCS]->context_size ? "logical" : + "fake"); return 0; } @@ -882,11 +840,6 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv) return 0; } -static bool contexts_enabled(struct drm_device *dev) -{ - return i915.enable_execlists || to_i915(dev)->hw_context_size; -} - static bool client_is_banned(struct drm_i915_file_private *file_priv) { return file_priv->context_bans > I915_MAX_CLIENT_CONTEXT_BANS; @@ -895,12 +848,13 @@ static bool client_is_banned(struct drm_i915_file_private *file_priv) int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { + struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_gem_context_create *args = data; struct drm_i915_file_private *file_priv = file->driver_priv; struct i915_gem_context *ctx; int ret; - if (!contexts_enabled(dev)) + if (!dev_priv->engine[RCS]->context_size) return -ENODEV; if (args->pad != 0) @@ -918,7 +872,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, if (ret) return ret; - ctx = i915_gem_create_context(to_i915(dev), file_priv); + ctx = i915_gem_create_context(dev_priv, file_priv); mutex_unlock(&dev->struct_mutex); if (IS_ERR(ctx)) return PTR_ERR(ctx); diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 4cc97bf1bdac..7e85b5ab8ae2 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -1051,8 +1051,7 @@ static int guc_ads_create(struct intel_guc *guc) dev_priv->engine[RCS]->status_page.ggtt_offset; for_each_engine(engine, dev_priv, id) - blob->ads.eng_state_size[engine->guc_id] = - intel_lr_context_size(engine); + blob->ads.eng_state_size[engine->guc_id] = engine->context_size; base = guc_ggtt_offset(vma); blob->ads.scheduler_policies = base + ptr_offset(blob, policies); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4c72adae368b..ee8170cda93e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3370,16 +3370,6 @@ enum skl_disp_power_wells { #define GEN7_CXT_VFSTATE_SIZE(ctx_reg) (((ctx_reg) >> 0) & 0x3f) #define GEN7_CXT_TOTAL_SIZE(ctx_reg) (GEN7_CXT_EXTENDED_SIZE(ctx_reg) + \ GEN7_CXT_VFSTATE_SIZE(ctx_reg)) -/* Haswell does have the CXT_SIZE register however it does not appear to be - * valid. Now, docs explain in dwords what is in the context object. The full - * size is 70720 bytes, however, the power context and execlist context will - * never be saved (power context is stored elsewhere, and execlists don't work - * on HSW) - so the final size, including the extra state required for the - * Resource Streamer, is 66944 bytes, which rounds to 17 pages. - */ -#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE) -/* Same as Haswell, but 72064 bytes now. */ -#define GEN8_CXT_TOTAL_SIZE (18 * PAGE_SIZE) enum { INTEL_ADVANCED_CONTEXT = 0, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 82a274b336c5..6d3d83876da9 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -26,6 +26,22 @@ #include "intel_ringbuffer.h" #include "intel_lrc.h" +/* Haswell does have the CXT_SIZE register however it does not appear to be + * valid. Now, docs explain in dwords what is in the context object. The full + * size is 70720 bytes, however, the power context and execlist context will + * never be saved (power context is stored elsewhere, and execlists don't work + * on HSW) - so the final size, including the extra state required for the + * Resource Streamer, is 66944 bytes, which rounds to 17 pages. + */ +#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE) +/* Same as Haswell, but 72064 bytes now. */ +#define GEN8_CXT_TOTAL_SIZE (18 * PAGE_SIZE) + +#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE) +#define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE) + +#define GEN8_LR_CONTEXT_OTHER_SIZE ( 2 * PAGE_SIZE) + struct engine_class_info { const char *name; int (*init_legacy)(struct intel_engine_cs *engine); @@ -107,6 +123,69 @@ static const struct engine_info intel_engines[] = { }, }; +/** + * ___intel_engine_context_size() - return the size of the context for an engine + * @dev_priv: i915 device private + * @class: engine class + * + * Each engine class may require a different amount of space for a context + * image. + * + * Return: size (in bytes) of an engine class specific context image + * + * Note: this size includes the HWSP, which is part of the context image + * in LRC mode, but does not include the "shared data page" used with + * GuC submission. The caller should account for this if using the GuC. + */ +static u32 +__intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class) +{ + u32 cxt_size; + + BUILD_BUG_ON(I915_GTT_PAGE_SIZE != PAGE_SIZE); + + switch (class) { + case RENDER_CLASS: + switch (INTEL_GEN(dev_priv)) { + default: + MISSING_CASE(INTEL_GEN(dev_priv)); + case 9: + return GEN9_LR_CONTEXT_RENDER_SIZE; + case 8: + return i915.enable_execlists ? + GEN8_LR_CONTEXT_RENDER_SIZE : + GEN8_CXT_TOTAL_SIZE; + case 7: + if (IS_HASWELL(dev_priv)) + return HSW_CXT_TOTAL_SIZE; + + cxt_size = I915_READ(GEN7_CXT_SIZE); + return round_up(GEN7_CXT_TOTAL_SIZE(cxt_size) * 64, + PAGE_SIZE); + case 6: + cxt_size = I915_READ(CXT_SIZE); + return round_up(GEN6_CXT_TOTAL_SIZE(cxt_size) * 64, + PAGE_SIZE); + case 5: + case 4: + case 3: + case 2: + /* For the special day when i810 gets merged. */ + case 1: + return 0; + } + break; + default: + MISSING_CASE(class); + case VIDEO_DECODE_CLASS: + case VIDEO_ENHANCEMENT_CLASS: + case COPY_ENGINE_CLASS: + if (INTEL_GEN(dev_priv) < 8) + return 0; + return GEN8_LR_CONTEXT_OTHER_SIZE; + } +} + static int intel_engine_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id) @@ -135,6 +214,11 @@ intel_engine_setup(struct drm_i915_private *dev_priv, engine->class = info->class; engine->instance = info->instance; + engine->context_size = __intel_engine_context_size(dev_priv, + engine->class); + if (WARN_ON(engine->context_size > BIT(20))) + engine->context_size = 0; + /* Nothing to do here, execute in order of dependencies */ engine->schedule = NULL; @@ -145,12 +229,12 @@ intel_engine_setup(struct drm_i915_private *dev_priv, } /** - * intel_engines_init_early() - allocate the Engine Command Streamers + * intel_engines_init_mmio() - allocate and prepare the Engine Command Streamers * @dev_priv: i915 device private * * Return: non-zero if the initialization failed. */ -int intel_engines_init_early(struct drm_i915_private *dev_priv) +int intel_engines_init_mmio(struct drm_i915_private *dev_priv) { struct intel_device_info *device_info = mkwrite_device_info(dev_priv); const unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask; @@ -200,7 +284,7 @@ cleanup: } /** - * intel_engines_init() - allocate, populate and init the Engine Command Streamers + * intel_engines_init() - init the Engine Command Streamers * @dev_priv: i915 device private * * Return: non-zero if the initialization failed. diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 5ec064a56a7d..0909549ad320 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -138,10 +138,6 @@ #include "i915_drv.h" #include "intel_mocs.h" -#define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE) -#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE) -#define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE) - #define RING_EXECLIST_QFULL (1 << 0x2) #define RING_EXECLIST1_VALID (1 << 0x3) #define RING_EXECLIST0_VALID (1 << 0x4) @@ -1918,53 +1914,6 @@ populate_lr_context(struct i915_gem_context *ctx, return 0; } -/** - * intel_lr_context_size() - return the size of the context for an engine - * @engine: which engine to find the context size for - * - * Each engine may require a different amount of space for a context image, - * so when allocating (or copying) an image, this function can be used to - * find the right size for the specific engine. - * - * Return: size (in bytes) of an engine-specific context image - * - * Note: this size includes the HWSP, which is part of the context image - * in LRC mode, but does not include the "shared data page" used with - * GuC submission. The caller should account for this if using the GuC. - */ -uint32_t intel_lr_context_size(struct intel_engine_cs *engine) -{ - struct drm_i915_private *dev_priv = engine->i915; - int ret; - - WARN_ON(INTEL_GEN(dev_priv) < 8); - - switch (engine->class) { - case RENDER_CLASS: - switch (INTEL_GEN(dev_priv)) { - default: - MISSING_CASE(INTEL_GEN(dev_priv)); - case 9: - ret = GEN9_LR_CONTEXT_RENDER_SIZE; - break; - case 8: - ret = GEN8_LR_CONTEXT_RENDER_SIZE; - break; - } - break; - - default: - MISSING_CASE(engine->class); - case VIDEO_DECODE_CLASS: - case VIDEO_ENHANCEMENT_CLASS: - case COPY_ENGINE_CLASS: - ret = GEN8_LR_CONTEXT_OTHER_SIZE; - break; - } - - return ret; -} - static int execlists_context_deferred_alloc(struct i915_gem_context *ctx, struct intel_engine_cs *engine) { @@ -1977,8 +1926,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx, WARN_ON(ce->state); - context_size = round_up(intel_lr_context_size(engine), - I915_GTT_PAGE_SIZE); + context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE); /* One extra page as the sharing data between driver and GuC */ context_size += PAGE_SIZE * LRC_PPHWSP_PN; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index e8015e7bf4e9..52b3a1fd4059 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -78,8 +78,6 @@ int logical_xcs_ring_init(struct intel_engine_cs *engine); struct drm_i915_private; struct i915_gem_context; -uint32_t intel_lr_context_size(struct intel_engine_cs *engine); - void intel_lr_context_resume(struct drm_i915_private *dev_priv); uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 61f612454ce7..29b5afac7856 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1444,7 +1444,7 @@ alloc_context_vma(struct intel_engine_cs *engine) struct drm_i915_gem_object *obj; struct i915_vma *vma; - obj = i915_gem_object_create(i915, i915->hw_context_size); + obj = i915_gem_object_create(i915, engine->context_size); if (IS_ERR(obj)) return ERR_CAST(obj); @@ -1487,7 +1487,7 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine, return 0; GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ - if (engine->id == RCS && !ce->state && engine->i915->hw_context_size) { + if (!ce->state && engine->context_size) { struct i915_vma *vma; vma = alloc_context_vma(engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 2506bbe26fa0..02d741ef99ad 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -196,13 +196,14 @@ struct intel_engine_cs { enum intel_engine_id id; unsigned int uabi_id; unsigned int hw_id; + unsigned int guc_id; u8 class; u8 instance; - - unsigned int guc_id; - u32 mmio_base; + u32 context_size; + u32 mmio_base; unsigned int irq_shift; + struct intel_ring *buffer; struct intel_timeline *timeline; -- cgit v1.2.3 From 266a240bf0abf1e00e72e571f3724ec753a35f19 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 4 May 2017 10:33:08 +0100 Subject: drm/i915: Use engine->context_pin() to report the intel_ring Since unifying ringbuffer/execlist submission to use engine->pin_context, we ensure that the intel_ring is available before we start constructing the request. We can therefore move the assignment of the request->ring to the central i915_gem_request_alloc() and not require it in every engine->request_alloc() callback. Another small step towards simplification (of the core, but at a cost of handling error pointers in less important callers of engine->pin_context). v2: Rearrange a few branches to reduce impact of PTR_ERR() on gcc's code generation. Signed-off-by: Chris Wilson Cc: Oscar Mateo Cc: Joonas Lahtinen Reviewed-by: Oscar Mateo Link: http://patchwork.freedesktop.org/patch/msgid/20170504093308.4137-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gvt/scheduler.c | 6 ++++-- drivers/gpu/drm/i915/i915_gem_request.c | 9 ++++++--- drivers/gpu/drm/i915/i915_perf.c | 13 ++++++------- drivers/gpu/drm/i915/intel_engine_cs.c | 7 ++++--- drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++--------- drivers/gpu/drm/i915/intel_ringbuffer.c | 25 +++++++++++++------------ drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++-- drivers/gpu/drm/i915/selftests/mock_engine.c | 8 ++++---- 8 files changed, 47 insertions(+), 42 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 1256fe21850b..6ae286cb5804 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -180,6 +180,7 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) struct intel_engine_cs *engine = dev_priv->engine[ring_id]; struct drm_i915_gem_request *rq; struct intel_vgpu *vgpu = workload->vgpu; + struct intel_ring *ring; int ret; gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n", @@ -198,8 +199,9 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) * shadow_ctx pages invalid. So gvt need to pin itself. After update * the guest context, gvt can unpin the shadow_ctx safely. */ - ret = engine->context_pin(engine, shadow_ctx); - if (ret) { + ring = engine->context_pin(engine, shadow_ctx); + if (IS_ERR(ring)) { + ret = PTR_ERR(ring); gvt_vgpu_err("fail to pin shadow context\n"); workload->status = ret; mutex_unlock(&dev_priv->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 9074303c8888..10361c7e3b37 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -551,6 +551,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, { struct drm_i915_private *dev_priv = engine->i915; struct drm_i915_gem_request *req; + struct intel_ring *ring; int ret; lockdep_assert_held(&dev_priv->drm.struct_mutex); @@ -565,9 +566,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, * GGTT space, so do this first before we reserve a seqno for * ourselves. */ - ret = engine->context_pin(engine, ctx); - if (ret) - return ERR_PTR(ret); + ring = engine->context_pin(engine, ctx); + if (IS_ERR(ring)) + return ERR_CAST(ring); + GEM_BUG_ON(!ring); ret = reserve_seqno(engine); if (ret) @@ -633,6 +635,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, req->i915 = dev_priv; req->engine = engine; req->ctx = ctx; + req->ring = ring; /* No zalloc, must clear what we need by hand */ req->global_seqno = 0; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 060b171480d5..cdac68580cb1 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -744,6 +744,7 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; struct intel_engine_cs *engine = dev_priv->engine[RCS]; + struct intel_ring *ring; int ret; ret = i915_mutex_lock_interruptible(&dev_priv->drm); @@ -755,9 +756,10 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) * * NB: implied RCS engine... */ - ret = engine->context_pin(engine, stream->ctx); - if (ret) - goto unlock; + ring = engine->context_pin(engine, stream->ctx); + mutex_unlock(&dev_priv->drm.struct_mutex); + if (IS_ERR(ring)) + return PTR_ERR(ring); /* Explicitly track the ID (instead of calling i915_ggtt_offset() * on the fly) considering the difference with gen8+ and @@ -766,10 +768,7 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) dev_priv->perf.oa.specific_ctx_id = i915_ggtt_offset(stream->ctx->engine[engine->id].state); -unlock: - mutex_unlock(&dev_priv->drm.struct_mutex); - - return ret; + return 0; } /** diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 6d3d83876da9..483ed7635692 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -469,6 +469,7 @@ static void intel_engine_cleanup_scratch(struct intel_engine_cs *engine) */ int intel_engine_init_common(struct intel_engine_cs *engine) { + struct intel_ring *ring; int ret; engine->set_default_submission(engine); @@ -480,9 +481,9 @@ int intel_engine_init_common(struct intel_engine_cs *engine) * be available. To avoid this we always pin the default * context. */ - ret = engine->context_pin(engine, engine->i915->kernel_context); - if (ret) - return ret; + ring = engine->context_pin(engine, engine->i915->kernel_context); + if (IS_ERR(ring)) + return PTR_ERR(ring); ret = intel_engine_init_breadcrumbs(engine); if (ret) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 0909549ad320..319d9a8f37ca 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -740,8 +740,9 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio) /* XXX Do we need to preempt to make room for us and our deps? */ } -static int execlists_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static struct intel_ring * +execlists_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; unsigned int flags; @@ -750,8 +751,8 @@ static int execlists_context_pin(struct intel_engine_cs *engine, lockdep_assert_held(&ctx->i915->drm.struct_mutex); - if (ce->pin_count++) - return 0; + if (likely(ce->pin_count++)) + goto out; GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ if (!ce->state) { @@ -788,7 +789,8 @@ static int execlists_context_pin(struct intel_engine_cs *engine, ce->state->obj->mm.dirty = true; i915_gem_context_get(ctx); - return 0; +out: + return ce->ring; unpin_map: i915_gem_object_unpin_map(ce->state->obj); @@ -796,7 +798,7 @@ unpin_vma: __i915_vma_unpin(ce->state); err: ce->pin_count = 0; - return ret; + return ERR_PTR(ret); } static void execlists_context_unpin(struct intel_engine_cs *engine, @@ -833,9 +835,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request) */ request->reserved_space += EXECLISTS_REQUEST_SIZE; - GEM_BUG_ON(!ce->ring); - request->ring = ce->ring; - if (i915.enable_guc_submission) { /* * Check that the GuC has space for the request before diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 29b5afac7856..3ce1c87dec46 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1475,16 +1475,17 @@ alloc_context_vma(struct intel_engine_cs *engine) return vma; } -static int intel_ring_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static struct intel_ring * +intel_ring_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; int ret; lockdep_assert_held(&ctx->i915->drm.struct_mutex); - if (ce->pin_count++) - return 0; + if (likely(ce->pin_count++)) + goto out; GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ if (!ce->state && engine->context_size) { @@ -1493,7 +1494,7 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine, vma = alloc_context_vma(engine); if (IS_ERR(vma)) { ret = PTR_ERR(vma); - goto error; + goto err; } ce->state = vma; @@ -1502,7 +1503,7 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine, if (ce->state) { ret = context_pin(ctx); if (ret) - goto error; + goto err; ce->state->obj->mm.dirty = true; } @@ -1518,11 +1519,14 @@ static int intel_ring_context_pin(struct intel_engine_cs *engine, ce->initialised = true; i915_gem_context_get(ctx); - return 0; -error: +out: + /* One ringbuffer to rule them all */ + return engine->buffer; + +err: ce->pin_count = 0; - return ret; + return ERR_PTR(ret); } static void intel_ring_context_unpin(struct intel_engine_cs *engine, @@ -1634,9 +1638,6 @@ static int ring_request_alloc(struct drm_i915_gem_request *request) */ request->reserved_space += LEGACY_REQUEST_SIZE; - GEM_BUG_ON(!request->engine->buffer); - request->ring = request->engine->buffer; - cs = intel_ring_begin(request, 0); if (IS_ERR(cs)) return PTR_ERR(cs); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 02d741ef99ad..600713b29d79 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -271,8 +271,8 @@ struct intel_engine_cs { void (*set_default_submission)(struct intel_engine_cs *engine); - int (*context_pin)(struct intel_engine_cs *engine, - struct i915_gem_context *ctx); + struct intel_ring *(*context_pin)(struct intel_engine_cs *engine, + struct i915_gem_context *ctx); void (*context_unpin)(struct intel_engine_cs *engine, struct i915_gem_context *ctx); int (*request_alloc)(struct drm_i915_gem_request *req); diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index b8e53bdc3cc4..5b18a2dc19a8 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -52,11 +52,12 @@ static void hw_delay_complete(unsigned long data) spin_unlock(&engine->hw_lock); } -static int mock_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static struct intel_ring * +mock_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { i915_gem_context_get(ctx); - return 0; + return engine->buffer; } static void mock_context_unpin(struct intel_engine_cs *engine, @@ -72,7 +73,6 @@ static int mock_request_alloc(struct drm_i915_gem_request *request) INIT_LIST_HEAD(&mock->link); mock->delay = 0; - request->ring = request->engine->buffer; return 0; } -- cgit v1.2.3 From 0b71cea29fc29bbd8e9dd9c641fee6bd75f68274 Mon Sep 17 00:00:00 2001 From: Arkadiusz Hiler Date: Fri, 12 May 2017 13:20:15 +0200 Subject: drm/i915/gen9: Reintroduce WaEnableYV12BugFixInHalfSliceChicken7 This basically reverts commit 465418c6064c ("drm/i915/gen9: Remove WaEnableYV12BugFixInHalfSliceChicken7") with small addition - marking it as affecting GLK as well. It was incorrectly considered fixed in production steppings. References: HSD#2126385, HSD#2131381, HSDES#1504433555, BSID#0764 Cc: Mika Kuoppala Cc: Jeff McGee Signed-off-by: Arkadiusz Hiler Reviewed-by: Mika Kuoppala [Mika: s/KBL/GLK on commit message] Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/20170512112015.19082-1-arkadiusz.hiler@intel.com --- drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 483ed7635692..49c23152500e 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -851,8 +851,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) */ } + /* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt,kbl,glk */ /* WaEnableSamplerGPGPUPreemptionSupport:skl,bxt,kbl */ WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7, + GEN9_ENABLE_YV12_BUGFIX | GEN9_ENABLE_GPGPU_PREEMPTION); /* Wa4x4STCOptimizationDisable:skl,bxt,kbl,glk */ -- cgit v1.2.3 From 77f0d0e925e8a0f17a927a1f4e266d1f0e95cb72 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 17 May 2017 13:10:00 +0100 Subject: drm/i915/execlists: Pack the count into the low bits of the port.request add/remove: 1/1 grow/shrink: 5/4 up/down: 391/-578 (-187) function old new delta execlists_submit_ports 262 471 +209 port_assign.isra - 136 +136 capture 6344 6359 +15 reset_common_ring 438 452 +14 execlists_submit_request 228 238 +10 gen8_init_common_ring 334 341 +7 intel_engine_is_idle 106 105 -1 i915_engine_info 2314 2290 -24 __i915_gem_set_wedged_BKL 485 411 -74 intel_lrc_irq_handler 1789 1604 -185 execlists_update_context 294 - -294 The most important change there is the improve to the intel_lrc_irq_handler and excclist_submit_ports (net improvement since execlists_update_context is now inlined). v2: Use the port_api() for guc as well (even though currently we do not pack any counters in there, yet) and hide all port->request_count inside the helpers. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170517121007.27224-5-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++---- drivers/gpu/drm/i915/i915_gem.c | 6 +- drivers/gpu/drm/i915/i915_gpu_error.c | 13 +++- drivers/gpu/drm/i915/i915_guc_submission.c | 34 +++++--- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_lrc.c | 121 ++++++++++++++++------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 11 ++- 7 files changed, 126 insertions(+), 93 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 76abff186d01..e08ac708e547 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3353,6 +3353,7 @@ static int i915_engine_info(struct seq_file *m, void *unused) if (i915.enable_execlists) { u32 ptr, read, write; struct rb_node *rb; + unsigned int idx; seq_printf(m, "\tExeclist status: 0x%08x %08x\n", I915_READ(RING_EXECLIST_STATUS_LO(engine)), @@ -3370,8 +3371,7 @@ static int i915_engine_info(struct seq_file *m, void *unused) if (read > write) write += GEN8_CSB_ENTRIES; while (read < write) { - unsigned int idx = ++read % GEN8_CSB_ENTRIES; - + idx = ++read % GEN8_CSB_ENTRIES; seq_printf(m, "\tExeclist CSB[%d]: 0x%08x, context: %d\n", idx, I915_READ(RING_CONTEXT_STATUS_BUF_LO(engine, idx)), @@ -3379,21 +3379,19 @@ static int i915_engine_info(struct seq_file *m, void *unused) } rcu_read_lock(); - rq = READ_ONCE(engine->execlist_port[0].request); - if (rq) { - seq_printf(m, "\t\tELSP[0] count=%d, ", - engine->execlist_port[0].count); - print_request(m, rq, "rq: "); - } else { - seq_printf(m, "\t\tELSP[0] idle\n"); - } - rq = READ_ONCE(engine->execlist_port[1].request); - if (rq) { - seq_printf(m, "\t\tELSP[1] count=%d, ", - engine->execlist_port[1].count); - print_request(m, rq, "rq: "); - } else { - seq_printf(m, "\t\tELSP[1] idle\n"); + for (idx = 0; idx < ARRAY_SIZE(engine->execlist_port); idx++) { + unsigned int count; + + rq = port_unpack(&engine->execlist_port[idx], + &count); + if (rq) { + seq_printf(m, "\t\tELSP[%d] count=%d, ", + idx, count); + print_request(m, rq, "rq: "); + } else { + seq_printf(m, "\t\tELSP[%d] idle\n", + idx); + } } rcu_read_unlock(); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b63c3d1ccd9f..75d7575b81f4 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3019,12 +3019,14 @@ static void engine_set_wedged(struct intel_engine_cs *engine) */ if (i915.enable_execlists) { + struct execlist_port *port = engine->execlist_port; unsigned long flags; + unsigned int n; spin_lock_irqsave(&engine->timeline->lock, flags); - i915_gem_request_put(engine->execlist_port[0].request); - i915_gem_request_put(engine->execlist_port[1].request); + for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) + i915_gem_request_put(port_request(&port[n])); memset(engine->execlist_port, 0, sizeof(engine->execlist_port)); engine->execlist_queue = RB_ROOT; engine->execlist_first = NULL; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ec526d92f908..e18f350bc364 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1324,12 +1324,17 @@ static void engine_record_requests(struct intel_engine_cs *engine, static void error_record_engine_execlists(struct intel_engine_cs *engine, struct drm_i915_error_engine *ee) { + const struct execlist_port *port = engine->execlist_port; unsigned int n; - for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) - if (engine->execlist_port[n].request) - record_request(engine->execlist_port[n].request, - &ee->execlist[n]); + for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) { + struct drm_i915_gem_request *rq = port_request(&port[n]); + + if (!rq) + break; + + record_request(rq, &ee->execlist[n]); + } } static void record_context(struct drm_i915_error_context *e, diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 7e85b5ab8ae2..014cbd1a841e 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -653,10 +653,22 @@ static void nested_enable_signaling(struct drm_i915_gem_request *rq) spin_unlock(&rq->lock); } +static void port_assign(struct execlist_port *port, + struct drm_i915_gem_request *rq) +{ + GEM_BUG_ON(rq == port_request(port)); + + if (port_isset(port)) + i915_gem_request_put(port_request(port)); + + port_set(port, i915_gem_request_get(rq)); + nested_enable_signaling(rq); +} + static bool i915_guc_dequeue(struct intel_engine_cs *engine) { struct execlist_port *port = engine->execlist_port; - struct drm_i915_gem_request *last = port[0].request; + struct drm_i915_gem_request *last = port_request(port); struct rb_node *rb; bool submit = false; @@ -670,8 +682,7 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) if (port != engine->execlist_port) break; - i915_gem_request_assign(&port->request, last); - nested_enable_signaling(last); + port_assign(port, last); port++; } @@ -681,13 +692,12 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) rq->priotree.priority = INT_MAX; i915_guc_submit(rq); - trace_i915_gem_request_in(rq, port - engine->execlist_port); + trace_i915_gem_request_in(rq, port_index(port, engine)); last = rq; submit = true; } if (submit) { - i915_gem_request_assign(&port->request, last); - nested_enable_signaling(last); + port_assign(port, last); engine->execlist_first = rb; } spin_unlock_irq(&engine->timeline->lock); @@ -703,17 +713,19 @@ static void i915_guc_irq_handler(unsigned long data) bool submit; do { - rq = port[0].request; + rq = port_request(&port[0]); while (rq && i915_gem_request_completed(rq)) { trace_i915_gem_request_out(rq); i915_gem_request_put(rq); - port[0].request = port[1].request; - port[1].request = NULL; - rq = port[0].request; + + port[0] = port[1]; + memset(&port[1], 0, sizeof(port[1])); + + rq = port_request(&port[0]); } submit = false; - if (!port[1].request) + if (!port_count(&port[1])) submit = i915_guc_dequeue(engine); } while (submit); } diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 49c23152500e..e312decccaf1 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1233,7 +1233,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) return false; /* Both ports drained, no more ELSP submission? */ - if (engine->execlist_port[0].request) + if (port_request(&engine->execlist_port[0])) return false; /* Ring stopped? */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 9a1192d61538..53ec0d5713ad 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -337,39 +337,32 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq) static void execlists_submit_ports(struct intel_engine_cs *engine) { - struct drm_i915_private *dev_priv = engine->i915; struct execlist_port *port = engine->execlist_port; u32 __iomem *elsp = - dev_priv->regs + i915_mmio_reg_offset(RING_ELSP(engine)); - u64 desc[2]; - - GEM_BUG_ON(port[0].count > 1); - if (!port[0].count) - execlists_context_status_change(port[0].request, - INTEL_CONTEXT_SCHEDULE_IN); - desc[0] = execlists_update_context(port[0].request); - GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); - port[0].count++; - - if (port[1].request) { - GEM_BUG_ON(port[1].count); - execlists_context_status_change(port[1].request, - INTEL_CONTEXT_SCHEDULE_IN); - desc[1] = execlists_update_context(port[1].request); - GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1])); - port[1].count = 1; - } else { - desc[1] = 0; - } - GEM_BUG_ON(desc[0] == desc[1]); + engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); + unsigned int n; - /* You must always write both descriptors in the order below. */ - writel(upper_32_bits(desc[1]), elsp); - writel(lower_32_bits(desc[1]), elsp); + for (n = ARRAY_SIZE(engine->execlist_port); n--; ) { + struct drm_i915_gem_request *rq; + unsigned int count; + u64 desc; + + rq = port_unpack(&port[n], &count); + if (rq) { + GEM_BUG_ON(count > !n); + if (!count++) + execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); + port_set(&port[n], port_pack(rq, count)); + desc = execlists_update_context(rq); + GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc)); + } else { + GEM_BUG_ON(!n); + desc = 0; + } - writel(upper_32_bits(desc[0]), elsp); - /* The context is automatically loaded after the following */ - writel(lower_32_bits(desc[0]), elsp); + writel(upper_32_bits(desc), elsp); + writel(lower_32_bits(desc), elsp); + } } static bool ctx_single_port_submission(const struct i915_gem_context *ctx) @@ -390,6 +383,17 @@ static bool can_merge_ctx(const struct i915_gem_context *prev, return true; } +static void port_assign(struct execlist_port *port, + struct drm_i915_gem_request *rq) +{ + GEM_BUG_ON(rq == port_request(port)); + + if (port_isset(port)) + i915_gem_request_put(port_request(port)); + + port_set(port, port_pack(i915_gem_request_get(rq), port_count(port))); +} + static void execlists_dequeue(struct intel_engine_cs *engine) { struct drm_i915_gem_request *last; @@ -397,7 +401,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) struct rb_node *rb; bool submit = false; - last = port->request; + last = port_request(port); if (last) /* WaIdleLiteRestore:bdw,skl * Apply the wa NOOPs to prevent ring:HEAD == req:TAIL @@ -407,7 +411,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) */ last->tail = last->wa_tail; - GEM_BUG_ON(port[1].request); + GEM_BUG_ON(port_isset(&port[1])); /* Hardware submission is through 2 ports. Conceptually each port * has a (RING_START, RING_HEAD, RING_TAIL) tuple. RING_START is @@ -464,7 +468,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(last->ctx == cursor->ctx); - i915_gem_request_assign(&port->request, last); + if (submit) + port_assign(port, last); port++; } @@ -474,12 +479,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine) cursor->priotree.priority = INT_MAX; __i915_gem_request_submit(cursor); - trace_i915_gem_request_in(cursor, port - engine->execlist_port); + trace_i915_gem_request_in(cursor, port_index(port, engine)); last = cursor; submit = true; } if (submit) { - i915_gem_request_assign(&port->request, last); + port_assign(port, last); engine->execlist_first = rb; } spin_unlock_irq(&engine->timeline->lock); @@ -488,16 +493,11 @@ static void execlists_dequeue(struct intel_engine_cs *engine) execlists_submit_ports(engine); } -static bool execlists_elsp_idle(struct intel_engine_cs *engine) -{ - return !engine->execlist_port[0].request; -} - static bool execlists_elsp_ready(const struct intel_engine_cs *engine) { const struct execlist_port *port = engine->execlist_port; - return port[0].count + port[1].count < 2; + return port_count(&port[0]) + port_count(&port[1]) < 2; } /* @@ -547,7 +547,9 @@ static void intel_lrc_irq_handler(unsigned long data) tail = GEN8_CSB_WRITE_PTR(head); head = GEN8_CSB_READ_PTR(head); while (head != tail) { + struct drm_i915_gem_request *rq; unsigned int status; + unsigned int count; if (++head == GEN8_CSB_ENTRIES) head = 0; @@ -575,22 +577,26 @@ static void intel_lrc_irq_handler(unsigned long data) /* Check the context/desc id for this event matches */ GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) != - port[0].context_id); + port->context_id); - GEM_BUG_ON(port[0].count == 0); - if (--port[0].count == 0) { + rq = port_unpack(port, &count); + GEM_BUG_ON(count == 0); + if (--count == 0) { GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED); - GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); - execlists_context_status_change(port[0].request, - INTEL_CONTEXT_SCHEDULE_OUT); + GEM_BUG_ON(!i915_gem_request_completed(rq)); + execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT); + + trace_i915_gem_request_out(rq); + i915_gem_request_put(rq); - trace_i915_gem_request_out(port[0].request); - i915_gem_request_put(port[0].request); port[0] = port[1]; memset(&port[1], 0, sizeof(port[1])); + } else { + port_set(port, port_pack(rq, count)); } - GEM_BUG_ON(port[0].count == 0 && + /* After the final element, the hw should be idle */ + GEM_BUG_ON(port_count(port) == 0 && !(status & GEN8_CTX_STATUS_ACTIVE_IDLE)); } @@ -1147,6 +1153,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) struct drm_i915_private *dev_priv = engine->i915; struct execlist_port *port = engine->execlist_port; unsigned int n; + bool submit; int ret; ret = intel_mocs_init_engine(engine); @@ -1168,19 +1175,21 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) /* After a GPU reset, we may have requests to replay */ clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); + submit = false; for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) { - if (!port[n].request) + if (!port_isset(&port[n])) break; DRM_DEBUG_DRIVER("Restarting %s:%d from 0x%x\n", engine->name, n, - port[n].request->global_seqno); + port_request(&port[n])->global_seqno); /* Discard the current inflight count */ - port[n].count = 0; + port_set(&port[n], port_request(&port[n])); + submit = true; } - if (!i915.enable_guc_submission && !execlists_elsp_idle(engine)) + if (submit && !i915.enable_guc_submission) execlists_submit_ports(engine); return 0; @@ -1258,13 +1267,13 @@ static void reset_common_ring(struct intel_engine_cs *engine, intel_ring_update_space(request->ring); /* Catch up with any missed context-switch interrupts */ - if (request->ctx != port[0].request->ctx) { - i915_gem_request_put(port[0].request); + if (request->ctx != port_request(port)->ctx) { + i915_gem_request_put(port_request(port)); port[0] = port[1]; memset(&port[1], 0, sizeof(port[1])); } - GEM_BUG_ON(request->ctx != port[0].request->ctx); + GEM_BUG_ON(request->ctx != port_request(port)->ctx); /* Reset WaIdleLiteRestore:bdw,skl as well */ request->tail = diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index ec16fb6fde62..162f0a9c6abe 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -368,8 +368,15 @@ struct intel_engine_cs { /* Execlists */ struct tasklet_struct irq_tasklet; struct execlist_port { - struct drm_i915_gem_request *request; - unsigned int count; + struct drm_i915_gem_request *request_count; +#define EXECLIST_COUNT_BITS 2 +#define port_request(p) ptr_mask_bits((p)->request_count, EXECLIST_COUNT_BITS) +#define port_count(p) ptr_unmask_bits((p)->request_count, EXECLIST_COUNT_BITS) +#define port_pack(rq, count) ptr_pack_bits(rq, count, EXECLIST_COUNT_BITS) +#define port_unpack(p, count) ptr_unpack_bits((p)->request_count, count, EXECLIST_COUNT_BITS) +#define port_set(p, packed) ((p)->request_count = (packed)) +#define port_isset(p) ((p)->request_count) +#define port_index(p, e) ((p) - (e)->execlist_port) GEM_DEBUG_DECL(u32 context_id); } execlist_port[2]; struct rb_root execlist_queue; -- cgit v1.2.3 From 6c067579e69b42bff476959fd7bb561ffa3f11e0 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 17 May 2017 13:10:03 +0100 Subject: drm/i915: Split execlist priority queue into rbtree + linked list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All the requests at the same priority are executed in FIFO order. They do not need to be stored in the rbtree themselves, as they are a simple list within a level. If we move the requests at one priority into a list, we can then reduce the rbtree to the set of priorities. This should keep the height of the rbtree small, as the number of active priorities can not exceed the number of active requests and should be typically only a few. Currently, we have ~2k possible different priority levels, that may increase to allow even more fine grained selection. Allocating those in advance seems a waste (and may be impossible), so we opt for allocating upon first use, and freeing after its requests are depleted. To avoid the possibility of an allocation failure causing us to lose a request, we preallocate the default priority (0) and bump any request to that priority if we fail to allocate it the appropriate plist. Having a request (that is ready to run, so not leading to corruption) execute out-of-order is better than leaking the request (and its dependency tree) entirely. There should be a benefit to reducing execlists_dequeue() to principally using a simple list (and reducing the frequency of both rbtree iteration and balancing on erase) but for typical workloads, request coalescing should be small enough that we don't notice any change. The main gain is from improving PI calls to schedule, and the explicit list within a level should make request unwinding simpler (we just need to insert at the head of the list rather than the tail and not have to make the rbtree search more complicated). v2: Avoid use-after-free when deleting a depleted priolist v3: Michał found the solution to handling the allocation failure gracefully. If we disable all priority scheduling following the allocation failure, those requests will be executed in fifo and we will ensure that this request and its dependencies are in strict fifo (even when it doesn't realise it is only a single list). Normal scheduling is restored once we know the device is idle, until the next failure! Suggested-by: Michał Wajdeczko Signed-off-by: Chris Wilson Cc: Michał Winiarski Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Reviewed-by: Michał Winiarski Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170517121007.27224-8-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_debugfs.c | 11 +- drivers/gpu/drm/i915/i915_gem.c | 7 +- drivers/gpu/drm/i915/i915_gem_request.c | 4 +- drivers/gpu/drm/i915/i915_gem_request.h | 2 +- drivers/gpu/drm/i915/i915_guc_submission.c | 50 ++++---- drivers/gpu/drm/i915/i915_utils.h | 9 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 12 ++ drivers/gpu/drm/i915/intel_lrc.c | 183 +++++++++++++++++++---------- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 ++ 9 files changed, 192 insertions(+), 95 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e08ac708e547..8abb93994c48 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3352,7 +3352,6 @@ static int i915_engine_info(struct seq_file *m, void *unused) if (i915.enable_execlists) { u32 ptr, read, write; - struct rb_node *rb; unsigned int idx; seq_printf(m, "\tExeclist status: 0x%08x %08x\n", @@ -3396,9 +3395,13 @@ static int i915_engine_info(struct seq_file *m, void *unused) rcu_read_unlock(); spin_lock_irq(&engine->timeline->lock); - for (rb = engine->execlist_first; rb; rb = rb_next(rb)) { - rq = rb_entry(rb, typeof(*rq), priotree.node); - print_request(m, rq, "\t\tQ "); + for (rb = engine->execlist_first; rb; rb = rb_next(rb)){ + struct i915_priolist *p = + rb_entry(rb, typeof(*p), node); + + list_for_each_entry(rq, &p->requests, + priotree.link) + print_request(m, rq, "\t\tQ "); } spin_unlock_irq(&engine->timeline->lock); } else if (INTEL_GEN(dev_priv) > 6) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 75d7575b81f4..3d9161c8c1a1 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3155,8 +3155,6 @@ i915_gem_idle_work_handler(struct work_struct *work) struct drm_i915_private *dev_priv = container_of(work, typeof(*dev_priv), gt.idle_work.work); struct drm_device *dev = &dev_priv->drm; - struct intel_engine_cs *engine; - enum intel_engine_id id; bool rearm_hangcheck; if (!READ_ONCE(dev_priv->gt.awake)) @@ -3194,10 +3192,7 @@ i915_gem_idle_work_handler(struct work_struct *work) if (wait_for(intel_engines_are_idle(dev_priv), 10)) DRM_ERROR("Timeout waiting for engines to idle\n"); - for_each_engine(engine, dev_priv, id) { - intel_engine_disarm_breadcrumbs(engine); - i915_gem_batch_pool_fini(&engine->batch_pool); - } + intel_engines_mark_idle(dev_priv); i915_gem_timelines_mark_idle(dev_priv); GEM_BUG_ON(!dev_priv->gt.awake); diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 10361c7e3b37..1ccf2522cdfd 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -159,7 +159,7 @@ i915_priotree_fini(struct drm_i915_private *i915, struct i915_priotree *pt) { struct i915_dependency *dep, *next; - GEM_BUG_ON(!RB_EMPTY_NODE(&pt->node)); + GEM_BUG_ON(!list_empty(&pt->link)); /* Everyone we depended upon (the fences we wait to be signaled) * should retire before us and remove themselves from our list. @@ -185,7 +185,7 @@ i915_priotree_init(struct i915_priotree *pt) { INIT_LIST_HEAD(&pt->signalers_list); INIT_LIST_HEAD(&pt->waiters_list); - RB_CLEAR_NODE(&pt->node); + INIT_LIST_HEAD(&pt->link); pt->priority = INT_MIN; } diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index 0ecfc5e2d707..8c508bd9088e 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -67,7 +67,7 @@ struct i915_dependency { struct i915_priotree { struct list_head signalers_list; /* those before us, we depend upon */ struct list_head waiters_list; /* those after us, they depend upon us */ - struct rb_node node; + struct list_head link; int priority; #define I915_PRIORITY_MAX 1024 #define I915_PRIORITY_NORMAL 0 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 014cbd1a841e..3b9cdb0907c2 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -674,32 +674,42 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) spin_lock_irq(&engine->timeline->lock); rb = engine->execlist_first; + GEM_BUG_ON(rb_first(&engine->execlist_queue) != rb); while (rb) { - struct drm_i915_gem_request *rq = - rb_entry(rb, typeof(*rq), priotree.node); - - if (last && rq->ctx != last->ctx) { - if (port != engine->execlist_port) - break; - - port_assign(port, last); - port++; + struct i915_priolist *p = rb_entry(rb, typeof(*p), node); + struct drm_i915_gem_request *rq, *rn; + + list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) { + if (last && rq->ctx != last->ctx) { + if (port != engine->execlist_port) { + __list_del_many(&p->requests, + &rq->priotree.link); + goto done; + } + + port_assign(port, last); + port++; + } + + INIT_LIST_HEAD(&rq->priotree.link); + rq->priotree.priority = INT_MAX; + + i915_guc_submit(rq); + trace_i915_gem_request_in(rq, port_index(port, engine)); + last = rq; + submit = true; } rb = rb_next(rb); - rb_erase(&rq->priotree.node, &engine->execlist_queue); - RB_CLEAR_NODE(&rq->priotree.node); - rq->priotree.priority = INT_MAX; - - i915_guc_submit(rq); - trace_i915_gem_request_in(rq, port_index(port, engine)); - last = rq; - submit = true; + rb_erase(&p->node, &engine->execlist_queue); + INIT_LIST_HEAD(&p->requests); + if (p->priority != I915_PRIORITY_NORMAL) + kfree(p); } - if (submit) { +done: + engine->execlist_first = rb; + if (submit) port_assign(port, last); - engine->execlist_first = rb; - } spin_unlock_irq(&engine->timeline->lock); return submit; diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index d9df23795f9a..16ecd1ab108d 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -105,4 +105,13 @@ __idx; \ }) +#include + +static inline void __list_del_many(struct list_head *head, + struct list_head *first) +{ + first->prev = head; + WRITE_ONCE(head->next, first); +} + #endif /* !__I915_UTILS_H */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index e312decccaf1..413bfd8d4bf4 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1274,6 +1274,18 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) engine->set_default_submission(engine); } +void intel_engines_mark_idle(struct drm_i915_private *i915) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + + for_each_engine(engine, i915, id) { + intel_engine_disarm_breadcrumbs(engine); + i915_gem_batch_pool_fini(&engine->batch_pool); + engine->no_priolist = false; + } +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 53ec0d5713ad..626db6185a21 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -436,57 +436,75 @@ static void execlists_dequeue(struct intel_engine_cs *engine) spin_lock_irq(&engine->timeline->lock); rb = engine->execlist_first; + GEM_BUG_ON(rb_first(&engine->execlist_queue) != rb); while (rb) { - struct drm_i915_gem_request *cursor = - rb_entry(rb, typeof(*cursor), priotree.node); - - /* Can we combine this request with the current port? It has to - * be the same context/ringbuffer and not have any exceptions - * (e.g. GVT saying never to combine contexts). - * - * If we can combine the requests, we can execute both by - * updating the RING_TAIL to point to the end of the second - * request, and so we never need to tell the hardware about - * the first. - */ - if (last && !can_merge_ctx(cursor->ctx, last->ctx)) { - /* If we are on the second port and cannot combine - * this request with the last, then we are done. - */ - if (port != engine->execlist_port) - break; - - /* If GVT overrides us we only ever submit port[0], - * leaving port[1] empty. Note that we also have - * to be careful that we don't queue the same - * context (even though a different request) to - * the second port. + struct i915_priolist *p = rb_entry(rb, typeof(*p), node); + struct drm_i915_gem_request *rq, *rn; + + list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) { + /* + * Can we combine this request with the current port? + * It has to be the same context/ringbuffer and not + * have any exceptions (e.g. GVT saying never to + * combine contexts). + * + * If we can combine the requests, we can execute both + * by updating the RING_TAIL to point to the end of the + * second request, and so we never need to tell the + * hardware about the first. */ - if (ctx_single_port_submission(last->ctx) || - ctx_single_port_submission(cursor->ctx)) - break; + if (last && !can_merge_ctx(rq->ctx, last->ctx)) { + /* + * If we are on the second port and cannot + * combine this request with the last, then we + * are done. + */ + if (port != engine->execlist_port) { + __list_del_many(&p->requests, + &rq->priotree.link); + goto done; + } + + /* + * If GVT overrides us we only ever submit + * port[0], leaving port[1] empty. Note that we + * also have to be careful that we don't queue + * the same context (even though a different + * request) to the second port. + */ + if (ctx_single_port_submission(last->ctx) || + ctx_single_port_submission(rq->ctx)) { + __list_del_many(&p->requests, + &rq->priotree.link); + goto done; + } + + GEM_BUG_ON(last->ctx == rq->ctx); + + if (submit) + port_assign(port, last); + port++; + } - GEM_BUG_ON(last->ctx == cursor->ctx); + INIT_LIST_HEAD(&rq->priotree.link); + rq->priotree.priority = INT_MAX; - if (submit) - port_assign(port, last); - port++; + __i915_gem_request_submit(rq); + trace_i915_gem_request_in(rq, port_index(port, engine)); + last = rq; + submit = true; } rb = rb_next(rb); - rb_erase(&cursor->priotree.node, &engine->execlist_queue); - RB_CLEAR_NODE(&cursor->priotree.node); - cursor->priotree.priority = INT_MAX; - - __i915_gem_request_submit(cursor); - trace_i915_gem_request_in(cursor, port_index(port, engine)); - last = cursor; - submit = true; + rb_erase(&p->node, &engine->execlist_queue); + INIT_LIST_HEAD(&p->requests); + if (p->priority != I915_PRIORITY_NORMAL) + kfree(p); } - if (submit) { +done: + engine->execlist_first = rb; + if (submit) port_assign(port, last); - engine->execlist_first = rb; - } spin_unlock_irq(&engine->timeline->lock); if (submit) @@ -610,28 +628,66 @@ static void intel_lrc_irq_handler(unsigned long data) intel_uncore_forcewake_put(dev_priv, engine->fw_domains); } -static bool insert_request(struct i915_priotree *pt, struct rb_root *root) +static bool +insert_request(struct intel_engine_cs *engine, + struct i915_priotree *pt, + int prio) { - struct rb_node **p, *rb; + struct i915_priolist *p; + struct rb_node **parent, *rb; bool first = true; + if (unlikely(engine->no_priolist)) + prio = I915_PRIORITY_NORMAL; + +find_priolist: /* most positive priority is scheduled first, equal priorities fifo */ rb = NULL; - p = &root->rb_node; - while (*p) { - struct i915_priotree *pos; - - rb = *p; - pos = rb_entry(rb, typeof(*pos), node); - if (pt->priority > pos->priority) { - p = &rb->rb_left; - } else { - p = &rb->rb_right; + parent = &engine->execlist_queue.rb_node; + while (*parent) { + rb = *parent; + p = rb_entry(rb, typeof(*p), node); + if (prio > p->priority) { + parent = &rb->rb_left; + } else if (prio < p->priority) { + parent = &rb->rb_right; first = false; + } else { + list_add_tail(&pt->link, &p->requests); + return false; + } + } + + if (prio == I915_PRIORITY_NORMAL) { + p = &engine->default_priolist; + } else { + p = kmalloc(sizeof(*p), GFP_ATOMIC); + /* Convert an allocation failure to a priority bump */ + if (unlikely(!p)) { + prio = I915_PRIORITY_NORMAL; /* recurses just once */ + + /* To maintain ordering with all rendering, after an + * allocation failure we have to disable all scheduling. + * Requests will then be executed in fifo, and schedule + * will ensure that dependencies are emitted in fifo. + * There will be still some reordering with existing + * requests, so if userspace lied about their + * dependencies that reordering may be visible. + */ + engine->no_priolist = true; + goto find_priolist; } } - rb_link_node(&pt->node, rb, p); - rb_insert_color(&pt->node, root); + + p->priority = prio; + rb_link_node(&p->node, rb, parent); + rb_insert_color(&p->node, &engine->execlist_queue); + + INIT_LIST_HEAD(&p->requests); + list_add_tail(&pt->link, &p->requests); + + if (first) + engine->execlist_first = &p->node; return first; } @@ -644,12 +700,16 @@ static void execlists_submit_request(struct drm_i915_gem_request *request) /* Will be called from irq-context when using foreign fences. */ spin_lock_irqsave(&engine->timeline->lock, flags); - if (insert_request(&request->priotree, &engine->execlist_queue)) { - engine->execlist_first = &request->priotree.node; + if (insert_request(engine, + &request->priotree, + request->priotree.priority)) { if (execlists_elsp_ready(engine)) tasklet_hi_schedule(&engine->irq_tasklet); } + GEM_BUG_ON(!engine->execlist_first); + GEM_BUG_ON(list_empty(&request->priotree.link)); + spin_unlock_irqrestore(&engine->timeline->lock, flags); } @@ -734,10 +794,9 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio) continue; pt->priority = prio; - if (!RB_EMPTY_NODE(&pt->node)) { - rb_erase(&pt->node, &engine->execlist_queue); - if (insert_request(pt, &engine->execlist_queue)) - engine->execlist_first = &pt->node; + if (!list_empty(&pt->link)) { + __list_del_entry(&pt->link); + insert_request(engine, pt, prio); } } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 162f0a9c6abe..6aa20ac8cde3 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -177,6 +177,12 @@ enum intel_engine_id { VECS }; +struct i915_priolist { + struct rb_node node; + struct list_head requests; + int priority; +}; + #define INTEL_ENGINE_CS_MAX_NAME 8 struct intel_engine_cs { @@ -367,6 +373,8 @@ struct intel_engine_cs { /* Execlists */ struct tasklet_struct irq_tasklet; + struct i915_priolist default_priolist; + bool no_priolist; struct execlist_port { struct drm_i915_gem_request *request_count; #define EXECLIST_COUNT_BITS 2 @@ -723,6 +731,7 @@ static inline u32 *gen8_emit_pipe_control(u32 *batch, u32 flags, u32 offset) bool intel_engine_is_idle(struct intel_engine_cs *engine); bool intel_engines_are_idle(struct drm_i915_private *dev_priv); +void intel_engines_mark_idle(struct drm_i915_private *i915); void intel_engines_reset_default_submission(struct drm_i915_private *i915); #endif /* _INTEL_RINGBUFFER_H_ */ -- cgit v1.2.3 From a091d4ee931b16ce4fef945d39a20b851a7e17b7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 30 May 2017 13:13:33 +0100 Subject: drm/i915: Hold a wakeref for probing the ring registers Allow intel_engine_is_idle() to be called outside of the GT wakeref by acquiring the device runtime pm for ourselves. This allows the function to act as check after we assume the engine is idle and we release the GT wakeref held whilst we have requests. At the moment, we do not call it outside of an awake context but taking the wakeref as required makes it more convenient to use for quick debugging in future. [ 2613.401647] RPM wakelock ref not held during HW access [ 2613.401684] ------------[ cut here ]------------ [ 2613.401720] WARNING: CPU: 5 PID: 7739 at drivers/gpu/drm/i915/intel_drv.h:1787 gen6_read32+0x21f/0x2b0 [i915] [ 2613.401731] Modules linked in: snd_hda_intel i915 vgem snd_hda_codec_hdmi x86_pkg_temp_thermal intel_powerclamp snd_hda_codec_realtek coretemp snd_hda_codec_generic crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm r8169 mii mei_me lpc_ich mei prime_numbers [last unloaded: i915] [ 2613.401823] CPU: 5 PID: 7739 Comm: drv_missed_irq Tainted: G U 4.12.0-rc2-CI-CI_DRM_421+ #1 [ 2613.401825] Hardware name: MSI MS-7924/Z97M-G43(MS-7924), BIOS V1.12 02/15/2016 [ 2613.401840] task: ffff880409e3a740 task.stack: ffffc900084dc000 [ 2613.401861] RIP: 0010:gen6_read32+0x21f/0x2b0 [i915] [ 2613.401863] RSP: 0018:ffffc900084dfce8 EFLAGS: 00010292 [ 2613.401869] RAX: 000000000000002a RBX: ffff8804016a8000 RCX: 0000000000000006 [ 2613.401871] RDX: 0000000000000006 RSI: ffffffff81cbf2d9 RDI: ffffffff81c9e3a7 [ 2613.401874] RBP: ffffc900084dfd18 R08: ffff880409e3afc8 R09: 0000000000000000 [ 2613.401877] R10: 000000008a1c483f R11: 0000000000000000 R12: 000000000000209c [ 2613.401879] R13: 0000000000000001 R14: ffff8804016a8000 R15: ffff8804016ac150 [ 2613.401882] FS: 00007f39ef3dd8c0(0000) GS:ffff88041fb40000(0000) knlGS:0000000000000000 [ 2613.401885] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 2613.401887] CR2: 00000000023717c8 CR3: 00000002e7b34000 CR4: 00000000001406e0 [ 2613.401889] Call Trace: [ 2613.401912] intel_engine_is_idle+0x76/0x90 [i915] [ 2613.401931] i915_gem_wait_for_idle+0xe6/0x1e0 [i915] [ 2613.401951] fault_irq_set+0x40/0x90 [i915] [ 2613.401970] i915_ring_test_irq_set+0x42/0x50 [i915] [ 2613.401976] simple_attr_write+0xc7/0xe0 [ 2613.401981] full_proxy_write+0x4f/0x70 [ 2613.401987] __vfs_write+0x23/0x120 [ 2613.401992] ? rcu_read_lock_sched_held+0x75/0x80 [ 2613.401996] ? rcu_sync_lockdep_assert+0x2a/0x50 [ 2613.401999] ? __sb_start_write+0xfa/0x1f0 [ 2613.402004] vfs_write+0xc5/0x1d0 [ 2613.402008] ? trace_hardirqs_on_caller+0xe7/0x1c0 [ 2613.402013] SyS_write+0x44/0xb0 [ 2613.402020] entry_SYSCALL_64_fastpath+0x1c/0xb1 [ 2613.402022] RIP: 0033:0x7f39eded6670 [ 2613.402025] RSP: 002b:00007fffdcdcb1a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 [ 2613.402030] RAX: ffffffffffffffda RBX: ffffffff81470203 RCX: 00007f39eded6670 [ 2613.402033] RDX: 0000000000000001 RSI: 000000000041bc33 RDI: 0000000000000006 [ 2613.402036] RBP: ffffc900084dff88 R08: 00007f39ef3dd8c0 R09: 0000000000000001 [ 2613.402038] R10: 0000000000000000 R11: 0000000000000246 R12: 000000000041bc33 [ 2613.402041] R13: 0000000000000006 R14: 0000000000000000 R15: 0000000000000000 [ 2613.402046] ? __this_cpu_preempt_check+0x13/0x20 [ 2613.402052] Code: 01 9b fa e0 0f ff e9 28 fe ff ff 80 3d 6a dd 0e 00 00 0f 85 29 fe ff ff 48 c7 c7 48 19 29 a0 c6 05 56 dd 0e 00 01 e8 da 9a fa e0 <0f> ff e9 0f fe ff ff b9 01 00 00 00 ba 01 00 00 00 44 89 e6 48 [ 2613.402199] ---[ end trace 31f0cfa93ab632bf ]--- Fixes: 5400367a864d ("drm/i915: Ensure the engine is idle before manually changing HWS") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170530121334.17364-2-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_engine_cs.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 413bfd8d4bf4..699f2d3861c7 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1205,6 +1205,22 @@ int intel_ring_workarounds_emit(struct drm_i915_gem_request *req) return 0; } +static bool ring_is_idle(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + bool idle = true; + + intel_runtime_pm_get(dev_priv); + + /* No bit for gen2, so assume the CS parser is idle */ + if (INTEL_GEN(dev_priv) > 2 && !(I915_READ_MODE(engine) & MODE_IDLE)) + idle = false; + + intel_runtime_pm_put(dev_priv); + + return idle; +} + /** * intel_engine_is_idle() - Report if the engine has finished process all work * @engine: the intel_engine_cs @@ -1237,7 +1253,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) return false; /* Ring stopped? */ - if (INTEL_GEN(dev_priv) > 2 && !(I915_READ_MODE(engine) & MODE_IDLE)) + if (!ring_is_idle(engine)) return false; return true; -- cgit v1.2.3 From aed2fc102ffaa9bd41e4cff140d3ec8482d13ff5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 30 May 2017 13:13:34 +0100 Subject: drm/i915: Check the ring is empty when declaring the engines are idle As another precaution when testing whether the CS engine is actually idle, also inspect the ring's HEAD/TAIL registers, which should be equal when there are no commands left to execute by the GPU. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/20170530121334.17364-3-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 699f2d3861c7..bc38bd128b76 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1212,6 +1212,11 @@ static bool ring_is_idle(struct intel_engine_cs *engine) intel_runtime_pm_get(dev_priv); + /* First check that no commands are left in the ring */ + if ((I915_READ_HEAD(engine) & HEAD_ADDR) != + (I915_READ_TAIL(engine) & TAIL_ADDR)) + idle = false; + /* No bit for gen2, so assume the CS parser is idle */ if (INTEL_GEN(dev_priv) > 2 && !(I915_READ_MODE(engine) & MODE_IDLE)) idle = false; -- cgit v1.2.3 From 46c26662d2fb8377f5d387cf2e5ee3246af780b7 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Fri, 16 Jun 2017 15:49:58 -0700 Subject: drm/i915/cfl: Introduce Coffee Lake workarounds. Coffee Lake inherit most of Kabylake production workarounds. v2: Fix typo on commit message and remove WaDisableKillLogic and GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC, since as Mika pointed out they shouldn't be here for cfl according to BSpec. Cc: Dhinakaran Pandiyan Signed-off-by: Rodrigo Vivi Reviewed-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1497653398-15722-1-git-send-email-rodrigo.vivi@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 81 +++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 23 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_engine_cs.c') diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 205dd91d3601..61fc7e90a7da 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1884,7 +1884,7 @@ static void gtt_write_workarounds(struct drm_i915_private *dev_priv) * called on driver load and after a GPU reset, so you can place * workarounds here even if they get overwritten by GPU reset. */ - /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,kbl,glk */ + /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,kbl,glk,cfl */ if (IS_BROADWELL(dev_priv)) I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW); else if (IS_CHERRYVIEW(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index bc38bd128b76..a4487c5b7e37 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -814,26 +814,27 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) struct drm_i915_private *dev_priv = engine->i915; int ret; - /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */ + /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */ I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); - /* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk */ + /* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */ I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); - /* WaDisableKillLogic:bxt,skl,kbl */ + /* WaDisableKillLogic:bxt,skl,kbl,cfl */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | ECOCHK_DIS_TLB); - /* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk */ - /* WaDisablePartialInstShootdown:skl,bxt,kbl,glk */ + /* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk,cfl */ + /* WaDisablePartialInstShootdown:skl,bxt,kbl,glk,cfl */ WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, FLOW_CONTROL_ENABLE | PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); /* Syncing dependencies between camera and graphics:skl,bxt,kbl */ - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, - GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); + if (!IS_COFFEELAKE(dev_priv)) + WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, + GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); /* WaDisableDgMirrorFixInHalfSliceChicken5:bxt */ if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) @@ -851,18 +852,18 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) */ } - /* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt,kbl,glk */ - /* WaEnableSamplerGPGPUPreemptionSupport:skl,bxt,kbl */ + /* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt,kbl,glk,cfl */ + /* WaEnableSamplerGPGPUPreemptionSupport:skl,bxt,kbl,cfl */ WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7, GEN9_ENABLE_YV12_BUGFIX | GEN9_ENABLE_GPGPU_PREEMPTION); - /* Wa4x4STCOptimizationDisable:skl,bxt,kbl,glk */ - /* WaDisablePartialResolveInVc:skl,bxt,kbl */ + /* Wa4x4STCOptimizationDisable:skl,bxt,kbl,glk,cfl */ + /* WaDisablePartialResolveInVc:skl,bxt,kbl,cfl */ WA_SET_BIT_MASKED(CACHE_MODE_1, (GEN8_4x4_STC_OPTIMIZATION_DISABLE | GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE)); - /* WaCcsTlbPrefetchDisable:skl,bxt,kbl,glk */ + /* WaCcsTlbPrefetchDisable:skl,bxt,kbl,glk,cfl */ WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5, GEN9_CCS_TLB_PREFETCH_ENABLE); @@ -871,7 +872,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) WA_SET_BIT_MASKED(SLICE_ECO_CHICKEN0, PIXEL_MASK_CAMMING_DISABLE); - /* WaForceContextSaveRestoreNonCoherent:skl,bxt,kbl */ + /* WaForceContextSaveRestoreNonCoherent:skl,bxt,kbl,cfl */ WA_SET_BIT_MASKED(HDC_CHICKEN0, HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT | HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE); @@ -889,39 +890,41 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) * a TLB invalidation occurs during a PSD flush. */ - /* WaForceEnableNonCoherent:skl,bxt,kbl */ + /* WaForceEnableNonCoherent:skl,bxt,kbl,cfl */ WA_SET_BIT_MASKED(HDC_CHICKEN0, HDC_FORCE_NON_COHERENT); /* WaDisableHDCInvalidation:skl,bxt,kbl */ - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | - BDW_DISABLE_HDC_INVALIDATION); + if (!IS_COFFEELAKE(dev_priv)) + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + BDW_DISABLE_HDC_INVALIDATION); - /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt,kbl */ + /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt,kbl,cfl */ if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) || + IS_COFFEELAKE(dev_priv) || IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0)) WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, GEN8_SAMPLER_POWER_BYPASS_DIS); - /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk */ + /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk,cfl */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); - /* WaOCLCoherentLineFlush:skl,bxt,kbl */ + /* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */ I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | GEN8_LQSC_FLUSH_COHERENT_LINES)); - /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk */ + /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */ ret = wa_ring_whitelist_reg(engine, GEN9_CTX_PREEMPT_REG); if (ret) return ret; - /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl */ + /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl */ ret= wa_ring_whitelist_reg(engine, GEN8_CS_CHICKEN1); if (ret) return ret; - /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk */ + /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */ ret = wa_ring_whitelist_reg(engine, GEN8_HDC_CHICKEN1); if (ret) return ret; @@ -1140,6 +1143,38 @@ static int glk_init_workarounds(struct intel_engine_cs *engine) return 0; } +static int cfl_init_workarounds(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + int ret; + + ret = gen9_init_workarounds(engine); + if (ret) + return ret; + + /* WaEnableGapsTsvCreditFix:cfl */ + I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) | + GEN9_GAPS_TSV_CREDIT_DISABLE)); + + /* WaToEnableHwFixForPushConstHWBug:cfl */ + WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, + GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION); + + /* WaDisableGafsUnitClkGating:cfl */ + WA_SET_BIT(GEN7_UCGCTL4, GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE); + + /* WaDisableSbeCacheDispatchPortSharing:cfl */ + WA_SET_BIT_MASKED( + GEN7_HALF_SLICE_CHICKEN1, + GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); + + /* WaInPlaceDecompressionHang:cfl */ + WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA, + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS); + + return 0; +} + int init_workarounds_ring(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; @@ -1162,6 +1197,8 @@ int init_workarounds_ring(struct intel_engine_cs *engine) err = kbl_init_workarounds(engine); else if (IS_GEMINILAKE(dev_priv)) err = glk_init_workarounds(engine); + else if (IS_COFFEELAKE(dev_priv)) + err = cfl_init_workarounds(engine); else err = 0; if (err) -- cgit v1.2.3