From 97ea656521c8e94c202d24f9d953cb65297f9aec Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 29 Jan 2021 00:49:32 +0000 Subject: drm/i915/gvt: Parse default state to update reg whitelist Rather than break existing context objects by incorrectly forcing them to rogue cache coherency and trying to assert a new mapping, read the reg whitelist from the default context image. And use gvt->gt, never &dev_priv->gt. Fixes: 493f30cd086e ("drm/i915/gvt: parse init context to update cmd accessible reg whitelist") Acked-by: Zhenyu Wang Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Kevin Tian Cc: Wang Zhi Cc: Yan Zhao Cc: Zhenyu Wang Cc: Zhi Wang Signed-off-by: Zhenyu Wang Link: http://patchwork.freedesktop.org/patch/msgid/20210129004933.29755-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 93 ++++++++--------------------------- 1 file changed, 20 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index d54ea0e4681d..fef1e857cefc 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -41,6 +41,7 @@ #include "gt/intel_lrc.h" #include "gt/intel_ring.h" #include "gt/intel_gt_requests.h" +#include "gt/shmem_utils.h" #include "gvt.h" #include "i915_pvinfo.h" #include "trace.h" @@ -3094,71 +3095,28 @@ int intel_gvt_scan_and_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) */ void intel_gvt_update_reg_whitelist(struct intel_vgpu *vgpu) { + const unsigned long start = LRC_STATE_PN * PAGE_SIZE; struct intel_gvt *gvt = vgpu->gvt; - struct drm_i915_private *dev_priv = gvt->gt->i915; struct intel_engine_cs *engine; enum intel_engine_id id; - const unsigned long start = LRC_STATE_PN * PAGE_SIZE; - struct i915_request *rq; - struct intel_vgpu_submission *s = &vgpu->submission; - struct i915_request *requests[I915_NUM_ENGINES] = {}; - bool is_ctx_pinned[I915_NUM_ENGINES] = {}; - int ret = 0; if (gvt->is_reg_whitelist_updated) return; - for_each_engine(engine, &dev_priv->gt, id) { - ret = intel_context_pin(s->shadow[id]); - if (ret) { - gvt_vgpu_err("fail to pin shadow ctx\n"); - goto out; - } - is_ctx_pinned[id] = true; - - rq = i915_request_create(s->shadow[id]); - if (IS_ERR(rq)) { - gvt_vgpu_err("fail to alloc default request\n"); - ret = -EIO; - goto out; - } - requests[id] = i915_request_get(rq); - i915_request_add(rq); - } - - if (intel_gt_wait_for_idle(&dev_priv->gt, - I915_GEM_IDLE_TIMEOUT) == -ETIME) { - ret = -EIO; - goto out; - } - /* scan init ctx to update cmd accessible list */ - for_each_engine(engine, &dev_priv->gt, id) { - int size = engine->context_size - PAGE_SIZE; - void *vaddr; + for_each_engine(engine, gvt->gt, id) { struct parser_exec_state s; - struct drm_i915_gem_object *obj; - struct i915_request *rq; - - rq = requests[id]; - GEM_BUG_ON(!i915_request_completed(rq)); - GEM_BUG_ON(!intel_context_is_pinned(rq->context)); - obj = rq->context->state->obj; - - if (!obj) { - ret = -EIO; - goto out; - } + void *vaddr; + int ret; - i915_gem_object_set_cache_coherency(obj, - I915_CACHE_LLC); + if (!engine->default_state) + continue; - vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); + vaddr = shmem_pin_map(engine->default_state); if (IS_ERR(vaddr)) { - gvt_err("failed to pin init ctx obj, ring=%d, err=%lx\n", - id, PTR_ERR(vaddr)); - ret = PTR_ERR(vaddr); - goto out; + gvt_err("failed to map %s->default state, err:%zd\n", + engine->name, PTR_ERR(vaddr)); + return; } s.buf_type = RING_BUFFER_CTX; @@ -3166,9 +3124,9 @@ void intel_gvt_update_reg_whitelist(struct intel_vgpu *vgpu) s.vgpu = vgpu; s.engine = engine; s.ring_start = 0; - s.ring_size = size; + s.ring_size = engine->context_size - start; s.ring_head = 0; - s.ring_tail = size; + s.ring_tail = s.ring_size; s.rb_va = vaddr + start; s.workload = NULL; s.is_ctx_wa = false; @@ -3176,29 +3134,18 @@ void intel_gvt_update_reg_whitelist(struct intel_vgpu *vgpu) /* skipping the first RING_CTX_SIZE(0x50) dwords */ ret = ip_gma_set(&s, RING_CTX_SIZE); - if (ret) { - i915_gem_object_unpin_map(obj); - goto out; + if (ret == 0) { + ret = command_scan(&s, 0, s.ring_size, 0, s.ring_size); + if (ret) + gvt_err("Scan init ctx error\n"); } - ret = command_scan(&s, 0, size, 0, size); + shmem_unpin_map(engine->default_state, vaddr); if (ret) - gvt_err("Scan init ctx error\n"); - - i915_gem_object_unpin_map(obj); + return; } -out: - if (!ret) - gvt->is_reg_whitelist_updated = true; - - for (id = 0; id < I915_NUM_ENGINES ; id++) { - if (requests[id]) - i915_request_put(requests[id]); - - if (is_ctx_pinned[id]) - intel_context_unpin(s->shadow[id]); - } + gvt->is_reg_whitelist_updated = true; } int intel_gvt_scan_engine_context(struct intel_vgpu_workload *workload) -- cgit v1.2.3 From d18ac1a7eef9ec266142b637253353a8d9f95cc1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 29 Jan 2021 00:49:33 +0000 Subject: drm/i915/gvt: Purge dev_priv->gt Use the right intel_gt stored as a backpointer in intel_vgpu. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang Link: http://patchwork.freedesktop.org/patch/msgid/20210129004933.29755-2-chris@chris-wilson.co.uk Reviewed-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 8 +++----- drivers/gpu/drm/i915/gvt/scheduler.c | 3 +-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index 158873f269b1..c8dcda6d4f0d 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -522,12 +522,11 @@ static void init_vgpu_execlist(struct intel_vgpu *vgpu, static void clean_execlist(struct intel_vgpu *vgpu, intel_engine_mask_t engine_mask) { - struct drm_i915_private *dev_priv = vgpu->gvt->gt->i915; - struct intel_engine_cs *engine; struct intel_vgpu_submission *s = &vgpu->submission; + struct intel_engine_cs *engine; intel_engine_mask_t tmp; - for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) { + for_each_engine_masked(engine, vgpu->gvt->gt, engine_mask, tmp) { kfree(s->ring_scan_buffer[engine->id]); s->ring_scan_buffer[engine->id] = NULL; s->ring_scan_buffer_size[engine->id] = 0; @@ -537,11 +536,10 @@ static void clean_execlist(struct intel_vgpu *vgpu, static void reset_execlist(struct intel_vgpu *vgpu, intel_engine_mask_t engine_mask) { - struct drm_i915_private *dev_priv = vgpu->gvt->gt->i915; struct intel_engine_cs *engine; intel_engine_mask_t tmp; - for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) + for_each_engine_masked(engine, vgpu->gvt->gt, engine_mask, tmp) init_vgpu_execlist(vgpu, engine); } diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 43f31c2eab14..a55ae50dbbe1 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -1015,13 +1015,12 @@ void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu, intel_engine_mask_t engine_mask) { struct intel_vgpu_submission *s = &vgpu->submission; - struct drm_i915_private *dev_priv = vgpu->gvt->gt->i915; struct intel_engine_cs *engine; struct intel_vgpu_workload *pos, *n; intel_engine_mask_t tmp; /* free the unsubmited workloads in the queues. */ - for_each_engine_masked(engine, &dev_priv->gt, engine_mask, tmp) { + for_each_engine_masked(engine, vgpu->gvt->gt, engine_mask, tmp) { list_for_each_entry_safe(pos, n, &s->workload_q_head[engine->id], list) { list_del_init(&pos->list); -- cgit v1.2.3 From 67f1120381df022a7016f4acc8d4880da9a66c03 Mon Sep 17 00:00:00 2001 From: Zhi Wang Date: Sun, 10 Jan 2021 23:43:05 +0200 Subject: drm/i915/gvt: Introduce per object locking in GVT scheduler. To support ww locking and per-object implemented in i915, GVT scheduler needs to be refined. Most of the changes are located in shadow batch buffer, shadow wa context in GVT-g, where use quite a lot of i915 gem object APIs. v2: - Adjust the usage of ww lock on context pin/unpin. (maarten) - Rebase the patch on the newest staging branch. Fixes: 6b05030496f7 ("drm/i915: Convert i915_gem_object/client_blt.c to use ww locking as well, v2.") Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Zhenyu Wang Signed-off-by: Zhi Wang Signed-off-by: Zhenyu Wang Link: http://patchwork.freedesktop.org/patch/msgid/1610314985-26065-1-git-send-email-zhi.wang.linux@gmail.com Reviewed-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 49 +++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index a55ae50dbbe1..fc735692f21f 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -412,7 +412,9 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (!wa_ctx->indirect_ctx.obj) return; + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL); i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); i915_gem_object_put(wa_ctx->indirect_ctx.obj); wa_ctx->indirect_ctx.obj = NULL; @@ -520,6 +522,7 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) struct intel_gvt *gvt = workload->vgpu->gvt; const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd; struct intel_vgpu_shadow_bb *bb; + struct i915_gem_ww_ctx ww; int ret; list_for_each_entry(bb, &workload->shadow_bb, list) { @@ -544,10 +547,19 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) * directly */ if (!bb->ppgtt) { - bb->vma = i915_gem_object_ggtt_pin(bb->obj, - NULL, 0, 0, 0); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(bb->obj, &ww); + + bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww, + NULL, 0, 0, 0); if (IS_ERR(bb->vma)) { ret = PTR_ERR(bb->vma); + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } goto err; } @@ -561,13 +573,15 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) 0); if (ret) goto err; - } - /* No one is going to touch shadow bb from now on. */ - i915_gem_object_flush_map(bb->obj); + /* No one is going to touch shadow bb from now on. */ + i915_gem_object_flush_map(bb->obj); + i915_gem_object_unlock(bb->obj); + } } return 0; err: + i915_gem_ww_ctx_fini(&ww); release_shadow_batch_buffer(workload); return ret; } @@ -594,14 +608,29 @@ static int prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) unsigned char *per_ctx_va = (unsigned char *)wa_ctx->indirect_ctx.shadow_va + wa_ctx->indirect_ctx.size; + struct i915_gem_ww_ctx ww; + int ret; if (wa_ctx->indirect_ctx.size == 0) return 0; - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, - 0, CACHELINE_BYTES, 0); - if (IS_ERR(vma)) - return PTR_ERR(vma); + i915_gem_ww_ctx_init(&ww, false); +retry: + i915_gem_object_lock(wa_ctx->indirect_ctx.obj, &ww); + + vma = i915_gem_object_ggtt_pin_ww(wa_ctx->indirect_ctx.obj, &ww, NULL, + 0, CACHELINE_BYTES, 0); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + if (ret == -EDEADLK) { + ret = i915_gem_ww_ctx_backoff(&ww); + if (!ret) + goto retry; + } + return ret; + } + + i915_gem_object_unlock(wa_ctx->indirect_ctx.obj); /* FIXME: we are not tracking our pinned VMA leaving it * up to the core to fix up the stray pin_count upon @@ -635,12 +664,14 @@ static void release_shadow_batch_buffer(struct intel_vgpu_workload *workload) list_for_each_entry_safe(bb, pos, &workload->shadow_bb, list) { if (bb->obj) { + i915_gem_object_lock(bb->obj, NULL); if (bb->va && !IS_ERR(bb->va)) i915_gem_object_unpin_map(bb->obj); if (bb->vma && !IS_ERR(bb->vma)) i915_vma_unpin(bb->vma); + i915_gem_object_unlock(bb->obj); i915_gem_object_put(bb->obj); } list_del(&bb->list); -- cgit v1.2.3 From 34b07d47dd003168556a1774558240fefbb9b461 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Tue, 16 Feb 2021 14:29:54 -0700 Subject: drm/i915: Enable -Wuninitialized -Wunintialized was disabled in commit c5627461490e ("drm/i915: Disable -Wuninitialized") because there were two warnings that were false positives. The first was due to DECLARE_WAIT_QUEUE_HEAD_ONSTACK, which was fixed in LLVM 9.0.0. The second was in busywait_stop, which was fixed in LLVM 10.0.0 (issue 415). The kernel's minimum version for LLVM is 10.0.1 so this warning can be safely enabled, where it has already caught a couple bugs. Link: https://github.com/ClangBuiltLinux/linux/issues/220 Link: https://github.com/ClangBuiltLinux/linux/issues/415 Link: https://github.com/ClangBuiltLinux/linux/issues/499 Link: https://github.com/llvm/llvm-project/commit/2e040398f8d691cc378c1abb098824ff49f3f28f Link: https://github.com/llvm/llvm-project/commit/c667cdc850c2aa821ffeedbc08c24bc985c59edd Fixes: c5627461490e ("drm/i915: Disable -Wuninitialized") References: 2ea4a7ba9bf6 ("drm/i915/gt: Avoid uninitialized use of rpcurupei in frequency_show") References: 2034c2129bc4 ("drm/i915/display: Ensure that ret is always initialized in icl_combo_phy_verify_state") Reported-by: Arnd Bergmann Signed-off-by: Nathan Chancellor Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20210216212953.24458-1-nathan@kernel.org (cherry picked from commit b2423184ac3352a52fc7562fa0e7d23435fe67b9) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 32bd1fdffffe..dae6e93fc1e5 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -21,7 +21,6 @@ subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) subdir-ccflags-y += $(call cc-disable-warning, sign-compare) subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized) subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides) -subdir-ccflags-y += $(call cc-disable-warning, uninitialized) subdir-ccflags-y += $(call cc-disable-warning, frame-address) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror -- cgit v1.2.3 From ed428ffc28521b9f1b6a71584d1875318a122859 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 5 Feb 2021 22:23:22 +0200 Subject: drm/i915: Nuke INTEL_OUTPUT_FORMAT_INVALID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We tend to use output_format!=RGB as a shorthand for YCbCr, but this fails if we have a disabled crtc where output_format==INVALID. We're now getting some fail from intel_color_check() when we have: hw.enable==false hw.ctm!=NULL output_format==INVALID Let's avoid that by throwing INTEL_OUTPUT_FORMAT_INVALID to the dumpster, and thus everything defaults to RGB when the crtc is disabled. This does beg the deeper question of how much of the state should we in fact be validating when hw/uapi.enable==false. And should we even be doing the uapi->hw copy when uapi.enable==false? So far I've not been able to come up with satisfactory answers for myself, so I'm putting it off for the moment. Cc: Lee Shawn C Fixes: 0aa5c3835c8a ("drm/i915: support two CSC module on gen11 and later") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2964 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20210205202322.27608-1-ville.syrjala@linux.intel.com Reviewed-by: José Roberto de Souza (cherry picked from commit 7e07c68f06a248441b485249de4c4115cba262cc) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/display/intel_crtc.c | 1 - drivers/gpu/drm/i915/display/intel_display.c | 3 +-- drivers/gpu/drm/i915/display/intel_display_types.h | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 57b0a3ebe908..8e77ca7ddf11 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -109,7 +109,6 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state, crtc_state->cpu_transcoder = INVALID_TRANSCODER; crtc_state->master_transcoder = INVALID_TRANSCODER; crtc_state->hsw_workaround_pipe = INVALID_PIPE; - crtc_state->output_format = INTEL_OUTPUT_FORMAT_INVALID; crtc_state->scaler_state.scaler_id = -1; crtc_state->mst_master_transcoder = INVALID_TRANSCODER; } diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 7ea1e5b487b6..8d7aaa68c6f6 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -10211,7 +10211,6 @@ static void snprintf_output_types(char *buf, size_t len, } static const char * const output_format_str[] = { - [INTEL_OUTPUT_FORMAT_INVALID] = "Invalid", [INTEL_OUTPUT_FORMAT_RGB] = "RGB", [INTEL_OUTPUT_FORMAT_YCBCR420] = "YCBCR4:2:0", [INTEL_OUTPUT_FORMAT_YCBCR444] = "YCBCR4:4:4", @@ -10220,7 +10219,7 @@ static const char * const output_format_str[] = { static const char *output_formats(enum intel_output_format format) { if (format >= ARRAY_SIZE(output_format_str)) - format = INTEL_OUTPUT_FORMAT_INVALID; + return "invalid"; return output_format_str[format]; } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 39397748b4b0..184ecbbcec99 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -830,7 +830,6 @@ struct intel_crtc_wm_state { }; enum intel_output_format { - INTEL_OUTPUT_FORMAT_INVALID, INTEL_OUTPUT_FORMAT_RGB, INTEL_OUTPUT_FORMAT_YCBCR420, INTEL_OUTPUT_FORMAT_YCBCR444, -- cgit v1.2.3