From aefff4915632185e286c8c94f6f5ccb939fc51d7 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 15 May 2018 09:54:33 +0100 Subject: drm/xen-front: fix spelling mistake: "conector" -> "connector" Trivial fix to spelling mistake in DRM_INFO message. Signed-off-by: Colin Ian King Reviewed-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko Link: https://patchwork.freedesktop.org/patch/msgid/20180515085433.8245-1-colin.king@canonical.com --- drivers/gpu/drm/xen/xen_drm_front.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index 3345ac71b391..93bf9f092086 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -623,7 +623,7 @@ static int displback_initwait(struct xen_drm_front_info *front_info) if (ret < 0) return ret; - DRM_INFO("Have %d conector(s)\n", cfg->num_connectors); + DRM_INFO("Have %d connector(s)\n", cfg->num_connectors); /* Create event channels for all connectors and publish */ ret = xen_drm_front_evtchnl_create_all(front_info); if (ret < 0) -- cgit v1.2.3-58-ga151 From 584a0146ec4989f30d0aef46ce1ea6f6ba22a690 Mon Sep 17 00:00:00 2001 From: Philippe Cornu Date: Mon, 9 Apr 2018 17:24:27 +0200 Subject: drm: clarify adjusted_mode documentation for bridges This patch clarifies the adjusted_mode documentation for bridges. Signed-off-by: Philippe Cornu Signed-off-by: Laurent Pinchart Reviewed-by: Archit Taneja Link: https://patchwork.freedesktop.org/patch/msgid/20180409152427.12449-1-philippe.cornu@st.com --- include/drm/drm_bridge.h | 16 ++++++++++++++++ include/drm/drm_crtc.h | 11 +++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 3270fec46979..9917651a7fdd 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -178,6 +178,22 @@ struct drm_bridge_funcs { * then this would be &drm_encoder_helper_funcs.mode_set. The display * pipe (i.e. clocks and timing signals) is off when this function is * called. + * + * The adjusted_mode parameter is the mode output by the CRTC for the + * first bridge in the chain. It can be different from the mode + * parameter that contains the desired mode for the connector at the end + * of the bridges chain, for instance when the first bridge in the chain + * performs scaling. The adjusted mode is mostly useful for the first + * bridge in the chain and is likely irrelevant for the other bridges. + * + * For atomic drivers the adjusted_mode is the mode stored in + * &drm_crtc_state.adjusted_mode. + * + * NOTE: + * + * If a need arises to store and access modes adjusted for other + * locations than the connection between the CRTC and the first bridge, + * the DRM framework will have to be extended with DRM bridge states. */ void (*mode_set)(struct drm_bridge *bridge, struct drm_display_mode *mode, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index a2d81d2907a9..5cf7adeae6a5 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -134,10 +134,13 @@ struct drm_crtc_state { * * Internal display timings which can be used by the driver to handle * differences between the mode requested by userspace in @mode and what - * is actually programmed into the hardware. It is purely driver - * implementation defined what exactly this adjusted mode means. Usually - * it is used to store the hardware display timings used between the - * CRTC and encoder blocks. + * is actually programmed into the hardware. + * + * For drivers using drm_bridge, this stores hardware display timings + * used between the CRTC and the first bridge. For other drivers, the + * meaning of the adjusted_mode field is purely driver implementation + * defined information, and will usually be used to store the hardware + * display timings used between the CRTC and encoder blocks. */ struct drm_display_mode adjusted_mode; -- cgit v1.2.3-58-ga151 From 1c6ccad8a4b110bac3f9ff6db954ab5a74c2e861 Mon Sep 17 00:00:00 2001 From: Tina Zhang Date: Mon, 14 May 2018 13:59:18 +0800 Subject: drm/i915/gvt: Deliver guest cursor hotspot info Guest OS driver uses PV info registers to deliver cursor hotspot info to host. This patch is used to get cursor hotspot info from virtual registers and deliver it to host userspace. v4->v5: - remove CI warning. v3->v4: - return UINT_MAX when x_hot/y_hot is invalid. (Zhenyu) - correct version. v2->v3: - add validate_hotspot(). (Zhenyu) v1->v2: - name as cursor_x_hot/cursor_y_hot. (Zhenyu) - use i915_reg_t definition instead of magic numbers. (Zhenyu) Signed-off-by: Tina Zhang Cc: Zhenyu Wang Cc: Zhi Wang Cc: Gerd Hoffmann Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/dmabuf.c | 22 ++++++++++++++++------ drivers/gpu/drm/i915/gvt/fb_decoder.c | 3 +++ drivers/gpu/drm/i915/gvt/handlers.c | 4 ++-- drivers/gpu/drm/i915/gvt/vgpu.c | 3 +++ drivers/gpu/drm/i915/i915_pvinfo.h | 5 ++++- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c index 6f4f8e941fc2..d2eb2f7754b9 100644 --- a/drivers/gpu/drm/i915/gvt/dmabuf.c +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c @@ -192,6 +192,14 @@ static struct drm_i915_gem_object *vgpu_create_gem(struct drm_device *dev, return obj; } +static bool validate_hotspot(struct intel_vgpu_cursor_plane_format *c) +{ + if (c && c->x_hot <= c->width && c->y_hot <= c->height) + return true; + else + return false; +} + static int vgpu_get_plane_info(struct drm_device *dev, struct intel_vgpu *vgpu, struct intel_vgpu_fb_info *info, @@ -229,12 +237,14 @@ static int vgpu_get_plane_info(struct drm_device *dev, info->x_pos = c.x_pos; info->y_pos = c.y_pos; - /* The invalid cursor hotspot value is delivered to host - * until we find a way to get the cursor hotspot info of - * guest OS. - */ - info->x_hot = UINT_MAX; - info->y_hot = UINT_MAX; + if (validate_hotspot(&c)) { + info->x_hot = c.x_hot; + info->y_hot = c.y_hot; + } else { + info->x_hot = UINT_MAX; + info->y_hot = UINT_MAX; + } + info->size = (((info->stride * c.height * c.bpp) / 8) + (PAGE_SIZE - 1)) >> PAGE_SHIFT; } else { diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c index 1c120683e958..5e7468bd1b36 100644 --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c @@ -36,6 +36,7 @@ #include #include "i915_drv.h" #include "gvt.h" +#include "i915_pvinfo.h" #define PRIMARY_FORMAT_NUM 16 struct pixel_format { @@ -384,6 +385,8 @@ int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu, plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT; plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT; + plane->x_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)); + plane->y_hot = vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)); return 0; } diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 4b6532fb789a..d5e206661048 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -1204,8 +1204,8 @@ static int pvinfo_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, ret = handle_g2v_notification(vgpu, data); break; /* add xhot and yhot to handled list to avoid error log */ - case 0x78830: - case 0x78834: + case _vgtif_reg(cursor_x_hot): + case _vgtif_reg(cursor_y_hot): case _vgtif_reg(pdp[0].lo): case _vgtif_reg(pdp[0].hi): case _vgtif_reg(pdp[1].lo): diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index 2e0a02a80fe4..bf75300c1ec1 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -58,6 +58,9 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu) vgpu_vreg_t(vgpu, vgtif_reg(avail_rs.fence_num)) = vgpu_fence_sz(vgpu); + vgpu_vreg_t(vgpu, vgtif_reg(cursor_x_hot)) = UINT_MAX; + vgpu_vreg_t(vgpu, vgtif_reg(cursor_y_hot)) = UINT_MAX; + gvt_dbg_core("Populate PVINFO PAGE for vGPU %d\n", vgpu->id); gvt_dbg_core("aperture base [GMADR] 0x%llx size 0x%llx\n", vgpu_aperture_gmadr_base(vgpu), vgpu_aperture_sz(vgpu)); diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h index 195203f298df..d61914a11756 100644 --- a/drivers/gpu/drm/i915/i915_pvinfo.h +++ b/drivers/gpu/drm/i915/i915_pvinfo.h @@ -93,7 +93,10 @@ struct vgt_if { u32 rsv5[4]; u32 g2v_notify; - u32 rsv6[7]; + u32 rsv6[5]; + + u32 cursor_x_hot; + u32 cursor_y_hot; struct { u32 lo; -- cgit v1.2.3-58-ga151 From f75f91574617a3c6fbc821c6b156f5777a59d0ed Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 15 May 2018 15:31:49 +0100 Subject: drm/i915: Shrink search list for active timelines When switching to the kernel context, we force the switch to occur after all currently active requests (so that we know the GPU won't switch immediately away and the kernel context remains current as we work). To do so we have to inspect all the timelines and add a fence from the active work to queue our switch afterwards. We can use the tracked set of active rings to shrink our search for active timelines. v2: Use a local to shrink the list_for_each_entry() Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180515143149.4795-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_context.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 33f8a4b3c981..4bf18b5c6f1d 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -596,41 +596,44 @@ last_request_on_engine(struct i915_timeline *timeline, static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine) { - struct i915_timeline *timeline; + struct list_head * const active_rings = &engine->i915->gt.active_rings; + struct intel_ring *ring; - list_for_each_entry(timeline, &engine->i915->gt.timelines, link) { - if (last_request_on_engine(timeline, engine)) + lockdep_assert_held(&engine->i915->drm.struct_mutex); + + list_for_each_entry(ring, active_rings, active_link) { + if (last_request_on_engine(ring->timeline, engine)) return false; } return intel_engine_has_kernel_context(engine); } -int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv) +int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) { struct intel_engine_cs *engine; - struct i915_timeline *timeline; enum intel_engine_id id; - lockdep_assert_held(&dev_priv->drm.struct_mutex); + lockdep_assert_held(&i915->drm.struct_mutex); - i915_retire_requests(dev_priv); + i915_retire_requests(i915); - for_each_engine(engine, dev_priv, id) { + for_each_engine(engine, i915, id) { + struct intel_ring *ring; struct i915_request *rq; if (engine_has_idle_kernel_context(engine)) continue; - rq = i915_request_alloc(engine, dev_priv->kernel_context); + rq = i915_request_alloc(engine, i915->kernel_context); if (IS_ERR(rq)) return PTR_ERR(rq); /* Queue this switch after all other activity */ - list_for_each_entry(timeline, &dev_priv->gt.timelines, link) { + list_for_each_entry(ring, &i915->gt.active_rings, active_link) { struct i915_request *prev; - prev = last_request_on_engine(timeline, engine); + prev = last_request_on_engine(ring->timeline, engine); if (prev) i915_sw_fence_await_sw_fence_gfp(&rq->submit, &prev->submit, -- cgit v1.2.3-58-ga151 From e7f2af7894b1ac76f2062f32724d51f23438249b Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 9 May 2018 11:16:06 +0100 Subject: drm/i915/dp: fix spelling mistakes: "seqeuncer" and "seqeuencer" Trivial fix to spelling mistakes in WARN warning message text and in comments: "seqeuncer", "seqeuencer" -> "sequencer" Signed-off-by: Colin Ian King Signed-off-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/20180509101606.17483-1-colin.king@canonical.com --- drivers/gpu/drm/i915/intel_dp.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index dde92e4af5d3..2cc58596ff5a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -513,7 +513,7 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp) uint32_t DP; if (WARN(I915_READ(intel_dp->output_reg) & DP_PORT_EN, - "skipping pipe %c power seqeuncer kick due to port %c being active\n", + "skipping pipe %c power sequencer kick due to port %c being active\n", pipe_name(pipe), port_name(intel_dig_port->base.port))) return; @@ -554,7 +554,7 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp) /* * Similar magic as in intel_dp_enable_port(). * We _must_ do this port enable + disable trick - * to make this power seqeuencer lock onto the port. + * to make this power sequencer lock onto the port. * Otherwise even VDD force bit won't work. */ I915_WRITE(intel_dp->output_reg, DP); @@ -3066,11 +3066,11 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp) edp_panel_vdd_off_sync(intel_dp); /* - * VLV seems to get confused when multiple power seqeuencers + * VLV seems to get confused when multiple power sequencers * have the same port selected (even if only one has power/vdd * enabled). The failure manifests as vlv_wait_port_ready() failing * CHV on the other hand doesn't seem to mind having the same port - * selected in multiple power seqeuencers, but let's clear the + * selected in multiple power sequencers, but let's clear the * port select always when logically disconnecting a power sequencer * from a port. */ @@ -5698,7 +5698,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp, /* * On some VLV machines the BIOS can leave the VDD - * enabled even on power seqeuencers which aren't + * enabled even on power sequencers which aren't * hooked up to any port. This would mess up the * power domain tracking the first time we pick * one of these power sequencers for use since @@ -5706,7 +5706,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp, * already on and therefore wouldn't grab the power * domain reference. Disable VDD first to avoid this. * This also avoids spuriously turning the VDD on as - * soon as the new power seqeuencer gets initialized. + * soon as the new power sequencer gets initialized. */ if (force_disable_vdd) { u32 pp = ironlake_get_pp_control(intel_dp); -- cgit v1.2.3-58-ga151 From b8444cf85b629413ad23f5b6ed9bf5267f38acab Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:48 +0100 Subject: drm/i915: Remove tasklet flush before disable The idea was to try and let the existing tasklet run to completion before we began the reset, but it involves a racy check against anything else that tries to run the tasklet. Rather than acknowledge and ignore the race, let it be and don't try and be too clever. The tasklet will resume execution after reset (after spinning a bit during reset), but before we allow it to resume we will have cleared all the pending state. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0a2070112b66..0dc369a9ec4d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3035,16 +3035,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) * calling engine->init_hw() and also writing the ELSP. * Turning off the execlists->tasklet until the reset is over * prevents the race. - * - * Note that this needs to be a single atomic operation on the - * tasklet (flush existing tasks, prevent new tasks) to prevent - * a race between reset and set-wedged. It is not, so we do the best - * we can atm and make sure we don't lock the machine up in the more - * common case of recursively being called from set-wedged from inside - * i915_reset. */ - if (!atomic_read(&engine->execlists.tasklet.count)) - tasklet_kill(&engine->execlists.tasklet); tasklet_disable(&engine->execlists.tasklet); /* -- cgit v1.2.3-58-ga151 From f351d087d8329a08eca9e69872c3906c139e1f11 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:49 +0100 Subject: drm/i915: Only sync tasklets once for recursive reset preparation When setting up reset, we may need to recursively prepare an engine. In which case we should only synchronously flush the tasklets on the outer most call, the inner calls will then be inside an atomic section where the tasklet will never be run (and so the sync will never complete). Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0dc369a9ec4d..982393907b80 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3036,7 +3036,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) * Turning off the execlists->tasklet until the reset is over * prevents the race. */ - tasklet_disable(&engine->execlists.tasklet); + __tasklet_disable_sync_once(&engine->execlists.tasklet); /* * We're using worker to queue preemption requests from the tasklet in diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index 525920404ede..5bf24cfc218c 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -26,6 +26,7 @@ #define __I915_GEM_H__ #include +#include struct drm_i915_private; @@ -72,4 +73,10 @@ struct drm_i915_private; void i915_gem_park(struct drm_i915_private *i915); void i915_gem_unpark(struct drm_i915_private *i915); +static inline void __tasklet_disable_sync_once(struct tasklet_struct *t) +{ + if (atomic_inc_return(&t->count) == 1) + tasklet_unlock_wait(t); +} + #endif /* __I915_GEM_H__ */ -- cgit v1.2.3-58-ga151 From ef2fb7204638f0c67446a90b75e1eae4330682f8 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:50 +0100 Subject: drm/i915/execlists: Refactor out complete_preempt_context() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As a complement to inject_preempt_context(), follow up with the function to handle its completion. This will be useful should we wish to extend the duties of the preempt-context for execlists. v2: And do the same for the guc. Signed-off-by: Chris Wilson Cc: Jeff McGee Cc: Michał Winiarski Reviewed-by: Jeff McGee #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-3-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_guc_submission.c | 26 +++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.c | 23 +++++++++++++---------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 2feb65096966..ca38ac9ff4fa 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -623,6 +623,21 @@ static void wait_for_guc_preempt_report(struct intel_engine_cs *engine) report->report_return_status = INTEL_GUC_REPORT_STATUS_UNKNOWN; } +static void complete_preempt_context(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists *execlists = &engine->execlists; + + GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)); + + execlists_cancel_port_requests(execlists); + execlists_unwind_incomplete_requests(execlists); + + wait_for_guc_preempt_report(engine); + intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0); + + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); +} + /** * guc_submit() - Submit commands through GuC * @engine: engine associated with the commands @@ -793,15 +808,8 @@ static void guc_submission_tasklet(unsigned long data) if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) && intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) == - GUC_PREEMPT_FINISHED) { - execlists_cancel_port_requests(&engine->execlists); - execlists_unwind_incomplete_requests(execlists); - - wait_for_guc_preempt_report(engine); - - execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); - intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0); - } + GUC_PREEMPT_FINISHED) + complete_preempt_context(engine); if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)) guc_dequeue(engine); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 7c6164d14bd8..fafa0e9bd0cd 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -552,8 +552,18 @@ static void inject_preempt_context(struct intel_engine_cs *engine) if (execlists->ctrl_reg) writel(EL_CTRL_LOAD, execlists->ctrl_reg); - execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); - execlists_set_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT); + execlists_clear_active(execlists, EXECLISTS_ACTIVE_HWACK); + execlists_set_active(execlists, EXECLISTS_ACTIVE_PREEMPT); +} + +static void complete_preempt_context(struct intel_engine_execlists *execlists) +{ + GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)); + + execlists_cancel_port_requests(execlists); + execlists_unwind_incomplete_requests(execlists); + + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); } static bool __execlists_dequeue(struct intel_engine_cs *engine) @@ -1064,14 +1074,7 @@ static void execlists_submission_tasklet(unsigned long data) if (status & GEN8_CTX_STATUS_COMPLETE && buf[2*head + 1] == execlists->preempt_complete_status) { GEM_TRACE("%s preempt-idle\n", engine->name); - - execlists_cancel_port_requests(execlists); - execlists_unwind_incomplete_requests(execlists); - - GEM_BUG_ON(!execlists_is_active(execlists, - EXECLISTS_ACTIVE_PREEMPT)); - execlists_clear_active(execlists, - EXECLISTS_ACTIVE_PREEMPT); + complete_preempt_context(execlists); continue; } -- cgit v1.2.3-58-ga151 From 5adfb772f8ac1224fe4d651f3628a2e03190f5af Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:51 +0100 Subject: drm/i915: Move engine reset prepare/finish to backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation to more carefully handling incomplete preemption during reset by execlists, we move the existing code wholesale to the backends under a couple of new reset vfuncs. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Reviewed-by: Jeff McGee Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-4-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 38 ++++---------------------- drivers/gpu/drm/i915/intel_lrc.c | 47 ++++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++++++++++++--- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++++-- 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 982393907b80..abf661d40641 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3004,7 +3004,7 @@ i915_gem_find_active_request(struct intel_engine_cs *engine) struct i915_request * i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) { - struct i915_request *request = NULL; + struct i915_request *request; /* * During the reset sequence, we must prevent the engine from @@ -3027,31 +3027,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) */ kthread_park(engine->breadcrumbs.signaler); - /* - * Prevent request submission to the hardware until we have - * completed the reset in i915_gem_reset_finish(). If a request - * is completed by one engine, it may then queue a request - * to a second via its execlists->tasklet *just* as we are - * calling engine->init_hw() and also writing the ELSP. - * Turning off the execlists->tasklet until the reset is over - * prevents the race. - */ - __tasklet_disable_sync_once(&engine->execlists.tasklet); - - /* - * We're using worker to queue preemption requests from the tasklet in - * GuC submission mode. - * Even though tasklet was disabled, we may still have a worker queued. - * Let's make sure that all workers scheduled before disabling the - * tasklet are completed before continuing with the reset. - */ - if (engine->i915->guc.preempt_wq) - flush_workqueue(engine->i915->guc.preempt_wq); - - if (engine->irq_seqno_barrier) - engine->irq_seqno_barrier(engine); - - request = i915_gem_find_active_request(engine); + request = engine->reset.prepare(engine); if (request && request->fence.error == -EIO) request = ERR_PTR(-EIO); /* Previous reset failed! */ @@ -3202,13 +3178,8 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine, if (request) request = i915_gem_reset_request(engine, request, stalled); - if (request) { - DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n", - engine->name, request->global_seqno); - } - /* Setup the CS to resume from the breadcrumb of the hung request */ - engine->reset_hw(engine, request); + engine->reset.reset(engine, request); } void i915_gem_reset(struct drm_i915_private *dev_priv, @@ -3256,7 +3227,8 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { - tasklet_enable(&engine->execlists.tasklet); + engine->reset.finish(engine); + kthread_unpark(engine->breadcrumbs.signaler); intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index fafa0e9bd0cd..a8f2704e377c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1827,8 +1827,39 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine) return 0; } -static void reset_common_ring(struct intel_engine_cs *engine, - struct i915_request *request) +static struct i915_request * +execlists_reset_prepare(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists * const execlists = &engine->execlists; + + GEM_TRACE("%s\n", engine->name); + + /* + * Prevent request submission to the hardware until we have + * completed the reset in i915_gem_reset_finish(). If a request + * is completed by one engine, it may then queue a request + * to a second via its execlists->tasklet *just* as we are + * calling engine->init_hw() and also writing the ELSP. + * Turning off the execlists->tasklet until the reset is over + * prevents the race. + */ + __tasklet_disable_sync_once(&execlists->tasklet); + + /* + * We're using worker to queue preemption requests from the tasklet in + * GuC submission mode. + * Even though tasklet was disabled, we may still have a worker queued. + * Let's make sure that all workers scheduled before disabling the + * tasklet are completed before continuing with the reset. + */ + if (engine->i915->guc.preempt_wq) + flush_workqueue(engine->i915->guc.preempt_wq); + + return i915_gem_find_active_request(engine); +} + +static void execlists_reset(struct intel_engine_cs *engine, + struct i915_request *request) { struct intel_engine_execlists * const execlists = &engine->execlists; unsigned long flags; @@ -1908,6 +1939,13 @@ static void reset_common_ring(struct intel_engine_cs *engine, unwind_wa_tail(request); } +static void execlists_reset_finish(struct intel_engine_cs *engine) +{ + tasklet_enable(&engine->execlists.tasklet); + + GEM_TRACE("%s\n", engine->name); +} + static int intel_logical_ring_emit_pdps(struct i915_request *rq) { struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; @@ -2237,7 +2275,10 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) { /* Default vfuncs which can be overriden by each engine. */ engine->init_hw = gen8_init_common_ring; - engine->reset_hw = reset_common_ring; + + engine->reset.prepare = execlists_reset_prepare; + engine->reset.reset = execlists_reset; + engine->reset.finish = execlists_reset_finish; engine->context_pin = execlists_context_pin; engine->context_unpin = execlists_context_unpin; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8f19349a6055..af5a178366ed 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -531,9 +531,20 @@ out: return ret; } -static void reset_ring_common(struct intel_engine_cs *engine, - struct i915_request *request) +static struct i915_request *reset_prepare(struct intel_engine_cs *engine) { + if (engine->irq_seqno_barrier) + engine->irq_seqno_barrier(engine); + + return i915_gem_find_active_request(engine); +} + +static void reset_ring(struct intel_engine_cs *engine, + struct i915_request *request) +{ + GEM_TRACE("%s seqno=%x\n", + engine->name, request ? request->global_seqno : 0); + /* * RC6 must be prevented until the reset is complete and the engine * reinitialised. If it occurs in the middle of this sequence, the @@ -597,6 +608,10 @@ static void reset_ring_common(struct intel_engine_cs *engine, } } +static void reset_finish(struct intel_engine_cs *engine) +{ +} + static int intel_rcs_ctx_init(struct i915_request *rq) { int ret; @@ -2006,7 +2021,9 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv, intel_ring_init_semaphores(dev_priv, engine); engine->init_hw = init_ring_common; - engine->reset_hw = reset_ring_common; + engine->reset.prepare = reset_prepare; + engine->reset.reset = reset_ring; + engine->reset.finish = reset_finish; engine->context_pin = intel_ring_context_pin; engine->context_unpin = intel_ring_context_unpin; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 010750e8ee44..1e8bacba7754 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -423,8 +423,13 @@ struct intel_engine_cs { void (*irq_disable)(struct intel_engine_cs *engine); int (*init_hw)(struct intel_engine_cs *engine); - void (*reset_hw)(struct intel_engine_cs *engine, - struct i915_request *rq); + + struct { + struct i915_request *(*prepare)(struct intel_engine_cs *engine); + void (*reset)(struct intel_engine_cs *engine, + struct i915_request *rq); + void (*finish)(struct intel_engine_cs *engine); + } reset; void (*park)(struct intel_engine_cs *engine); void (*unpark)(struct intel_engine_cs *engine); -- cgit v1.2.3-58-ga151 From 1329115c6c4370e93813b44084ee06610c3279bd Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:52 +0100 Subject: drm/i915: Split execlists/guc reset preparations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the next patch, we will make the execlists reset prepare callback take into account preemption by flushing the context-switch handler. This is not applicable to the GuC submission backend, so split the two into their own backend callbacks. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Reviewed-by: Jeff McGee Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-5-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_guc_submission.c | 34 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 12 ++-------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index ca38ac9ff4fa..637e852888ec 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -815,6 +815,37 @@ static void guc_submission_tasklet(unsigned long data) guc_dequeue(engine); } +static struct i915_request * +guc_reset_prepare(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists * const execlists = &engine->execlists; + + GEM_TRACE("%s\n", engine->name); + + /* + * Prevent request submission to the hardware until we have + * completed the reset in i915_gem_reset_finish(). If a request + * is completed by one engine, it may then queue a request + * to a second via its execlists->tasklet *just* as we are + * calling engine->init_hw() and also writing the ELSP. + * Turning off the execlists->tasklet until the reset is over + * prevents the race. + */ + __tasklet_disable_sync_once(&execlists->tasklet); + + /* + * We're using worker to queue preemption requests from the tasklet in + * GuC submission mode. + * Even though tasklet was disabled, we may still have a worker queued. + * Let's make sure that all workers scheduled before disabling the + * tasklet are completed before continuing with the reset. + */ + if (engine->i915->guc.preempt_wq) + flush_workqueue(engine->i915->guc.preempt_wq); + + return i915_gem_find_active_request(engine); +} + /* * Everything below here is concerned with setup & teardown, and is * therefore not part of the somewhat time-critical batch-submission @@ -1275,6 +1306,9 @@ int intel_guc_submission_enable(struct intel_guc *guc) &engine->execlists; execlists->tasklet.func = guc_submission_tasklet; + + engine->reset.prepare = guc_reset_prepare; + engine->park = guc_submission_park; engine->unpark = guc_submission_unpark; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a8f2704e377c..acbdf36d1c28 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1845,16 +1845,6 @@ execlists_reset_prepare(struct intel_engine_cs *engine) */ __tasklet_disable_sync_once(&execlists->tasklet); - /* - * We're using worker to queue preemption requests from the tasklet in - * GuC submission mode. - * Even though tasklet was disabled, we may still have a worker queued. - * Let's make sure that all workers scheduled before disabling the - * tasklet are completed before continuing with the reset. - */ - if (engine->i915->guc.preempt_wq) - flush_workqueue(engine->i915->guc.preempt_wq); - return i915_gem_find_active_request(engine); } @@ -2256,6 +2246,8 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->schedule = execlists_schedule; engine->execlists.tasklet.func = execlists_submission_tasklet; + engine->reset.prepare = execlists_reset_prepare; + engine->park = NULL; engine->unpark = NULL; -- cgit v1.2.3-58-ga151 From 73377dbcc754f1e673b60f238c237c5e909f92b1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:53 +0100 Subject: drm/i915/execlists: Split out CSB processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull the CSB event processing into its own routine so that we can reuse it during reset to flush any missed interrupts/events. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-6-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_lrc.c | 91 +++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index acbdf36d1c28..f38391e6431a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -958,34 +958,14 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) local_irq_restore(flags); } -/* - * Check the unread Context Status Buffers and manage the submission of new - * contexts to the ELSP accordingly. - */ -static void execlists_submission_tasklet(unsigned long data) +static void process_csb(struct intel_engine_cs *engine) { - struct intel_engine_cs * const engine = (struct intel_engine_cs *)data; struct intel_engine_execlists * const execlists = &engine->execlists; struct execlist_port *port = execlists->port; - struct drm_i915_private *dev_priv = engine->i915; + struct drm_i915_private *i915 = engine->i915; bool fw = false; - /* - * We can skip acquiring intel_runtime_pm_get() here as it was taken - * on our behalf by the request (see i915_gem_mark_busy()) and it will - * not be relinquished until the device is idle (see - * i915_gem_idle_work_handler()). As a precaution, we make sure - * that all ELSP are drained i.e. we have processed the CSB, - * before allowing ourselves to idle and calling intel_runtime_pm_put(). - */ - GEM_BUG_ON(!dev_priv->gt.awake); - - /* - * Prefer doing test_and_clear_bit() as a two stage operation to avoid - * imposing the cost of a locked atomic transaction when submitting a - * new request (outside of the context-switch interrupt). - */ - while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) { + do { /* The HWSP contains a (cacheable) mirror of the CSB */ const u32 *buf = &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX]; @@ -993,28 +973,27 @@ static void execlists_submission_tasklet(unsigned long data) if (unlikely(execlists->csb_use_mmio)) { buf = (u32 * __force) - (dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0))); - execlists->csb_head = -1; /* force mmio read of CSB ptrs */ + (i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0))); + execlists->csb_head = -1; /* force mmio read of CSB */ } /* Clear before reading to catch new interrupts */ clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); smp_mb__after_atomic(); - if (unlikely(execlists->csb_head == -1)) { /* following a reset */ + if (unlikely(execlists->csb_head == -1)) { /* after a reset */ if (!fw) { - intel_uncore_forcewake_get(dev_priv, - execlists->fw_domains); + intel_uncore_forcewake_get(i915, execlists->fw_domains); fw = true; } - head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); + head = readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); tail = GEN8_CSB_WRITE_PTR(head); head = GEN8_CSB_READ_PTR(head); execlists->csb_head = head; } else { const int write_idx = - intel_hws_csb_write_index(dev_priv) - + intel_hws_csb_write_index(i915) - I915_HWS_CSB_BUF0_INDEX; head = execlists->csb_head; @@ -1023,8 +1002,8 @@ static void execlists_submission_tasklet(unsigned long data) } GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n", engine->name, - head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?", - tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?"); + head, GEN8_CSB_READ_PTR(readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?", + tail, GEN8_CSB_WRITE_PTR(readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?"); while (head != tail) { struct i915_request *rq; @@ -1034,7 +1013,8 @@ static void execlists_submission_tasklet(unsigned long data) if (++head == GEN8_CSB_ENTRIES) head = 0; - /* We are flying near dragons again. + /* + * We are flying near dragons again. * * We hold a reference to the request in execlist_port[] * but no more than that. We are operating in softirq @@ -1143,15 +1123,48 @@ static void execlists_submission_tasklet(unsigned long data) if (head != execlists->csb_head) { execlists->csb_head = head; writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK, head << 8), - dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); + i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); } - } + } while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)); - if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)) - execlists_dequeue(engine); + if (unlikely(fw)) + intel_uncore_forcewake_put(i915, execlists->fw_domains); +} - if (fw) - intel_uncore_forcewake_put(dev_priv, execlists->fw_domains); +/* + * Check the unread Context Status Buffers and manage the submission of new + * contexts to the ELSP accordingly. + */ +static void execlists_submission_tasklet(unsigned long data) +{ + struct intel_engine_cs * const engine = (struct intel_engine_cs *)data; + + GEM_TRACE("%s awake?=%d, active=%x, irq-posted?=%d\n", + engine->name, + engine->i915->gt.awake, + engine->execlists.active, + test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)); + + /* + * We can skip acquiring intel_runtime_pm_get() here as it was taken + * on our behalf by the request (see i915_gem_mark_busy()) and it will + * not be relinquished until the device is idle (see + * i915_gem_idle_work_handler()). As a precaution, we make sure + * that all ELSP are drained i.e. we have processed the CSB, + * before allowing ourselves to idle and calling intel_runtime_pm_put(). + */ + GEM_BUG_ON(!engine->i915->gt.awake); + + /* + * Prefer doing test_and_clear_bit() as a two stage operation to avoid + * imposing the cost of a locked atomic transaction when submitting a + * new request (outside of the context-switch interrupt). + */ + if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) + process_csb(engine); + + if (!execlists_is_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT)) + execlists_dequeue(engine); /* If the engine is now idle, so should be the flag; and vice versa. */ GEM_BUG_ON(execlists_is_active(&engine->execlists, -- cgit v1.2.3-58-ga151 From 63572937cebf5d229a87fb11b201864f7d0b8171 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:54 +0100 Subject: drm/i915/execlists: Flush pending preemption events during reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catch up with the inflight CSB events, after disabling the tasklet before deciding which request was truly guilty of hanging the GPU. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-7-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_lrc.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f38391e6431a..a83273c66d49 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1844,6 +1844,7 @@ static struct i915_request * execlists_reset_prepare(struct intel_engine_cs *engine) { struct intel_engine_execlists * const execlists = &engine->execlists; + struct i915_request *request, *active; GEM_TRACE("%s\n", engine->name); @@ -1858,7 +1859,40 @@ execlists_reset_prepare(struct intel_engine_cs *engine) */ __tasklet_disable_sync_once(&execlists->tasklet); - return i915_gem_find_active_request(engine); + /* + * We want to flush the pending context switches, having disabled + * the tasklet above, we can assume exclusive access to the execlists. + * For this allows us to catch up with an inflight preemption event, + * and avoid blaming an innocent request if the stall was due to the + * preemption itself. + */ + if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) + process_csb(engine); + + /* + * The last active request can then be no later than the last request + * now in ELSP[0]. So search backwards from there, so that if the GPU + * has advanced beyond the last CSB update, it will be pardoned. + */ + active = NULL; + request = port_request(execlists->port); + if (request) { + unsigned long flags; + + spin_lock_irqsave(&engine->timeline.lock, flags); + list_for_each_entry_from_reverse(request, + &engine->timeline.requests, + link) { + if (__i915_request_completed(request, + request->global_seqno)) + break; + + active = request; + } + spin_unlock_irqrestore(&engine->timeline.lock, flags); + } + + return active; } static void execlists_reset(struct intel_engine_cs *engine, -- cgit v1.2.3-58-ga151 From 3f6e9822308127104a7bb007ca569f2c57d03b67 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 May 2018 19:33:55 +0100 Subject: drm/i915: Stop parking the signaler around reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We cannot call kthread_park() from softirq context, so let's avoid it entirely during the reset. We wanted to suspend the signaler so that it would not mark a request as complete at the same time as we marked it as being in error. Instead of parking the signaling, stop the engine from advancing so that the GPU doesn't emit the breadcrumb for our chosen "guilty" request. v2: Refactor setting STOP_RING so that we don't have the same code thrice Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michałt Winiarski CC: Michel Thierry Cc: Jeff McGee Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-8-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 14 -------------- drivers/gpu/drm/i915/intel_engine_cs.c | 29 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ drivers/gpu/drm/i915/intel_uncore.c | 12 +++--------- 6 files changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index abf661d40641..b0fe452ce17c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3015,18 +3015,6 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) */ intel_uncore_forcewake_get(engine->i915, FORCEWAKE_ALL); - /* - * Prevent the signaler thread from updating the request - * state (by calling dma_fence_signal) as we are processing - * the reset. The write from the GPU of the seqno is - * asynchronous and the signaler thread may see a different - * value to us and declare the request complete, even though - * the reset routine have picked that request as the active - * (incomplete) request. This conflict is not handled - * gracefully! - */ - kthread_park(engine->breadcrumbs.signaler); - request = engine->reset.prepare(engine); if (request && request->fence.error == -EIO) request = ERR_PTR(-EIO); /* Previous reset failed! */ @@ -3229,8 +3217,6 @@ void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { engine->reset.finish(engine); - kthread_unpark(engine->breadcrumbs.signaler); - intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); } diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 6bfd7e3ed152..d4e159ae65a6 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -769,6 +769,35 @@ u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine) return bbaddr; } +int intel_engine_stop_cs(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + const u32 base = engine->mmio_base; + const i915_reg_t mode = RING_MI_MODE(base); + int err; + + if (INTEL_GEN(dev_priv) < 3) + return -ENODEV; + + GEM_TRACE("%s\n", engine->name); + + I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING)); + + err = 0; + if (__intel_wait_for_register_fw(dev_priv, + mode, MODE_IDLE, MODE_IDLE, + 1000, 0, + NULL)) { + GEM_TRACE("%s: timed out on STOP_RING -> IDLE\n", engine->name); + err = -ETIMEDOUT; + } + + /* A final mmio read to let GPU writes be hopefully flushed to memory */ + POSTING_READ_FW(mode); + + return err; +} + const char *i915_cache_level_str(struct drm_i915_private *i915, int type) { switch (type) { diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a83273c66d49..493e6bd9e28a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1879,6 +1879,12 @@ execlists_reset_prepare(struct intel_engine_cs *engine) if (request) { unsigned long flags; + /* + * Prevent the breadcrumb from advancing before we decide + * which request is currently active. + */ + intel_engine_stop_cs(engine); + spin_lock_irqsave(&engine->timeline.lock, flags); list_for_each_entry_from_reverse(request, &engine->timeline.requests, diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index af5a178366ed..6f200a747176 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -533,6 +533,8 @@ out: static struct i915_request *reset_prepare(struct intel_engine_cs *engine) { + intel_engine_stop_cs(engine); + if (engine->irq_seqno_barrier) engine->irq_seqno_barrier(engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 1e8bacba7754..61f385a92484 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -878,6 +878,8 @@ int intel_init_bsd_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); +int intel_engine_stop_cs(struct intel_engine_cs *engine); + u64 intel_engine_get_active_head(const struct intel_engine_cs *engine); u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 448293eb638d..b36a3b5736a0 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1702,15 +1702,9 @@ static void gen3_stop_engine(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; const u32 base = engine->mmio_base; - const i915_reg_t mode = RING_MI_MODE(base); - - I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING)); - if (__intel_wait_for_register_fw(dev_priv, - mode, MODE_IDLE, MODE_IDLE, - 500, 0, - NULL)) - DRM_DEBUG_DRIVER("%s: timed out on STOP_RING\n", - engine->name); + + if (intel_engine_stop_cs(engine)) + DRM_DEBUG_DRIVER("%s: timed out on STOP_RING\n", engine->name); I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base))); POSTING_READ_FW(RING_HEAD(base)); /* paranoia */ -- cgit v1.2.3-58-ga151 From 579e2f6d999991d2d2dd39c7185cba0a97137cee Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 15 Mar 2017 17:39:50 +0000 Subject: drm/i915/gvt: Use offsetofend() rather than offsetof + sizeof Compute the offset of the end of the crc32 field using offsetofend() rather than open-coding. Signed-off-by: Chris Wilson Cc: Zhenyu Wang Cc: Zhi Wang Reviewed-by: Mika Kuoppala Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/firmware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index a73e1d418c22..4ac18b447247 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -162,7 +162,7 @@ static int verify_firmware(struct intel_gvt *gvt, h = (struct gvt_firmware_header *)fw->data; - crc32_start = offsetof(struct gvt_firmware_header, crc32) + 4; + crc32_start = offsetofend(struct gvt_firmware_header, crc32); mem = fw->data + crc32_start; #define VERIFY(s, a, b) do { \ -- cgit v1.2.3-58-ga151 From 3257ec797d3a8c5232389eb1952d4451e80f3931 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 17 May 2018 15:37:59 +0200 Subject: drm/vc4: plane: Expand the lower bits by repeating the higher bits The vc4 HVS uses an internal RGB888 representation of the frames, and will by default expand formats using a lower depth using zeros. This causes an issue when we try to use other compositing software such as pixman that fill the missing bits by repeating the higher significant bits. As such, we can't check the display output in a reliable way by doing a software composition and an hardware one and compare both. To prevent this, force the same behaviour so that we can do such things. Signed-off-by: Maxime Ripard Signed-off-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20180517133759.25626-1-maxime.ripard@bootlin.com --- drivers/gpu/drm/vc4/vc4_plane.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 3483c05cc3d6..6e8984aee613 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -544,6 +544,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, /* Control word */ vc4_dlist_write(vc4_state, SCALER_CTL0_VALID | + VC4_SET_FIELD(SCALER_CTL0_RGBA_EXPAND_ROUND, SCALER_CTL0_RGBA_EXPAND) | (format->pixel_order << SCALER_CTL0_ORDER_SHIFT) | (format->hvs << SCALER_CTL0_PIXEL_FORMAT_SHIFT) | VC4_SET_FIELD(tiling, SCALER_CTL0_TILING) | -- cgit v1.2.3-58-ga151 From 5d435b46fa68c53af8dc3a69ff2936b8f17aa294 Mon Sep 17 00:00:00 2001 From: Philippe Cornu Date: Tue, 15 May 2018 22:37:36 +0200 Subject: drm/bridge: spelling and coding style minor fixes Minor fixes detected with "scripts/checkpatch.pl --strict" Signed-off-by: Philippe Cornu Acked-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20180515203736.9224-1-philippe.cornu@st.com --- include/drm/drm_bridge.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 9917651a7fdd..70131ab57e8f 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -97,7 +97,7 @@ struct drm_bridge_funcs { /** * @mode_fixup: * - * This callback is used to validate and adjust a mode. The paramater + * This callback is used to validate and adjust a mode. The parameter * mode is the display mode that should be fed to the next element in * the display chain, either the final &drm_connector or the next * &drm_bridge. The parameter adjusted_mode is the input mode the bridge @@ -301,15 +301,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, struct drm_bridge *previous); bool drm_bridge_mode_fixup(struct drm_bridge *bridge, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge, const struct drm_display_mode *mode); void drm_bridge_disable(struct drm_bridge *bridge); void drm_bridge_post_disable(struct drm_bridge *bridge); void drm_bridge_mode_set(struct drm_bridge *bridge, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); void drm_bridge_pre_enable(struct drm_bridge *bridge); void drm_bridge_enable(struct drm_bridge *bridge); -- cgit v1.2.3-58-ga151 From 50525c332b55f899fb231d786931d0b45a3f3d41 Mon Sep 17 00:00:00 2001 From: Stanislav Lisovskiy Date: Tue, 15 May 2018 16:59:27 +0300 Subject: drm: content-type property for HDMI connector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added content_type property to drm_connector_state in order to properly handle external HDMI TV content-type setting. v2: * Moved helper function which attaches content type property to the drm core, as was suggested. Removed redundant connector state initialization. v3: * Removed caps in drm_content_type_enum_list. After some discussion it turned out that HDMI Spec 1.4 was wrongly assuming that IT Content(itc) bit doesn't affect Content type states, however itc bit needs to be manupulated as well. In order to not expose additional property for itc, for sake of simplicity it was decided to bind those together in same "content type" property. v4: * Added it_content checking in intel_digital_connector_atomic_check. Fixed documentation for new content type enum. v5: * Moved patch revision's description to commit messages. v6: * Minor naming fix for the content type enumeration string. v7: * Fix parameter name for documentation and parameter alignment in order not to get warning. Added Content Type description to new HDMI connector properties section. v8: * Thrown away unneeded numbers from HDMI content-type property description. Switch to strings desription instead of plain definitions. v9: * Moved away hdmi specific content-type enum from drm_connector_state. Content type property should probably not be bound to any specific connector interface in drm_connector_state. Same probably should be done to hdmi_picture_aspect_ration enum which is also contained in drm_connector_state. Added special helper function to get derive hdmi specific relevant infoframe fields. v10: * Added usage description to HDMI properties kernel doc. v11: * Created centralized function for filling HDMI AVI infoframe, based on correspondent DRM property value. Acked-by: Hans Verkuil Acked-by: Daniel Vetter Signed-off-by: Stanislav Lisovskiy Link: https://patchwork.freedesktop.org/patch/msgid/20180515135928.31092-2-stanislav.lisovskiy@intel.com [vsyrjala: clean up checkpatch multiple blank lines warnings] Signed-off-by: Ville Syrjälä --- Documentation/gpu/drm-kms.rst | 6 ++ Documentation/gpu/kms-properties.csv | 1 + drivers/gpu/drm/drm_atomic.c | 4 ++ drivers/gpu/drm/drm_connector.c | 113 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_edid.c | 8 +++ include/drm/drm_connector.h | 13 ++++ include/drm/drm_mode_config.h | 5 ++ include/uapi/drm/drm_mode.h | 7 +++ 8 files changed, 157 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 1dffd1ac4cd4..e233c2626bd0 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -517,6 +517,12 @@ Standard Connector Properties .. kernel-doc:: drivers/gpu/drm/drm_connector.c :doc: standard connector properties +HDMI Specific Connector Properties +----------------------------- + +.. kernel-doc:: drivers/gpu/drm/drm_connector.c + :doc: HDMI connector properties + Plane Composition Properties ---------------------------- diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv index 07ed22ea3bd6..bfde04eddd14 100644 --- a/Documentation/gpu/kms-properties.csv +++ b/Documentation/gpu/kms-properties.csv @@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,De ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an X offset for a connector ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an Y offset for a connector ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" }",Connector,TDB +,Optional,"""content type""",ENUM,"{ ""No Data"", ""Graphics"", ""Photo"", ""Cinema"", ""Game"" }",Connector,TBD i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is set, the hardware will be programmed with the result of the multiplication of CTM by the limited range matrix to ensure the pixels normaly in the range 0..1.0 are remapped to the range 16/255..235/255." ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } etc.",Connector,TBD diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index dc850b4b6e21..07fef42869aa 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1270,6 +1270,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, state->link_status = val; } else if (property == config->aspect_ratio_property) { state->picture_aspect_ratio = val; + } else if (property == config->content_type_property) { + state->content_type = val; } else if (property == connector->scaling_mode_property) { state->scaling_mode = val; } else if (property == connector->content_protection_property) { @@ -1355,6 +1357,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, *val = state->link_status; } else if (property == config->aspect_ratio_property) { *val = state->picture_aspect_ratio; + } else if (property == config->content_type_property) { + *val = state->content_type; } else if (property == connector->scaling_mode_property) { *val = state->scaling_mode; } else if (property == connector->content_protection_property) { diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 9b9ba5d5ec0c..002b244391f9 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -720,6 +720,14 @@ static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" }, }; +static const struct drm_prop_enum_list drm_content_type_enum_list[] = { + { DRM_MODE_CONTENT_TYPE_NO_DATA, "No Data" }, + { DRM_MODE_CONTENT_TYPE_GRAPHICS, "Graphics" }, + { DRM_MODE_CONTENT_TYPE_PHOTO, "Photo" }, + { DRM_MODE_CONTENT_TYPE_CINEMA, "Cinema" }, + { DRM_MODE_CONTENT_TYPE_GAME, "Game" }, +}; + static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = { { DRM_MODE_PANEL_ORIENTATION_NORMAL, "Normal" }, { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down" }, @@ -996,6 +1004,84 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_dvi_i_properties); +/** + * DOC: HDMI connector properties + * + * content type (HDMI specific): + * Indicates content type setting to be used in HDMI infoframes to indicate + * content type for the external device, so that it adjusts it's display + * settings accordingly. + * + * The value of this property can be one of the following: + * + * No Data: + * Content type is unknown + * Graphics: + * Content type is graphics + * Photo: + * Content type is photo + * Cinema: + * Content type is cinema + * Game: + * Content type is game + * + * Drivers can set up this property by calling + * drm_connector_attach_content_type_property(). Decoding to + * infoframe values is done through + * drm_hdmi_get_content_type_from_property() and + * drm_hdmi_get_itc_bit_from_property(). + */ + +/** + * drm_connector_attach_content_type_property - attach content-type property + * @connector: connector to attach content type property on. + * + * Called by a driver the first time a HDMI connector is made. + */ +int drm_connector_attach_content_type_property(struct drm_connector *connector) +{ + if (!drm_mode_create_content_type_property(connector->dev)) + drm_object_attach_property(&connector->base, + connector->dev->mode_config.content_type_property, + DRM_MODE_CONTENT_TYPE_NO_DATA); + return 0; +} +EXPORT_SYMBOL(drm_connector_attach_content_type_property); + + +/** + * drm_hdmi_avi_infoframe_content_type() - fill the HDMI AVI infoframe + * content type information, based + * on correspondent DRM property. + * @frame: HDMI AVI infoframe + * @conn_state: DRM display connector state + * + */ +void drm_hdmi_avi_infoframe_content_type(struct hdmi_avi_infoframe *frame, + const struct drm_connector_state *conn_state) +{ + switch (conn_state->content_type) { + case DRM_MODE_CONTENT_TYPE_GRAPHICS: + frame->content_type = HDMI_CONTENT_TYPE_GRAPHICS; + break; + case DRM_MODE_CONTENT_TYPE_CINEMA: + frame->content_type = HDMI_CONTENT_TYPE_CINEMA; + break; + case DRM_MODE_CONTENT_TYPE_GAME: + frame->content_type = HDMI_CONTENT_TYPE_GAME; + break; + case DRM_MODE_CONTENT_TYPE_PHOTO: + frame->content_type = HDMI_CONTENT_TYPE_PHOTO; + break; + default: + /* Graphics is the default(0) */ + frame->content_type = HDMI_CONTENT_TYPE_GRAPHICS; + } + + frame->itc = conn_state->content_type != DRM_MODE_CONTENT_TYPE_NO_DATA; +} +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_content_type); + /** * drm_create_tv_properties - create TV specific connector properties * @dev: DRM device @@ -1260,6 +1346,33 @@ int drm_mode_create_aspect_ratio_property(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); +/** + * drm_mode_create_content_type_property - create content type property + * @dev: DRM device + * + * Called by a driver the first time it's needed, must be attached to desired + * connectors. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_mode_create_content_type_property(struct drm_device *dev) +{ + if (dev->mode_config.content_type_property) + return 0; + + dev->mode_config.content_type_property = + drm_property_create_enum(dev, 0, "content type", + drm_content_type_enum_list, + ARRAY_SIZE(drm_content_type_enum_list)); + + if (dev->mode_config.content_type_property == NULL) + return -ENOMEM; + + return 0; +} +EXPORT_SYMBOL(drm_mode_create_content_type_property); + /** * drm_mode_create_suggested_offset_properties - create suggests offset properties * @dev: DRM device diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 40e1e24f2ff0..82f1ab09169d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4872,6 +4872,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; + /* + * As some drivers don't support atomic, we can't use connector state. + * So just initialize the frame with default values, just the same way + * as it's done with other properties here. + */ + frame->content_type = HDMI_CONTENT_TYPE_GRAPHICS; + frame->itc = 0; + /* * Populate picture aspect ratio from either * user input (if specified) or from the CEA mode list. diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 675cc3f8cf85..ee4c48218c85 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -418,6 +418,14 @@ struct drm_connector_state { */ enum hdmi_picture_aspect picture_aspect_ratio; + /** + * @content_type: Connector property to control the + * HDMI infoframe content type setting. + * The %DRM_MODE_CONTENT_TYPE_\* values much + * match the values. + */ + unsigned int content_type; + /** * @scaling_mode: Connector property to control the * upscaling, mostly used for built-in panels. @@ -1089,11 +1097,16 @@ int drm_mode_create_tv_properties(struct drm_device *dev, unsigned int num_modes, const char * const modes[]); int drm_mode_create_scaling_mode_property(struct drm_device *dev); +int drm_connector_attach_content_type_property(struct drm_connector *dev); int drm_connector_attach_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); int drm_connector_attach_content_protection_property( struct drm_connector *connector); int drm_mode_create_aspect_ratio_property(struct drm_device *dev); +int drm_mode_create_content_type_property(struct drm_device *dev); +void drm_hdmi_avi_infoframe_content_type(struct hdmi_avi_infoframe *frame, + const struct drm_connector_state *conn_state); + int drm_mode_create_suggested_offset_properties(struct drm_device *dev); int drm_mode_connector_set_path_property(struct drm_connector *connector, diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 33b3a96d66d0..fb45839179dd 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -726,6 +726,11 @@ struct drm_mode_config { * HDMI infoframe aspect ratio setting. */ struct drm_property *aspect_ratio_property; + /** + * @content_type_property: Optional connector property to control the + * HDMI infoframe content type setting. + */ + struct drm_property *content_type_property; /** * @degamma_lut_property: Optional CRTC property to set the LUT used to * convert the framebuffer's colors to linear gamma. diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 4b3a1bb58e68..971c016b368c 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -96,6 +96,13 @@ extern "C" { #define DRM_MODE_PICTURE_ASPECT_64_27 3 #define DRM_MODE_PICTURE_ASPECT_256_135 4 +/* Content type options */ +#define DRM_MODE_CONTENT_TYPE_NO_DATA 0 +#define DRM_MODE_CONTENT_TYPE_GRAPHICS 1 +#define DRM_MODE_CONTENT_TYPE_PHOTO 2 +#define DRM_MODE_CONTENT_TYPE_CINEMA 3 +#define DRM_MODE_CONTENT_TYPE_GAME 4 + /* Aspect ratio flag bitmask (4 bits 22:19) */ #define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19) #define DRM_MODE_FLAG_PIC_AR_NONE \ -- cgit v1.2.3-58-ga151 From 6553b123eae8dd117f6adb4a42e2e6ef8bff321d Mon Sep 17 00:00:00 2001 From: Stanislav Lisovskiy Date: Tue, 15 May 2018 16:59:28 +0300 Subject: i915: content-type property for HDMI connector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added encoding of drm content_type property from drm_connector_state within AVI infoframe in order to properly handle external HDMI TV content-type setting. This requires also manipulationg ITC bit, as stated in HDMI spec. v2: * Moved helper function which attaches content type property to the drm core, as was suggested. Removed redundant connector state initialization. v3: * Removed caps in drm_content_type_enum_list. After some discussion it turned out that HDMI Spec 1.4 was wrongly assuming that IT Content(itc) bit doesn't affect Content type states, however itc bit needs to be manupulated as well. In order to not expose additional property for itc, for sake of simplicity it was decided to bind those together in same "content type" property. v4: * Added it_content checking in intel_digital_connector_atomic_check. Fixed documentation for new content type enum. v5: * Moved patch revision's description to commit messages. v6: * Minor naming fix for the content type enumeration string. v7: * Fix parameter name for documentation and parameter alignment in order not to get warning. Added Content Type description to new HDMI connector properties section. v8: * Thrown away unneeded numbers from HDMI content-type property description. Switch to strings desription instead of plain definitions. v9: * Moved away hdmi specific content-type enum from drm_connector_state. Content type property should probably not be bound to any specific connector interface in drm_connector_state. Same probably should be done to hdmi_picture_aspect_ration enum which is also contained in drm_connector_state. Added special helper function to get derive hdmi specific relevant infoframe fields. v10: * Added usage description to HDMI properties kernel doc. v11: * Created centralized function for filling HDMI AVI infoframe, based on correspondent DRM property value. Acked-by: Hans Verkuil Acked-by: Daniel Vetter Signed-off-by: Stanislav Lisovskiy Link: https://patchwork.freedesktop.org/patch/msgid/20180515135928.31092-3-stanislav.lisovskiy@intel.com [vsyrjala: clean up checkpatch multiple blank lines warnings] Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 40285d1b91b7..61ddb5871d8a 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, if (new_conn_state->force_audio != old_conn_state->force_audio || new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb || new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio || + new_conn_state->base.content_type != old_conn_state->base.content_type || new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode) crtc_state->mode_changed = true; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee929f31f7db..ea958ed0777d 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -461,7 +461,8 @@ static void intel_write_infoframe(struct drm_encoder *encoder, } static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, - const struct intel_crtc_state *crtc_state) + const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) { struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); const struct drm_display_mode *adjusted_mode = @@ -491,6 +492,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, intel_hdmi->rgb_quant_range_selectable, is_hdmi2_sink); + drm_hdmi_avi_infoframe_content_type(&frame.avi, + conn_state); + /* TODO: handle pixel repetition for YCBCR420 outputs */ intel_write_infoframe(encoder, crtc_state, &frame); } @@ -586,7 +590,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder, I915_WRITE(reg, val); POSTING_READ(reg); - intel_hdmi_set_avi_infoframe(encoder, crtc_state); + intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state); intel_hdmi_set_spd_infoframe(encoder, crtc_state); intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state); } @@ -727,7 +731,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder, I915_WRITE(reg, val); POSTING_READ(reg); - intel_hdmi_set_avi_infoframe(encoder, crtc_state); + intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state); intel_hdmi_set_spd_infoframe(encoder, crtc_state); intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state); } @@ -770,7 +774,7 @@ static void cpt_set_infoframes(struct drm_encoder *encoder, I915_WRITE(reg, val); POSTING_READ(reg); - intel_hdmi_set_avi_infoframe(encoder, crtc_state); + intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state); intel_hdmi_set_spd_infoframe(encoder, crtc_state); intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state); } @@ -823,7 +827,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder, I915_WRITE(reg, val); POSTING_READ(reg); - intel_hdmi_set_avi_infoframe(encoder, crtc_state); + intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state); intel_hdmi_set_spd_infoframe(encoder, crtc_state); intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state); } @@ -856,7 +860,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, I915_WRITE(reg, val); POSTING_READ(reg); - intel_hdmi_set_avi_infoframe(encoder, crtc_state); + intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state); intel_hdmi_set_spd_infoframe(encoder, crtc_state); intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state); } @@ -2065,6 +2069,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_force_audio_property(connector); intel_attach_broadcast_rgb_property(connector); intel_attach_aspect_ratio_property(connector); + drm_connector_attach_content_type_property(connector); connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; } -- cgit v1.2.3-58-ga151 From 6102a8ee8ad60c51b5194e425c3cc8ce43aca384 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Mon, 14 May 2018 20:24:19 +0300 Subject: drm/i915: Clean up ADPA pipe select bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean up the ADPA pipe select bits. To make the whole situation a bit less ugly we'll start to share the same code between .get_hw_state() and the port state asserts. v2: Order the defines shift,mask,value (Jani) Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180514172423.9302-1-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 11 +++++----- drivers/gpu/drm/i915/intel_crt.c | 40 ++++++++++++++++++------------------ drivers/gpu/drm/i915/intel_display.c | 24 +++++----------------- drivers/gpu/drm/i915/intel_drv.h | 2 ++ 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f11bb213ec07..ae3c26216996 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4133,11 +4133,12 @@ enum { #define ADPA_DAC_ENABLE (1<<31) #define ADPA_DAC_DISABLE 0 -#define ADPA_PIPE_SELECT_MASK (1<<30) -#define ADPA_PIPE_A_SELECT 0 -#define ADPA_PIPE_B_SELECT (1<<30) -#define ADPA_PIPE_SELECT(pipe) ((pipe) << 30) -/* CPT uses bits 29:30 for pch transcoder select */ +#define ADPA_PIPE_SEL_SHIFT 30 +#define ADPA_PIPE_SEL_MASK (1<<30) +#define ADPA_PIPE_SEL(pipe) ((pipe) << 30) +#define ADPA_PIPE_SEL_SHIFT_CPT 29 +#define ADPA_PIPE_SEL_MASK_CPT (3<<29) +#define ADPA_PIPE_SEL_CPT(pipe) ((pipe) << 29) #define ADPA_CRT_HOTPLUG_MASK 0x03ff0000 /* bit 25-16 */ #define ADPA_CRT_HOTPLUG_MONITOR_NONE (0<<24) #define ADPA_CRT_HOTPLUG_MONITOR_MASK (3<<24) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index de0e22322c76..211d601cd1b1 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -63,33 +63,35 @@ static struct intel_crt *intel_attached_crt(struct drm_connector *connector) return intel_encoder_to_crt(intel_attached_encoder(connector)); } +bool intel_crt_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t adpa_reg, enum pipe *pipe) +{ + u32 val; + + val = I915_READ(adpa_reg); + + /* asserts want to know the pipe even if the port is disabled */ + if (HAS_PCH_CPT(dev_priv)) + *pipe = (val & ADPA_PIPE_SEL_MASK_CPT) >> ADPA_PIPE_SEL_SHIFT_CPT; + else + *pipe = (val & ADPA_PIPE_SEL_MASK) >> ADPA_PIPE_SEL_SHIFT; + + return val & ADPA_DAC_ENABLE; +} + static bool intel_crt_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crt *crt = intel_encoder_to_crt(encoder); - u32 tmp; bool ret; if (!intel_display_power_get_if_enabled(dev_priv, encoder->power_domain)) return false; - ret = false; - - tmp = I915_READ(crt->adpa_reg); - - if (!(tmp & ADPA_DAC_ENABLE)) - goto out; + ret = intel_crt_port_enabled(dev_priv, crt->adpa_reg, pipe); - if (HAS_PCH_CPT(dev_priv)) - *pipe = PORT_TO_PIPE_CPT(tmp); - else - *pipe = PORT_TO_PIPE(tmp); - - ret = true; -out: intel_display_power_put(dev_priv, encoder->power_domain); return ret; @@ -168,11 +170,9 @@ static void intel_crt_set_dpms(struct intel_encoder *encoder, if (HAS_PCH_LPT(dev_priv)) ; /* Those bits don't exist here */ else if (HAS_PCH_CPT(dev_priv)) - adpa |= PORT_TRANS_SEL_CPT(crtc->pipe); - else if (crtc->pipe == 0) - adpa |= ADPA_PIPE_A_SELECT; + adpa |= ADPA_PIPE_SEL_CPT(crtc->pipe); else - adpa |= ADPA_PIPE_B_SELECT; + adpa |= ADPA_PIPE_SEL(crtc->pipe); if (!HAS_PCH_SPLIT(dev_priv)) I915_WRITE(BCLRPAT(crtc->pipe), 0); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ad588d564198..6daa8d97a0aa 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1360,21 +1360,6 @@ static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv, return true; } -static bool adpa_pipe_enabled(struct drm_i915_private *dev_priv, - enum pipe pipe, u32 val) -{ - if ((val & ADPA_DAC_ENABLE) == 0) - return false; - if (HAS_PCH_CPT(dev_priv)) { - if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe)) - return false; - } else { - if ((val & ADPA_PIPE_SELECT_MASK) != ADPA_PIPE_SELECT(pipe)) - return false; - } - return true; -} - static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, enum pipe pipe, i915_reg_t reg, u32 port_sel) @@ -1405,16 +1390,17 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, enum pipe pipe) { + enum pipe port_pipe; u32 val; assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B); assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C); assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D); - val = I915_READ(PCH_ADPA); - I915_STATE_WARN(adpa_pipe_enabled(dev_priv, pipe, val), - "PCH VGA enabled on transcoder %c, should be disabled\n", - pipe_name(pipe)); + I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) && + port_pipe == pipe, + "PCH VGA enabled on transcoder %c, should be disabled\n", + pipe_name(pipe)); val = I915_READ(PCH_LVDS); I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val), diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d7dbca1aabff..423795050970 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1377,6 +1377,8 @@ void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv); void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv); /* intel_crt.c */ +bool intel_crt_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t adpa_reg, enum pipe *pipe); void intel_crt_init(struct drm_i915_private *dev_priv); void intel_crt_reset(struct drm_encoder *encoder); -- cgit v1.2.3-58-ga151 From a44628b9293b81b53b00adbf90b3dd6e3b9dbc32 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Mon, 14 May 2018 21:28:27 +0300 Subject: drm/i915: Clean up LVDS pipe select bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean up the LVDS pipe select bits. To make the whole situation a bit less ugly we'll start to share the same code between .get_hw_state() and the port state asserts. v2: Order the defines shift,mask,value (Jani) Drop ruperfluous braces and whitesapce changes (Jani) Combine masks in compute_is_dual_link_lvds() (Jani) v3: Fix LVDS_PIPE_SEL_MASK_CPT Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180514182827.28629-1-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 9 ++++--- drivers/gpu/drm/i915/intel_display.c | 33 +++++------------------ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_lvds.c | 52 +++++++++++++++++++----------------- 4 files changed, 42 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ae3c26216996..753b8f110e2e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4392,9 +4392,12 @@ enum { */ #define LVDS_PORT_EN (1 << 31) /* Selects pipe B for LVDS data. Must be set on pre-965. */ -#define LVDS_PIPEB_SELECT (1 << 30) -#define LVDS_PIPE_MASK (1 << 30) -#define LVDS_PIPE(pipe) ((pipe) << 30) +#define LVDS_PIPE_SEL_SHIFT 30 +#define LVDS_PIPE_SEL_MASK (1 << 30) +#define LVDS_PIPE_SEL(pipe) ((pipe) << 30) +#define LVDS_PIPE_SEL_SHIFT_CPT 29 +#define LVDS_PIPE_SEL_MASK_CPT (3 << 29) +#define LVDS_PIPE_SEL_CPT(pipe) ((pipe) << 29) /* LVDS dithering flag on 965/g4x platform */ #define LVDS_ENABLE_DITHER (1 << 25) /* LVDS sync polarity flags. Set to invert (i.e. negative) */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6daa8d97a0aa..a1c6cc75f49c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1214,9 +1214,8 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe) pp_reg = PP_CONTROL(0); port_sel = I915_READ(PP_ON_DELAYS(0)) & PANEL_PORT_SELECT_MASK; - if (port_sel == PANEL_PORT_SELECT_LVDS && - I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT) - panel_pipe = PIPE_B; + if (port_sel == PANEL_PORT_SELECT_LVDS) + intel_lvds_port_enabled(dev_priv, PCH_LVDS, &panel_pipe); /* XXX: else fix for eDP */ } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { /* presumably write lock depends on pipe, not port select */ @@ -1224,8 +1223,7 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe) panel_pipe = pipe; } else { pp_reg = PP_CONTROL(0); - if (I915_READ(LVDS) & LVDS_PIPEB_SELECT) - panel_pipe = PIPE_B; + intel_lvds_port_enabled(dev_priv, LVDS, &panel_pipe); } val = I915_READ(pp_reg); @@ -1344,22 +1342,6 @@ static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv, return true; } -static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv, - enum pipe pipe, u32 val) -{ - if ((val & LVDS_PORT_EN) == 0) - return false; - - if (HAS_PCH_CPT(dev_priv)) { - if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe)) - return false; - } else { - if ((val & LVDS_PIPE_MASK) != LVDS_PIPE(pipe)) - return false; - } - return true; -} - static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, enum pipe pipe, i915_reg_t reg, u32 port_sel) @@ -1391,7 +1373,6 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, enum pipe pipe) { enum pipe port_pipe; - u32 val; assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B); assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C); @@ -1402,10 +1383,10 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, "PCH VGA enabled on transcoder %c, should be disabled\n", pipe_name(pipe)); - val = I915_READ(PCH_LVDS); - I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val), - "PCH LVDS enabled on transcoder %c, should be disabled\n", - pipe_name(pipe)); + I915_STATE_WARN(intel_lvds_port_enabled(dev_priv, PCH_LVDS, &port_pipe) && + port_pipe == pipe, + "PCH LVDS enabled on transcoder %c, should be disabled\n", + pipe_name(pipe)); assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB); assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 423795050970..911d4960ba7a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1824,6 +1824,8 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port); /* intel_lvds.c */ +bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t lvds_reg, enum pipe *pipe); void intel_lvds_init(struct drm_i915_private *dev_priv); struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev); bool intel_is_dual_link_lvds(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 8691c86f579c..cc4cc8669af3 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -85,34 +85,35 @@ static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *conn return container_of(connector, struct intel_lvds_connector, base.base); } +bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t lvds_reg, enum pipe *pipe) +{ + u32 val; + + val = I915_READ(lvds_reg); + + /* asserts want to know the pipe even if the port is disabled */ + if (HAS_PCH_CPT(dev_priv)) + *pipe = (val & LVDS_PIPE_SEL_MASK_CPT) >> LVDS_PIPE_SEL_SHIFT_CPT; + else + *pipe = (val & LVDS_PIPE_SEL_MASK) >> LVDS_PIPE_SEL_SHIFT; + + return val & LVDS_PORT_EN; +} + static bool intel_lvds_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base); - u32 tmp; bool ret; if (!intel_display_power_get_if_enabled(dev_priv, encoder->power_domain)) return false; - ret = false; + ret = intel_lvds_port_enabled(dev_priv, lvds_encoder->reg, pipe); - tmp = I915_READ(lvds_encoder->reg); - - if (!(tmp & LVDS_PORT_EN)) - goto out; - - if (HAS_PCH_CPT(dev_priv)) - *pipe = PORT_TO_PIPE_CPT(tmp); - else - *pipe = PORT_TO_PIPE(tmp); - - ret = true; - -out: intel_display_power_put(dev_priv, encoder->power_domain); return ret; @@ -255,14 +256,11 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder, temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP; if (HAS_PCH_CPT(dev_priv)) { - temp &= ~PORT_TRANS_SEL_MASK; - temp |= PORT_TRANS_SEL_CPT(pipe); + temp &= ~LVDS_PIPE_SEL_MASK_CPT; + temp |= LVDS_PIPE_SEL_CPT(pipe); } else { - if (pipe == 1) { - temp |= LVDS_PIPEB_SELECT; - } else { - temp &= ~LVDS_PIPEB_SELECT; - } + temp &= ~LVDS_PIPE_SEL_MASK; + temp |= LVDS_PIPE_SEL(pipe); } /* set the corresponsding LVDS_BORDER bit */ @@ -908,7 +906,11 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) * register is uninitialized. */ val = I915_READ(lvds_encoder->reg); - if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED))) + if (HAS_PCH_CPT(dev_priv)) + val &= ~(LVDS_DETECTED | LVDS_PIPE_SEL_MASK_CPT); + else + val &= ~(LVDS_DETECTED | LVDS_PIPE_SEL_MASK); + if (val == 0) val = dev_priv->vbt.bios_lvds_val; return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP; -- cgit v1.2.3-58-ga151 From 762034675ee7476180eb3e7fc6c7b52db71654ed Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Mon, 14 May 2018 20:24:21 +0300 Subject: drm/i915: Clean up SDVO pipe select bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean up the SDVO pipe select bits. To make the whole situation a bit less ugly we'll start to share the same code between .get_hw_state() and the port state asserts. v2: Order the defines shift,mask,value (Jani) Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180514172423.9302-3-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 10 ++++---- drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++----------------------- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_hdmi.c | 25 ++++---------------- drivers/gpu/drm/i915/intel_sdvo.c | 38 ++++++++++++++++++----------- 5 files changed, 52 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 753b8f110e2e..f1c83233a70a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4297,9 +4297,9 @@ enum { /* Gen 3 SDVO bits: */ #define SDVO_ENABLE (1 << 31) -#define SDVO_PIPE_SEL(pipe) ((pipe) << 30) +#define SDVO_PIPE_SEL_SHIFT 30 #define SDVO_PIPE_SEL_MASK (1 << 30) -#define SDVO_PIPE_B_SELECT (1 << 30) +#define SDVO_PIPE_SEL(pipe) ((pipe) << 30) #define SDVO_STALL_SELECT (1 << 29) #define SDVO_INTERRUPT_ENABLE (1 << 26) /* @@ -4339,12 +4339,14 @@ enum { #define SDVOB_HOTPLUG_ENABLE (1 << 23) /* SDVO only */ /* Gen 6 (CPT) SDVO/HDMI bits: */ -#define SDVO_PIPE_SEL_CPT(pipe) ((pipe) << 29) +#define SDVO_PIPE_SEL_SHIFT_CPT 29 #define SDVO_PIPE_SEL_MASK_CPT (3 << 29) +#define SDVO_PIPE_SEL_CPT(pipe) ((pipe) << 29) /* CHV SDVO/HDMI bits: */ -#define SDVO_PIPE_SEL_CHV(pipe) ((pipe) << 24) +#define SDVO_PIPE_SEL_SHIFT_CHV 24 #define SDVO_PIPE_SEL_MASK_CHV (3 << 24) +#define SDVO_PIPE_SEL_CHV(pipe) ((pipe) << 24) /* DVO port control */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a1c6cc75f49c..817d5d3f65eb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1323,25 +1323,6 @@ static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, return true; } -static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv, - enum pipe pipe, u32 val) -{ - if ((val & SDVO_ENABLE) == 0) - return false; - - if (HAS_PCH_CPT(dev_priv)) { - if ((val & SDVO_PIPE_SEL_MASK_CPT) != SDVO_PIPE_SEL_CPT(pipe)) - return false; - } else if (IS_CHERRYVIEW(dev_priv)) { - if ((val & SDVO_PIPE_SEL_MASK_CHV) != SDVO_PIPE_SEL_CHV(pipe)) - return false; - } else { - if ((val & SDVO_PIPE_SEL_MASK) != SDVO_PIPE_SEL(pipe)) - return false; - } - return true; -} - static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, enum pipe pipe, i915_reg_t reg, u32 port_sel) @@ -1357,16 +1338,21 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, } static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, - enum pipe pipe, i915_reg_t reg) + enum pipe pipe, enum port port, + i915_reg_t hdmi_reg) { - u32 val = I915_READ(reg); - I915_STATE_WARN(hdmi_pipe_enabled(dev_priv, pipe, val), - "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n", - i915_mmio_reg_offset(reg), pipe_name(pipe)); + enum pipe port_pipe; + bool state; + + state = intel_sdvo_port_enabled(dev_priv, hdmi_reg, &port_pipe); + + I915_STATE_WARN(state && port_pipe == pipe, + "PCH HDMI %c enabled on transcoder %c, should be disabled\n", + port_name(port), pipe_name(pipe)); - I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & SDVO_ENABLE) == 0 - && (val & SDVO_PIPE_B_SELECT), - "IBX PCH hdmi port still using transcoder B\n"); + I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B, + "IBX PCH HDMI %c still using transcoder B\n", + port_name(port)); } static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, @@ -1388,9 +1374,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, "PCH LVDS enabled on transcoder %c, should be disabled\n", pipe_name(pipe)); - assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB); - assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC); - assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); + assert_pch_hdmi_disabled(dev_priv, pipe, PORT_B, PCH_HDMIB); + assert_pch_hdmi_disabled(dev_priv, pipe, PORT_C, PCH_HDMIC); + assert_pch_hdmi_disabled(dev_priv, pipe, PORT_D, PCH_HDMID); } static void _vlv_enable_pll(struct intel_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 911d4960ba7a..12002fc77235 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2064,6 +2064,8 @@ void intel_init_ipc(struct drm_i915_private *dev_priv); void intel_enable_ipc(struct drm_i915_private *dev_priv); /* intel_sdvo.c */ +bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t sdvo_reg, enum pipe *pipe); bool intel_sdvo_init(struct drm_i915_private *dev_priv, i915_reg_t reg, enum port port); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee929f31f7db..ba5ea61fb7b9 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1161,33 +1161,16 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder, static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); - u32 tmp; bool ret; if (!intel_display_power_get_if_enabled(dev_priv, encoder->power_domain)) return false; - ret = false; - - tmp = I915_READ(intel_hdmi->hdmi_reg); - - if (!(tmp & SDVO_ENABLE)) - goto out; - - if (HAS_PCH_CPT(dev_priv)) - *pipe = PORT_TO_PIPE_CPT(tmp); - else if (IS_CHERRYVIEW(dev_priv)) - *pipe = SDVO_PORT_TO_PIPE_CHV(tmp); - else - *pipe = PORT_TO_PIPE(tmp); - - ret = true; + ret = intel_sdvo_port_enabled(dev_priv, intel_hdmi->hdmi_reg, pipe); -out: intel_display_power_put(dev_priv, encoder->power_domain); return ret; @@ -1421,8 +1404,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder, intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false); intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); - temp &= ~SDVO_PIPE_B_SELECT; - temp |= SDVO_ENABLE; + temp &= ~SDVO_PIPE_SEL_MASK; + temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A); /* * HW workaround, need to write this twice for issue * that may result in first write getting masked. diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 96e213ec202d..848b03a542d6 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1403,27 +1403,37 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector) return false; } +bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv, + i915_reg_t sdvo_reg, enum pipe *pipe) +{ + u32 val; + + val = I915_READ(sdvo_reg); + + /* asserts want to know the pipe even if the port is disabled */ + if (HAS_PCH_CPT(dev_priv)) + *pipe = (val & SDVO_PIPE_SEL_MASK_CPT) >> SDVO_PIPE_SEL_SHIFT_CPT; + else if (IS_CHERRYVIEW(dev_priv)) + *pipe = (val & SDVO_PIPE_SEL_MASK_CHV) >> SDVO_PIPE_SEL_SHIFT_CHV; + else + *pipe = (val & SDVO_PIPE_SEL_MASK) >> SDVO_PIPE_SEL_SHIFT; + + return val & SDVO_ENABLE; +} + static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_sdvo *intel_sdvo = to_sdvo(encoder); u16 active_outputs = 0; - u32 tmp; + bool ret; - tmp = I915_READ(intel_sdvo->sdvo_reg); intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs); - if (!(tmp & SDVO_ENABLE) && (active_outputs == 0)) - return false; - - if (HAS_PCH_CPT(dev_priv)) - *pipe = PORT_TO_PIPE_CPT(tmp); - else - *pipe = PORT_TO_PIPE(tmp); + ret = intel_sdvo_port_enabled(dev_priv, intel_sdvo->sdvo_reg, pipe); - return true; + return ret || active_outputs; } static void intel_sdvo_get_config(struct intel_encoder *encoder, @@ -1550,8 +1560,8 @@ static void intel_disable_sdvo(struct intel_encoder *encoder, intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false); intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); - temp &= ~SDVO_PIPE_B_SELECT; - temp |= SDVO_ENABLE; + temp &= ~SDVO_PIPE_SEL_MASK; + temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A); intel_sdvo_write_sdvox(intel_sdvo, temp); temp &= ~SDVO_ENABLE; -- cgit v1.2.3-58-ga151 From 4add0f6bde0506677a224e761cc9f764d24dbe96 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Mon, 14 May 2018 20:24:22 +0300 Subject: drm/i915: Clean up TV pipe select bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parametrize the TV pipe select bits. For consistency with the new way of doing things, let's read out the pipe select bits even when the port is disable, even though we don't need that behaviour for asserts in this case. v2: Order the defines shift,mask,value (Jani) Clear the stale pipe select bit in load detection (Jani) Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180514172423.9302-4-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 4 +++- drivers/gpu/drm/i915/intel_tv.c | 20 ++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f1c83233a70a..dae213d4c13a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4696,7 +4696,9 @@ enum { /* Enables the TV encoder */ # define TV_ENC_ENABLE (1 << 31) /* Sources the TV encoder input from pipe B instead of A. */ -# define TV_ENC_PIPEB_SELECT (1 << 30) +# define TV_ENC_PIPE_SEL_SHIFT 30 +# define TV_ENC_PIPE_SEL_MASK (1 << 30) +# define TV_ENC_PIPE_SEL(pipe) ((pipe) << 30) /* Outputs composite video (DAC A only) */ # define TV_ENC_OUTPUT_COMPOSITE (0 << 28) /* Outputs SVideo video (DAC B/C) */ diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 885fc3809f7f..99bc2368dda0 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -798,16 +798,12 @@ static struct intel_tv *intel_attached_tv(struct drm_connector *connector) static bool intel_tv_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); u32 tmp = I915_READ(TV_CTL); - if (!(tmp & TV_ENC_ENABLE)) - return false; + *pipe = (tmp & TV_ENC_PIPE_SEL_MASK) >> TV_ENC_PIPE_SEL_SHIFT; - *pipe = PORT_TO_PIPE(tmp); - - return true; + return tmp & TV_ENC_ENABLE; } static void @@ -1024,8 +1020,7 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, break; } - if (intel_crtc->pipe == 1) - tv_ctl |= TV_ENC_PIPEB_SELECT; + tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe); tv_ctl |= tv_mode->oversample; if (tv_mode->progressive) @@ -1149,12 +1144,9 @@ intel_tv_detect_type(struct intel_tv *intel_tv, save_tv_ctl = tv_ctl = I915_READ(TV_CTL); /* Poll for TV detection */ - tv_ctl &= ~(TV_ENC_ENABLE | TV_TEST_MODE_MASK); + tv_ctl &= ~(TV_ENC_ENABLE | TV_ENC_PIPE_SEL_MASK | TV_TEST_MODE_MASK); tv_ctl |= TV_TEST_MODE_MONITOR_DETECT; - if (intel_crtc->pipe == 1) - tv_ctl |= TV_ENC_PIPEB_SELECT; - else - tv_ctl &= ~TV_ENC_PIPEB_SELECT; + tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe); tv_dac &= ~(TVDAC_SENSE_MASK | DAC_A_MASK | DAC_B_MASK | DAC_C_MASK); tv_dac |= (TVDAC_STATE_CHG_EN | -- cgit v1.2.3-58-ga151 From b45a258897a4a37b6cbc0bf1dc21354432f3e8c1 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Mon, 14 May 2018 20:24:23 +0300 Subject: drm/i915: Clean up DVO pipe select bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parametrize the DVO pipe select bits. For consistency with the new way of doing things, let's read out the pipe select bits even when the port is disable, even though we don't need that behaviour for asserts in this case. v2: Order the defines shift,mask,value (Jani) Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180514172423.9302-5-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 4 +++- drivers/gpu/drm/i915/intel_dvo.c | 13 ++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index dae213d4c13a..196a0eb79272 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4357,7 +4357,9 @@ enum { #define _DVOC 0x61160 #define DVOC _MMIO(_DVOC) #define DVO_ENABLE (1 << 31) -#define DVO_PIPE_B_SELECT (1 << 30) +#define DVO_PIPE_SEL_SHIFT 30 +#define DVO_PIPE_SEL_MASK (1 << 30) +#define DVO_PIPE_SEL(pipe) ((pipe) << 30) #define DVO_PIPE_STALL_UNUSED (0 << 28) #define DVO_PIPE_STALL (1 << 28) #define DVO_PIPE_STALL_TV (2 << 28) diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c index eb0c559b2715..a86f0398570f 100644 --- a/drivers/gpu/drm/i915/intel_dvo.c +++ b/drivers/gpu/drm/i915/intel_dvo.c @@ -137,19 +137,15 @@ static bool intel_dvo_connector_get_hw_state(struct intel_connector *connector) static bool intel_dvo_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_dvo *intel_dvo = enc_to_dvo(encoder); u32 tmp; tmp = I915_READ(intel_dvo->dev.dvo_reg); - if (!(tmp & DVO_ENABLE)) - return false; - - *pipe = PORT_TO_PIPE(tmp); + *pipe = (tmp & DVO_PIPE_SEL_MASK) >> DVO_PIPE_SEL_SHIFT; - return true; + return tmp & DVO_ENABLE; } static void intel_dvo_get_config(struct intel_encoder *encoder, @@ -276,8 +272,7 @@ static void intel_dvo_pre_enable(struct intel_encoder *encoder, dvo_val |= DVO_DATA_ORDER_FP | DVO_BORDER_ENABLE | DVO_BLANK_ACTIVE_HIGH; - if (pipe == 1) - dvo_val |= DVO_PIPE_B_SELECT; + dvo_val |= DVO_PIPE_SEL(pipe); dvo_val |= DVO_PIPE_STALL; if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) dvo_val |= DVO_HSYNC_ACTIVE_HIGH; -- cgit v1.2.3-58-ga151 From 57877b70739a5d49d95bedf94218ba125e8afef3 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 12:56:47 +0100 Subject: drm/i915/execlists: HWACK checking superseded checking port[0].count The HWACK bit more generically solves the problem of resubmitting ESLP while the hardware is still processing the current ELSP write. We no longer need to check port[0].count itself. References: ba74cb10c775 ("drm/i915/execlists: Delay writing to ELSP until HW has processed the previous write") Signed-off-by: Chris Wilson Cc: Michel Thierry Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180517115647.17205-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_lrc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 493e6bd9e28a..ae5adad7cc63 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -612,8 +612,6 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); GEM_BUG_ON(!port_count(&port[0])); - if (port_count(&port[0]) > 1) - return false; /* * If we write to ELSP a second time before the HW has had -- cgit v1.2.3-58-ga151 From 96d4f03c20d04c80026b1ec3643c090cf4f0eb20 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 16:28:24 +0100 Subject: drm/i915: Nul-terminate legacy debug string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that when we don't have any scheduler attributes for the request, the string is terminated. Fixes: 247870ac8ea7 ("drm/i915: Build request info on stack before printk") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180517152824.11619-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index d4e159ae65a6..e78c6e769e8c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1143,7 +1143,7 @@ static void print_request(struct drm_printer *m, const char *prefix) { const char *name = rq->fence.ops->get_timeline_name(&rq->fence); - char buf[80]; + char buf[80] = ""; int x = 0; x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf)); -- cgit v1.2.3-58-ga151 From 560f6ad8edeaab1b14f1cb86083c7e0c1d37c749 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 16:07:27 +0100 Subject: drm/i915: Remove unused enable_cmd_parser modparam The command parser is feature complete, stable and required by userspace. In commit 41736a8e3331 ("drm/i915: Use the precomputed value for whether to enable command parsing") I accidentally removed control from the modparam, and as no one has complained, remove the left over modparam completely! References: 41736a8e3331 ("drm/i915: Use the precomputed value for whether to enable command parsing") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Jani Nikula Acked-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/20180517150727.10431-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_params.c | 3 --- drivers/gpu/drm/i915/i915_params.h | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 66ea3552c63e..49fcc4679db6 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -130,9 +130,6 @@ i915_param_named_unsafe(invert_brightness, int, 0600, i915_param_named(disable_display, bool, 0400, "Disable display (default: false)"); -i915_param_named_unsafe(enable_cmd_parser, bool, 0400, - "Enable command parsing (true=enabled [default], false=disabled)"); - i915_param_named(mmio_debug, int, 0600, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance."); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 6684025b7af8..aebe0469ddaa 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -58,7 +58,6 @@ struct drm_printer; param(unsigned int, inject_load_failure, 0) \ /* leave bools at the end to not create holes */ \ param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \ - param(bool, enable_cmd_parser, true) \ param(bool, enable_hangcheck, true) \ param(bool, fastboot, false) \ param(bool, prefault_disable, false) \ -- cgit v1.2.3-58-ga151 From 6b7a6a7b4ba12715fabde30fea96f9df3df2da93 Mon Sep 17 00:00:00 2001 From: Oscar Mateo Date: Thu, 10 May 2018 14:59:55 -0700 Subject: drm/i915/icl: Read the correct Gen11 interrupt registers Stop reading some now deprecated interrupt registers in both debugfs and error state. Instead, read the new equivalents in the Gen11 interrupt repartitioning scheme. Note that the equivalent to the PM ISR & IIR cannot be read without affecting the current state of the system, so I've opted for leaving them out. See gen11_reset_one_iir() for more info. v2: else if !!! (Paulo) v3: another else if (Vinay) v4: - Rebased - Renamed patch - Improved the ordering of GENs - Improved the printing of per-GEN info v5: Avoid maybe-unitialized & add comment explaining the lack of PM ISR & IIR Suggested-by: Paulo Zanoni Signed-off-by: Oscar Mateo Cc: Tvrtko Ursulin Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Vinay Belgaumkar Reviewed-by: Vinay Belgaumkar [Paulo: fix commit message and coding style.] Signed-off-by: Paulo Zanoni Link: https://patchwork.freedesktop.org/patch/msgid/1525989595-18220-1-git-send-email-oscar.mateo@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 33 +++++++++++++++++++++++---------- drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++++++++++- drivers/gpu/drm/i915/i915_gpu_error.h | 2 +- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 13e7b9e4a6e6..d78beaabc051 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1162,19 +1162,28 @@ static int i915_frequency_info(struct seq_file *m, void *unused) intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) { - pm_ier = I915_READ(GEN6_PMIER); - pm_imr = I915_READ(GEN6_PMIMR); - pm_isr = I915_READ(GEN6_PMISR); - pm_iir = I915_READ(GEN6_PMIIR); - pm_mask = I915_READ(GEN6_PMINTRMSK); - } else { + if (INTEL_GEN(dev_priv) >= 11) { + pm_ier = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + pm_imr = I915_READ(GEN11_GPM_WGBOXPERF_INTR_MASK); + /* + * The equivalent to the PM ISR & IIR cannot be read + * without affecting the current state of the system + */ + pm_isr = 0; + pm_iir = 0; + } else if (INTEL_GEN(dev_priv) >= 8) { pm_ier = I915_READ(GEN8_GT_IER(2)); pm_imr = I915_READ(GEN8_GT_IMR(2)); pm_isr = I915_READ(GEN8_GT_ISR(2)); pm_iir = I915_READ(GEN8_GT_IIR(2)); - pm_mask = I915_READ(GEN6_PMINTRMSK); + } else { + pm_ier = I915_READ(GEN6_PMIER); + pm_imr = I915_READ(GEN6_PMIMR); + pm_isr = I915_READ(GEN6_PMISR); + pm_iir = I915_READ(GEN6_PMIIR); } + pm_mask = I915_READ(GEN6_PMINTRMSK); + seq_printf(m, "Video Turbo Mode: %s\n", yesno(rpmodectl & GEN6_RP_MEDIA_TURBO)); seq_printf(m, "HW control enabled: %s\n", @@ -1182,8 +1191,12 @@ static int i915_frequency_info(struct seq_file *m, void *unused) seq_printf(m, "SW control enabled: %s\n", yesno((rpmodectl & GEN6_RP_MEDIA_MODE_MASK) == GEN6_RP_MEDIA_SW_MODE)); - seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x IIR=0x%08x, MASK=0x%08x\n", - pm_ier, pm_imr, pm_isr, pm_iir, pm_mask); + + seq_printf(m, "PM IER=0x%08x IMR=0x%08x, MASK=0x%08x\n", + pm_ier, pm_imr, pm_mask); + if (INTEL_GEN(dev_priv) <= 10) + seq_printf(m, "PM ISR=0x%08x IIR=0x%08x\n", + pm_isr, pm_iir); seq_printf(m, "pm_intrmsk_mbz: 0x%08x\n", rps->pm_intrmsk_mbz); seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index df234dc23274..efb808272460 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1667,7 +1667,16 @@ static void capture_reg_state(struct i915_gpu_state *error) } /* 4: Everything else */ - if (INTEL_GEN(dev_priv) >= 8) { + if (INTEL_GEN(dev_priv) >= 11) { + error->ier = I915_READ(GEN8_DE_MISC_IER); + error->gtier[0] = I915_READ(GEN11_RENDER_COPY_INTR_ENABLE); + error->gtier[1] = I915_READ(GEN11_VCS_VECS_INTR_ENABLE); + error->gtier[2] = I915_READ(GEN11_GUC_SG_INTR_ENABLE); + error->gtier[3] = I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE); + error->gtier[4] = I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE); + error->gtier[5] = I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE); + error->ngtier = 6; + } else if (INTEL_GEN(dev_priv) >= 8) { error->ier = I915_READ(GEN8_DE_MISC_IER); for (i = 0; i < 4; i++) error->gtier[i] = I915_READ(GEN8_GT_IER(i)); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h index dac0f8c4c1cf..58910f1dc67c 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.h +++ b/drivers/gpu/drm/i915/i915_gpu_error.h @@ -58,7 +58,7 @@ struct i915_gpu_state { u32 eir; u32 pgtbl_er; u32 ier; - u32 gtier[4], ngtier; + u32 gtier[6], ngtier; u32 ccid; u32 derrmr; u32 forcewake; -- cgit v1.2.3-58-ga151 From c8af5274c3cbacb0905a26bcdef85901216e1134 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Wed, 2 May 2018 14:58:51 -0700 Subject: drm/i915: enable the pipe/transcoder/planes later on HSW+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For all platforms that run haswell_crtc_enable, our spec tells us to configure the transcoder clocks and do link training before it tells us to set pipeconf and the other pipe/transcoder/plane registers. Starting from Icelake, we get machine hangs if we try to touch the pipe/transcoder registers without having the clocks configured and not having some chicken bits set. So this patch changes haswell_crtc_enable() to issue the calls at the appropriate order mandated by the spec. While setting the appropriate chicken bits would also work here, it's better if we actually program the hardware the way it is intended to be programmed. And the chicken bit also has some theoretical downsides that may or may not affect us. Also, correctly programming the hardware does not prevent us from setting the chicken bits in a later patch in case we decide to. v2: Don't forget link training (Ville). Cc: Arthur J Runyan Cc: James Ausmus Cc: Ville Syrjälä Signed-off-by: Paulo Zanoni Reviewed-by: Rodrigo Vivi Reviewed-by: Manasi Navare Link: https://patchwork.freedesktop.org/patch/msgid/20180502215851.30736-1-paulo.r.zanoni@intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 817d5d3f65eb..c9ec88acad9c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5581,6 +5581,11 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, if (INTEL_GEN(dev_priv) >= 11) icl_map_plls_to_ports(crtc, pipe_config, old_state); + intel_encoders_pre_enable(crtc, pipe_config, old_state); + + if (!transcoder_is_dsi(cpu_transcoder)) + intel_ddi_enable_pipe_clock(pipe_config); + if (intel_crtc_has_dp_encoder(intel_crtc->config)) intel_dp_set_m_n(intel_crtc, M1_N1); @@ -5609,11 +5614,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, intel_crtc->active = true; - intel_encoders_pre_enable(crtc, pipe_config, old_state); - - if (!transcoder_is_dsi(cpu_transcoder)) - intel_ddi_enable_pipe_clock(pipe_config); - /* Display WA #1180: WaDisableScalarClockGating: glk, cnl */ psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) && intel_crtc->config->pch_pfit.enabled; -- cgit v1.2.3-58-ga151 From f25a49ab8ab9c1b5587837c8a386b276403f315c Mon Sep 17 00:00:00 2001 From: Colin Xu Date: Sat, 19 May 2018 12:28:54 +0800 Subject: drm/i915/gvt: Use vgpu_lock to protect per vgpu access The patch set splits out 2 small locks from the original big gvt lock: - vgpu_lock protects per-vGPU data and logic, especially the vGPU trap emulation path. - sched_lock protects gvt scheudler structure, context schedule logic and vGPU's schedule data. Use vgpu_lock to replace the gvt big lock. By doing this, the mmio read/write trap path, vgpu virtual event emulation and other vgpu related process, would be protected under per vgpu_lock. v9: - Change commit author since the patches are improved a lot compared with original version. Original author: Pei Zhang - Rebase to latest gvt-staging. v8: - Correct coding and comment style. - Rebase to latest gvt-staging. v7: - Remove gtt_lock since already proteced by gvt_lock and vgpu_lock. - Fix a typo in intel_gvt_deactivate_vgpu, unlock the wrong lock. v6: - Rebase to latest gvt-staging. v5: - Rebase to latest gvt-staging. - intel_vgpu_page_track_handler should use vgpu_lock. v4: - Rebase to latest gvt-staging. - Protect vgpu->active access with vgpu_lock. - Do not wait gpu idle in vgpu_lock. v3: update to latest code base v2: add gvt->lock in function gvt_check_vblank_emulation Performance comparison on Kabylake platform. - Configuration: Host: Ubuntu 16.04. Guest 1 & 2: Ubuntu 16.04. glmark2 score comparison: - Configuration: Host: glxgears. Guests: glmark2. +--------------------------------+-----------------+ | Setup | glmark2 score | +--------------------------------+-----------------+ | unified lock, iommu=on | 58~62 (avg. 60) | +--------------------------------+-----------------+ | unified lock, iommu=igfx_off | 57~61 (avg. 59) | +--------------------------------+-----------------+ | per-logic lock, iommu=on | 60~68 (avg. 64) | +--------------------------------+-----------------+ | per-logic lock, iommu=igfx_off | 61~67 (avg. 64) | +--------------------------------+-----------------+ lock_stat comparison: - Configuration: Stop lock stat immediately after boot up. Boot 2 VM Guests. Run glmark2 in guests. Start perf lock_stat for 20 seconds and stop again. - Legend: c - contentions; w - waittime-avg +------------+-----------------+-----------+---------------+------------+ | | gvt_lock |sched_lock | vgpu_lock | gtt_lock | + lock type; +-----------------+-----------+---------------+------------+ | iommu set | c | w | c | w | c | w | c | w | +------------+-------+---------+----+------+------+--------+-----+------+ | unified; | 20697 | 839 |N/A | N/A | N/A | N/A | N/A | N/A | | on | | | | | | | | | +------------+-------+---------+----+------+------+--------+-----+------+ | unified; | 21838 | 658.15 |N/A | N/A | N/A | N/A | N/A | N/A | | igfx_off | | | | | | | | | +------------+-------+---------+----+------+------+--------+-----+------+ | per-logic; | 1553 | 1599.96 |9458|429.97| 5846 | 274.33 | 0 | 0.00 | | on | | | | | | | | | +------------+-------+---------+----+------+------+--------+-----+------+ | per-logic; | 1911 | 1678.32 |8335|445.16| 5451 | 244.80 | 0 | 0.00 | | igfx_off | | | | | | | | | +------------+-------+---------+----+------+------+--------+-----+------+ Signed-off-by: Pei Zhang Signed-off-by: Colin Xu Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.c | 35 ++++++++++++----------- drivers/gpu/drm/i915/gvt/gvt.c | 5 +--- drivers/gpu/drm/i915/gvt/gvt.h | 8 ++++++ drivers/gpu/drm/i915/gvt/handlers.c | 4 +++ drivers/gpu/drm/i915/gvt/mmio.c | 12 ++++---- drivers/gpu/drm/i915/gvt/page_track.c | 5 ++-- drivers/gpu/drm/i915/gvt/scheduler.c | 9 +++--- drivers/gpu/drm/i915/gvt/vgpu.c | 53 ++++++++++++++++++----------------- 8 files changed, 72 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index 6d8180e8d1e2..8e4a63c5b7d1 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -337,26 +337,28 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt) struct intel_gvt_irq *irq = &gvt->irq; struct intel_vgpu *vgpu; int pipe, id; + int found = false; - if (WARN_ON(!mutex_is_locked(&gvt->lock))) - return; - + mutex_lock(&gvt->lock); for_each_active_vgpu(gvt, vgpu, id) { for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) { - if (pipe_is_enabled(vgpu, pipe)) - goto out; + if (pipe_is_enabled(vgpu, pipe)) { + found = true; + break; + } } + if (found) + break; } /* all the pipes are disabled */ - hrtimer_cancel(&irq->vblank_timer.timer); - return; - -out: - hrtimer_start(&irq->vblank_timer.timer, - ktime_add_ns(ktime_get(), irq->vblank_timer.period), - HRTIMER_MODE_ABS); - + if (!found) + hrtimer_cancel(&irq->vblank_timer.timer); + else + hrtimer_start(&irq->vblank_timer.timer, + ktime_add_ns(ktime_get(), irq->vblank_timer.period), + HRTIMER_MODE_ABS); + mutex_unlock(&gvt->lock); } static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe) @@ -393,8 +395,10 @@ static void emulate_vblank(struct intel_vgpu *vgpu) { int pipe; + mutex_lock(&vgpu->vgpu_lock); for_each_pipe(vgpu->gvt->dev_priv, pipe) emulate_vblank_on_pipe(vgpu, pipe); + mutex_unlock(&vgpu->vgpu_lock); } /** @@ -409,11 +413,10 @@ void intel_gvt_emulate_vblank(struct intel_gvt *gvt) struct intel_vgpu *vgpu; int id; - if (WARN_ON(!mutex_is_locked(&gvt->lock))) - return; - + mutex_lock(&gvt->lock); for_each_active_vgpu(gvt, vgpu, id) emulate_vblank(vgpu); + mutex_unlock(&gvt->lock); } /** diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index 61bd14fcb649..769e06c3ae23 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.c +++ b/drivers/gpu/drm/i915/gvt/gvt.c @@ -271,11 +271,8 @@ static int gvt_service_thread(void *data) continue; if (test_and_clear_bit(INTEL_GVT_REQUEST_EMULATE_VBLANK, - (void *)&gvt->service_request)) { - mutex_lock(&gvt->lock); + (void *)&gvt->service_request)) intel_gvt_emulate_vblank(gvt); - mutex_unlock(&gvt->lock); - } if (test_bit(INTEL_GVT_REQUEST_SCHED, (void *)&gvt->service_request) || diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 05d15a095310..3600553c8b56 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -170,6 +170,7 @@ struct intel_vgpu_submission { struct intel_vgpu { struct intel_gvt *gvt; + struct mutex vgpu_lock; int id; unsigned long handle; /* vGPU handle used by hypervisor MPT modules */ bool active; @@ -294,6 +295,9 @@ struct intel_vgpu_type { }; struct intel_gvt { + /* GVT scope lock, protect GVT itself, and all resource currently + * not yet protected by special locks(vgpu and scheduler lock). + */ struct mutex lock; struct drm_i915_private *dev_priv; struct idr vgpu_idr; /* vGPU IDR pool */ @@ -314,6 +318,10 @@ struct intel_gvt { struct task_struct *service_thread; wait_queue_head_t service_thread_wq; + + /* service_request is always used in bit operation, we should always + * use it with atomic bit ops so that no need to use gvt big lock. + */ unsigned long service_request; struct { diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index d5e206661048..d60c2bee00fb 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -316,6 +316,7 @@ static int gdrst_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, } } + /* vgpu_lock already hold by emulate mmio r/w */ intel_gvt_reset_vgpu_locked(vgpu, false, engine_mask); /* sw will wait for the device to ack the reset request */ @@ -420,7 +421,10 @@ static int pipeconf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, vgpu_vreg(vgpu, offset) |= I965_PIPECONF_ACTIVE; else vgpu_vreg(vgpu, offset) &= ~I965_PIPECONF_ACTIVE; + /* vgpu_lock already hold by emulate mmio r/w */ + mutex_unlock(&vgpu->vgpu_lock); intel_gvt_check_vblank_emulation(vgpu->gvt); + mutex_lock(&vgpu->vgpu_lock); return 0; } diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c index e4960aff68bd..2be1be2cf49a 100644 --- a/drivers/gpu/drm/i915/gvt/mmio.c +++ b/drivers/gpu/drm/i915/gvt/mmio.c @@ -67,7 +67,7 @@ static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa, return; gvt = vgpu->gvt; - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa); if (reg_is_mmio(gvt, offset)) { if (read) @@ -85,7 +85,7 @@ static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa, memcpy(pt, p_data, bytes); } - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); } /** @@ -109,7 +109,7 @@ int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa, failsafe_emulate_mmio_rw(vgpu, pa, p_data, bytes, true); return 0; } - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa); @@ -156,7 +156,7 @@ err: gvt_vgpu_err("fail to emulate MMIO read %08x len %d\n", offset, bytes); out: - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); return ret; } @@ -182,7 +182,7 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa, return 0; } - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa); @@ -220,7 +220,7 @@ err: gvt_vgpu_err("fail to emulate MMIO write %08x len %d\n", offset, bytes); out: - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); return ret; } diff --git a/drivers/gpu/drm/i915/gvt/page_track.c b/drivers/gpu/drm/i915/gvt/page_track.c index 53e2bd79c97d..256d0db8bbb1 100644 --- a/drivers/gpu/drm/i915/gvt/page_track.c +++ b/drivers/gpu/drm/i915/gvt/page_track.c @@ -157,11 +157,10 @@ int intel_vgpu_disable_page_track(struct intel_vgpu *vgpu, unsigned long gfn) int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa, void *data, unsigned int bytes) { - struct intel_gvt *gvt = vgpu->gvt; struct intel_vgpu_page_track *page_track; int ret = 0; - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); page_track = intel_vgpu_find_page_track(vgpu, gpa >> PAGE_SHIFT); if (!page_track) { @@ -179,6 +178,6 @@ int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa, } out: - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); return ret; } diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index c2d183b91500..c0848dcf79c7 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -680,6 +680,7 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n", ring_id, workload); + mutex_lock(&vgpu->vgpu_lock); mutex_lock(&dev_priv->drm.struct_mutex); ret = intel_gvt_scan_and_shadow_workload(workload); @@ -704,6 +705,7 @@ out: } mutex_unlock(&dev_priv->drm.struct_mutex); + mutex_unlock(&vgpu->vgpu_lock); return ret; } @@ -861,14 +863,14 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) int event; mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); /* For the workload w/ request, needs to wait for the context * switch to make sure request is completed. * For the workload w/o request, directly complete the workload. */ if (workload->req) { - struct drm_i915_private *dev_priv = - workload->vgpu->gvt->dev_priv; + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; struct intel_engine_cs *engine = dev_priv->engine[workload->ring_id]; wait_event(workload->shadow_ctx_status_wq, @@ -939,6 +941,7 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) if (gvt->scheduler.need_reschedule) intel_gvt_request_service(gvt, INTEL_GVT_REQUEST_EVENT_SCHED); + mutex_unlock(&vgpu->vgpu_lock); mutex_unlock(&gvt->lock); } @@ -991,9 +994,7 @@ static int workload_thread(void *priv) intel_uncore_forcewake_get(gvt->dev_priv, FORCEWAKE_ALL); - mutex_lock(&gvt->lock); ret = dispatch_workload(workload); - mutex_unlock(&gvt->lock); if (ret) { vgpu = workload->vgpu; diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index bf75300c1ec1..889d10f8ee96 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -226,22 +226,20 @@ void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu) */ void intel_gvt_deactivate_vgpu(struct intel_vgpu *vgpu) { - struct intel_gvt *gvt = vgpu->gvt; - - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); vgpu->active = false; if (atomic_read(&vgpu->submission.running_workload_num)) { - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); intel_gvt_wait_vgpu_idle(vgpu); - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); } intel_vgpu_stop_schedule(vgpu); intel_vgpu_dmabuf_cleanup(vgpu); - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); } /** @@ -255,14 +253,11 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu) { struct intel_gvt *gvt = vgpu->gvt; - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); WARN(vgpu->active, "vGPU is still active!\n"); intel_gvt_debugfs_remove_vgpu(vgpu); - idr_remove(&gvt->vgpu_idr, vgpu->id); - if (idr_is_empty(&gvt->vgpu_idr)) - intel_gvt_clean_irq(gvt); intel_vgpu_clean_sched_policy(vgpu); intel_vgpu_clean_submission(vgpu); intel_vgpu_clean_display(vgpu); @@ -272,10 +267,16 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu) intel_vgpu_free_resource(vgpu); intel_vgpu_clean_mmio(vgpu); intel_vgpu_dmabuf_cleanup(vgpu); - vfree(vgpu); + mutex_unlock(&vgpu->vgpu_lock); + mutex_lock(&gvt->lock); + idr_remove(&gvt->vgpu_idr, vgpu->id); + if (idr_is_empty(&gvt->vgpu_idr)) + intel_gvt_clean_irq(gvt); intel_gvt_update_vgpu_types(gvt); mutex_unlock(&gvt->lock); + + vfree(vgpu); } #define IDLE_VGPU_IDR 0 @@ -301,6 +302,7 @@ struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt) vgpu->id = IDLE_VGPU_IDR; vgpu->gvt = gvt; + mutex_init(&vgpu->vgpu_lock); for (i = 0; i < I915_NUM_ENGINES; i++) INIT_LIST_HEAD(&vgpu->submission.workload_q_head[i]); @@ -327,7 +329,10 @@ out_free_vgpu: */ void intel_gvt_destroy_idle_vgpu(struct intel_vgpu *vgpu) { + mutex_lock(&vgpu->vgpu_lock); intel_vgpu_clean_sched_policy(vgpu); + mutex_unlock(&vgpu->vgpu_lock); + vfree(vgpu); } @@ -345,8 +350,6 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt, if (!vgpu) return ERR_PTR(-ENOMEM); - mutex_lock(&gvt->lock); - ret = idr_alloc(&gvt->vgpu_idr, vgpu, IDLE_VGPU_IDR + 1, GVT_MAX_VGPU, GFP_KERNEL); if (ret < 0) @@ -356,6 +359,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt, vgpu->handle = param->handle; vgpu->gvt = gvt; vgpu->sched_ctl.weight = param->weight; + mutex_init(&vgpu->vgpu_lock); INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head); INIT_RADIX_TREE(&vgpu->page_track_tree, GFP_KERNEL); idr_init(&vgpu->object_idr); @@ -403,8 +407,6 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt, if (ret) goto out_clean_sched_policy; - mutex_unlock(&gvt->lock); - return vgpu; out_clean_sched_policy: @@ -427,7 +429,6 @@ out_clean_idr: idr_remove(&gvt->vgpu_idr, vgpu->id); out_free_vgpu: vfree(vgpu); - mutex_unlock(&gvt->lock); return ERR_PTR(ret); } @@ -459,12 +460,12 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt, param.low_gm_sz = BYTES_TO_MB(param.low_gm_sz); param.high_gm_sz = BYTES_TO_MB(param.high_gm_sz); + mutex_lock(&gvt->lock); vgpu = __intel_gvt_create_vgpu(gvt, ¶m); - if (IS_ERR(vgpu)) - return vgpu; - - /* calculate left instance change for types */ - intel_gvt_update_vgpu_types(gvt); + if (!IS_ERR(vgpu)) + /* calculate left instance change for types */ + intel_gvt_update_vgpu_types(gvt); + mutex_unlock(&gvt->lock); return vgpu; } @@ -476,7 +477,7 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt, * @engine_mask: engines to reset for GT reset * * This function is called when user wants to reset a virtual GPU through - * device model reset or GT reset. The caller should hold the gvt lock. + * device model reset or GT reset. The caller should hold the vgpu lock. * * vGPU Device Model Level Reset (DMLR) simulates the PCI level reset to reset * the whole vGPU to default state as when it is created. This vGPU function @@ -516,9 +517,9 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr, * scheduler when the reset is triggered by current vgpu. */ if (scheduler->current_vgpu == NULL) { - mutex_unlock(&gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); intel_gvt_wait_vgpu_idle(vgpu); - mutex_lock(&gvt->lock); + mutex_lock(&vgpu->vgpu_lock); } intel_vgpu_reset_submission(vgpu, resetting_eng); @@ -558,7 +559,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr, */ void intel_gvt_reset_vgpu(struct intel_vgpu *vgpu) { - mutex_lock(&vgpu->gvt->lock); + mutex_lock(&vgpu->vgpu_lock); intel_gvt_reset_vgpu_locked(vgpu, true, 0); - mutex_unlock(&vgpu->gvt->lock); + mutex_unlock(&vgpu->vgpu_lock); } -- cgit v1.2.3-58-ga151 From 9a512e23f173a3598709b74d6ccf9a6616403967 Mon Sep 17 00:00:00 2001 From: Colin Xu Date: Sat, 19 May 2018 12:28:55 +0800 Subject: drm/i915/gvt: Use sched_lock to protect gvt scheduler logic. The scheduler lock(gvt->sched_lock) is used to protect gvt scheduler logic, including the gvt scheduler structure(gvt->scheduler and per vgpu schedule data(vgpu->sched_data, vgpu->sched_ctl). v9: - Change commit author since the patches are improved a lot compared with original version. Original author: Pei Zhang - Rebase to latest gvt-staging. v8: - Correct coding wqstyle. - Rebase to latest gvt-staging. v7: - Remove gtt_lock since already proteced by gvt_lock and vgpu_lock. v6: - Rebase to latest gvt-staging. v5: - Rebase to latest gvt-staging. v4: - Rebase to latest gvt-staging. v3: update to latest code base Signed-off-by: Pei Zhang Signed-off-by: Colin Xu Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gvt.c | 1 + drivers/gpu/drm/i915/gvt/gvt.h | 8 ++++++++ drivers/gpu/drm/i915/gvt/sched_policy.c | 36 +++++++++++++++++++++++++++++---- drivers/gpu/drm/i915/gvt/scheduler.c | 8 ++++---- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index 769e06c3ae23..22a3ddff38a3 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.c +++ b/drivers/gpu/drm/i915/gvt/gvt.c @@ -376,6 +376,7 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv) idr_init(&gvt->vgpu_idr); spin_lock_init(&gvt->scheduler.mmio_context_lock); mutex_init(&gvt->lock); + mutex_init(&gvt->sched_lock); gvt->dev_priv = dev_priv; init_device_info(gvt); diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 3600553c8b56..362e3d506d2d 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -177,6 +177,11 @@ struct intel_vgpu { bool pv_notified; bool failsafe; unsigned int resetting_eng; + + /* Both sched_data and sched_ctl can be seen a part of the global gvt + * scheduler structure. So below 2 vgpu data are protected + * by sched_lock, not vgpu_lock. + */ void *sched_data; struct vgpu_sched_ctl sched_ctl; @@ -299,6 +304,9 @@ struct intel_gvt { * not yet protected by special locks(vgpu and scheduler lock). */ struct mutex lock; + /* scheduler scope lock, protect gvt and vgpu schedule related data */ + struct mutex sched_lock; + struct drm_i915_private *dev_priv; struct idr vgpu_idr; /* vGPU IDR pool */ diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index d053cbe1dc94..09d7bb72b4ff 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -228,7 +228,7 @@ void intel_gvt_schedule(struct intel_gvt *gvt) struct gvt_sched_data *sched_data = gvt->scheduler.sched_data; ktime_t cur_time; - mutex_lock(&gvt->lock); + mutex_lock(&gvt->sched_lock); cur_time = ktime_get(); if (test_and_clear_bit(INTEL_GVT_REQUEST_SCHED, @@ -244,7 +244,7 @@ void intel_gvt_schedule(struct intel_gvt *gvt) vgpu_update_timeslice(gvt->scheduler.current_vgpu, cur_time); tbs_sched_func(sched_data); - mutex_unlock(&gvt->lock); + mutex_unlock(&gvt->sched_lock); } static enum hrtimer_restart tbs_timer_fn(struct hrtimer *timer_data) @@ -359,39 +359,65 @@ static struct intel_gvt_sched_policy_ops tbs_schedule_ops = { int intel_gvt_init_sched_policy(struct intel_gvt *gvt) { + int ret; + + mutex_lock(&gvt->sched_lock); gvt->scheduler.sched_ops = &tbs_schedule_ops; + ret = gvt->scheduler.sched_ops->init(gvt); + mutex_unlock(&gvt->sched_lock); - return gvt->scheduler.sched_ops->init(gvt); + return ret; } void intel_gvt_clean_sched_policy(struct intel_gvt *gvt) { + mutex_lock(&gvt->sched_lock); gvt->scheduler.sched_ops->clean(gvt); + mutex_unlock(&gvt->sched_lock); } +/* for per-vgpu scheduler policy, there are 2 per-vgpu data: + * sched_data, and sched_ctl. We see these 2 data as part of + * the global scheduler which are proteced by gvt->sched_lock. + * Caller should make their decision if the vgpu_lock should + * be hold outside. + */ + int intel_vgpu_init_sched_policy(struct intel_vgpu *vgpu) { - return vgpu->gvt->scheduler.sched_ops->init_vgpu(vgpu); + int ret; + + mutex_lock(&vgpu->gvt->sched_lock); + ret = vgpu->gvt->scheduler.sched_ops->init_vgpu(vgpu); + mutex_unlock(&vgpu->gvt->sched_lock); + + return ret; } void intel_vgpu_clean_sched_policy(struct intel_vgpu *vgpu) { + mutex_lock(&vgpu->gvt->sched_lock); vgpu->gvt->scheduler.sched_ops->clean_vgpu(vgpu); + mutex_unlock(&vgpu->gvt->sched_lock); } void intel_vgpu_start_schedule(struct intel_vgpu *vgpu) { struct vgpu_sched_data *vgpu_data = vgpu->sched_data; + mutex_lock(&vgpu->gvt->sched_lock); if (!vgpu_data->active) { gvt_dbg_core("vgpu%d: start schedule\n", vgpu->id); vgpu->gvt->scheduler.sched_ops->start_schedule(vgpu); } + mutex_unlock(&vgpu->gvt->sched_lock); } void intel_gvt_kick_schedule(struct intel_gvt *gvt) { + mutex_lock(&gvt->sched_lock); intel_gvt_request_service(gvt, INTEL_GVT_REQUEST_EVENT_SCHED); + mutex_unlock(&gvt->sched_lock); } void intel_vgpu_stop_schedule(struct intel_vgpu *vgpu) @@ -406,6 +432,7 @@ void intel_vgpu_stop_schedule(struct intel_vgpu *vgpu) gvt_dbg_core("vgpu%d: stop schedule\n", vgpu->id); + mutex_lock(&vgpu->gvt->sched_lock); scheduler->sched_ops->stop_schedule(vgpu); if (scheduler->next_vgpu == vgpu) @@ -425,4 +452,5 @@ void intel_vgpu_stop_schedule(struct intel_vgpu *vgpu) } } spin_unlock_bh(&scheduler->mmio_context_lock); + mutex_unlock(&vgpu->gvt->sched_lock); } diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index c0848dcf79c7..847f295e6755 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -715,7 +715,7 @@ static struct intel_vgpu_workload *pick_next_workload( struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler; struct intel_vgpu_workload *workload = NULL; - mutex_lock(&gvt->lock); + mutex_lock(&gvt->sched_lock); /* * no current vgpu / will be scheduled out / no workload @@ -761,7 +761,7 @@ static struct intel_vgpu_workload *pick_next_workload( atomic_inc(&workload->vgpu->submission.running_workload_num); out: - mutex_unlock(&gvt->lock); + mutex_unlock(&gvt->sched_lock); return workload; } @@ -862,8 +862,8 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) struct intel_vgpu_submission *s = &vgpu->submission; int event; - mutex_lock(&gvt->lock); mutex_lock(&vgpu->vgpu_lock); + mutex_lock(&gvt->sched_lock); /* For the workload w/ request, needs to wait for the context * switch to make sure request is completed. @@ -941,8 +941,8 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) if (gvt->scheduler.need_reschedule) intel_gvt_request_service(gvt, INTEL_GVT_REQUEST_EVENT_SCHED); + mutex_unlock(&gvt->sched_lock); mutex_unlock(&vgpu->vgpu_lock); - mutex_unlock(&gvt->lock); } struct workload_thread_param { -- cgit v1.2.3-58-ga151 From 4e0d64dba816adf18c17488d38ede67a3d0e9b40 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 22:26:30 +0100 Subject: drm/i915: Move request->ctx aside In the next patch, we want to store the intel_context pointer inside i915_request, as it is frequently access via a convoluted dance when submitting the request to hw. Having two context pointers inside i915_request leads to confusion so first rename the existing i915_gem_context pointer to i915_request.gem_context. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gvt/scheduler.c | 4 +-- drivers/gpu/drm/i915/i915_debugfs.c | 4 +-- drivers/gpu/drm/i915/i915_gem.c | 10 ++++---- drivers/gpu/drm/i915/i915_gpu_error.c | 18 ++++++++------ drivers/gpu/drm/i915/i915_request.c | 12 ++++----- drivers/gpu/drm/i915/i915_request.h | 2 +- drivers/gpu/drm/i915/i915_trace.h | 10 ++++---- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_guc_submission.c | 7 +++--- drivers/gpu/drm/i915/intel_lrc.c | 31 +++++++++++++----------- drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++----- drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 5 +++- drivers/gpu/drm/i915/selftests/intel_lrc.c | 2 +- 13 files changed, 64 insertions(+), 55 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index c2d183b91500..17f9f8d7e148 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -205,7 +205,7 @@ static int populate_shadow_context(struct intel_vgpu_workload *workload) static inline bool is_gvt_request(struct i915_request *req) { - return i915_gem_context_force_single_submission(req->ctx); + return i915_gem_context_force_single_submission(req->gem_context); } static void save_ring_hw_state(struct intel_vgpu *vgpu, int ring_id) @@ -305,7 +305,7 @@ static int copy_workload_to_ring_buffer(struct intel_vgpu_workload *workload) struct i915_request *req = workload->req; if (IS_KABYLAKE(req->i915) && - is_inhibit_context(req->ctx, req->engine->id)) + is_inhibit_context(req->gem_context, req->engine->id)) intel_vgpu_restore_inhibit_context(vgpu, req); /* allocate shadow ring buffer */ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d78beaabc051..52515445ac40 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -542,8 +542,8 @@ static int i915_gem_object_info(struct seq_file *m, void *data) struct i915_request, client_link); rcu_read_lock(); - task = pid_task(request && request->ctx->pid ? - request->ctx->pid : file->pid, + task = pid_task(request && request->gem_context->pid ? + request->gem_context->pid : file->pid, PIDTYPE_PID); print_file_stats(m, task ? task->comm : "", stats); rcu_read_unlock(); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b0fe452ce17c..a20f8db5729d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3067,7 +3067,7 @@ static void skip_request(struct i915_request *request) static void engine_skip_context(struct i915_request *request) { struct intel_engine_cs *engine = request->engine; - struct i915_gem_context *hung_ctx = request->ctx; + struct i915_gem_context *hung_ctx = request->gem_context; struct i915_timeline *timeline = request->timeline; unsigned long flags; @@ -3077,7 +3077,7 @@ static void engine_skip_context(struct i915_request *request) spin_lock_nested(&timeline->lock, SINGLE_DEPTH_NESTING); list_for_each_entry_continue(request, &engine->timeline.requests, link) - if (request->ctx == hung_ctx) + if (request->gem_context == hung_ctx) skip_request(request); list_for_each_entry(request, &timeline->requests, link) @@ -3123,11 +3123,11 @@ i915_gem_reset_request(struct intel_engine_cs *engine, } if (stalled) { - i915_gem_context_mark_guilty(request->ctx); + i915_gem_context_mark_guilty(request->gem_context); skip_request(request); /* If this context is now banned, skip all pending requests. */ - if (i915_gem_context_is_banned(request->ctx)) + if (i915_gem_context_is_banned(request->gem_context)) engine_skip_context(request); } else { /* @@ -3137,7 +3137,7 @@ i915_gem_reset_request(struct intel_engine_cs *engine, */ request = i915_gem_find_active_request(engine); if (request) { - i915_gem_context_mark_innocent(request->ctx); + i915_gem_context_mark_innocent(request->gem_context); dma_fence_set_error(&request->fence, -EAGAIN); /* Rewind the engine to replay the incomplete rq */ diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index efb808272460..37c9a42654ba 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1287,9 +1287,11 @@ static void error_record_engine_registers(struct i915_gpu_state *error, static void record_request(struct i915_request *request, struct drm_i915_error_request *erq) { - erq->context = request->ctx->hw_id; + struct i915_gem_context *ctx = request->gem_context; + + erq->context = ctx->hw_id; erq->sched_attr = request->sched.attr; - erq->ban_score = atomic_read(&request->ctx->ban_score); + erq->ban_score = atomic_read(&ctx->ban_score); erq->seqno = request->global_seqno; erq->jiffies = request->emitted_jiffies; erq->start = i915_ggtt_offset(request->ring->vma); @@ -1297,7 +1299,7 @@ static void record_request(struct i915_request *request, erq->tail = request->tail; rcu_read_lock(); - erq->pid = request->ctx->pid ? pid_nr(request->ctx->pid) : 0; + erq->pid = ctx->pid ? pid_nr(ctx->pid) : 0; rcu_read_unlock(); } @@ -1461,12 +1463,12 @@ static void gem_record_rings(struct i915_gpu_state *error) request = i915_gem_find_active_request(engine); if (request) { + struct i915_gem_context *ctx = request->gem_context; struct intel_ring *ring; - ee->vm = request->ctx->ppgtt ? - &request->ctx->ppgtt->base : &ggtt->base; + ee->vm = ctx->ppgtt ? &ctx->ppgtt->base : &ggtt->base; - record_context(&ee->context, request->ctx); + record_context(&ee->context, ctx); /* We need to copy these to an anonymous buffer * as the simplest method to avoid being overwritten @@ -1483,11 +1485,11 @@ static void gem_record_rings(struct i915_gpu_state *error) ee->ctx = i915_error_object_create(i915, - to_intel_context(request->ctx, + to_intel_context(ctx, engine)->state); error->simulated |= - i915_gem_context_no_error_capture(request->ctx); + i915_gem_context_no_error_capture(ctx); ee->rq_head = request->head; ee->rq_post = request->postfix; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 8928894dd9c7..fe8810a6a339 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -384,7 +384,7 @@ static void __retire_engine_request(struct intel_engine_cs *engine, */ if (engine->last_retired_context) intel_context_unpin(engine->last_retired_context, engine); - engine->last_retired_context = rq->ctx; + engine->last_retired_context = rq->gem_context; } static void __retire_engine_upto(struct intel_engine_cs *engine, @@ -455,8 +455,8 @@ static void i915_request_retire(struct i915_request *request) i915_request_remove_from_client(request); /* Retirement decays the ban score as it is a sign of ctx progress */ - atomic_dec_if_positive(&request->ctx->ban_score); - intel_context_unpin(request->ctx, request->engine); + atomic_dec_if_positive(&request->gem_context->ban_score); + intel_context_unpin(request->gem_context, request->engine); __retire_engine_upto(request->engine, request); @@ -760,7 +760,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) INIT_LIST_HEAD(&rq->active_list); rq->i915 = i915; rq->engine = engine; - rq->ctx = ctx; + rq->gem_context = ctx; rq->ring = ring; rq->timeline = ring->timeline; GEM_BUG_ON(rq->timeline == &engine->timeline); @@ -814,7 +814,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) goto err_unwind; /* Keep a second pin for the dual retirement along engine and ring */ - __intel_context_pin(rq->ctx, engine); + __intel_context_pin(rq->gem_context, engine); /* Check that we didn't interrupt ourselves with a new request */ GEM_BUG_ON(rq->timeline->seqno != rq->fence.seqno); @@ -1113,7 +1113,7 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) local_bh_disable(); rcu_read_lock(); /* RCU serialisation for set-wedged protection */ if (engine->schedule) - engine->schedule(request, &request->ctx->sched); + engine->schedule(request, &request->gem_context->sched); rcu_read_unlock(); i915_sw_fence_commit(&request->submit); local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h index eddbd4245cb3..dddecd9ffd0c 100644 --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -93,7 +93,7 @@ struct i915_request { * i915_request_free() will then decrement the refcount on the * context. */ - struct i915_gem_context *ctx; + struct i915_gem_context *gem_context; struct intel_engine_cs *engine; struct intel_ring *ring; struct i915_timeline *timeline; diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 8cc3a256f29d..5d4f78765083 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -624,7 +624,7 @@ TRACE_EVENT(i915_request_queue, TP_fast_assign( __entry->dev = rq->i915->drm.primary->index; - __entry->hw_id = rq->ctx->hw_id; + __entry->hw_id = rq->gem_context->hw_id; __entry->ring = rq->engine->id; __entry->ctx = rq->fence.context; __entry->seqno = rq->fence.seqno; @@ -651,7 +651,7 @@ DECLARE_EVENT_CLASS(i915_request, TP_fast_assign( __entry->dev = rq->i915->drm.primary->index; - __entry->hw_id = rq->ctx->hw_id; + __entry->hw_id = rq->gem_context->hw_id; __entry->ring = rq->engine->id; __entry->ctx = rq->fence.context; __entry->seqno = rq->fence.seqno; @@ -696,7 +696,7 @@ TRACE_EVENT(i915_request_in, TP_fast_assign( __entry->dev = rq->i915->drm.primary->index; - __entry->hw_id = rq->ctx->hw_id; + __entry->hw_id = rq->gem_context->hw_id; __entry->ring = rq->engine->id; __entry->ctx = rq->fence.context; __entry->seqno = rq->fence.seqno; @@ -727,7 +727,7 @@ TRACE_EVENT(i915_request_out, TP_fast_assign( __entry->dev = rq->i915->drm.primary->index; - __entry->hw_id = rq->ctx->hw_id; + __entry->hw_id = rq->gem_context->hw_id; __entry->ring = rq->engine->id; __entry->ctx = rq->fence.context; __entry->seqno = rq->fence.seqno; @@ -815,7 +815,7 @@ TRACE_EVENT(i915_request_wait_begin, */ TP_fast_assign( __entry->dev = rq->i915->drm.primary->index; - __entry->hw_id = rq->ctx->hw_id; + __entry->hw_id = rq->gem_context->hw_id; __entry->ring = rq->engine->id; __entry->ctx = rq->fence.context; __entry->seqno = rq->fence.seqno; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index e78c6e769e8c..7983b8a1ad44 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1020,7 +1020,7 @@ bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine) */ rq = __i915_gem_active_peek(&engine->timeline.last_request); if (rq) - return rq->ctx == kernel_context; + return rq->gem_context == kernel_context; else return engine->last_retired_context == kernel_context; } diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 637e852888ec..a432a193f3c4 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -513,8 +513,9 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) { struct intel_guc_client *client = guc->execbuf_client; struct intel_engine_cs *engine = rq->engine; - u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(rq->ctx, - engine)); + u32 ctx_desc = + lower_32_bits(intel_lr_context_descriptor(rq->gem_context, + engine)); u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64); spin_lock(&client->wq_lock); @@ -725,7 +726,7 @@ static bool __guc_dequeue(struct intel_engine_cs *engine) struct i915_request *rq, *rn; list_for_each_entry_safe(rq, rn, &p->requests, sched.link) { - if (last && rq->ctx != last->ctx) { + if (last && rq->gem_context != last->gem_context) { if (port == last_port) { __list_del_many(&p->requests, &rq->sched.link); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ae5adad7cc63..1e9cc55d785c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -418,9 +418,10 @@ execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state) static u64 execlists_update_context(struct i915_request *rq) { - struct intel_context *ce = to_intel_context(rq->ctx, rq->engine); + struct intel_context *ce = + to_intel_context(rq->gem_context, rq->engine); struct i915_hw_ppgtt *ppgtt = - rq->ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt; + rq->gem_context->ppgtt ?: rq->i915->mm.aliasing_ppgtt; u32 *reg_state = ce->lrc_reg_state; reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail); @@ -679,7 +680,8 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) * second request, and so we never need to tell the * hardware about the first. */ - if (last && !can_merge_ctx(rq->ctx, last->ctx)) { + if (last && !can_merge_ctx(rq->gem_context, + last->gem_context)) { /* * If we are on the second port and cannot * combine this request with the last, then we @@ -698,14 +700,14 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) * 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)) { + if (ctx_single_port_submission(last->gem_context) || + ctx_single_port_submission(rq->gem_context)) { __list_del_many(&p->requests, &rq->sched.link); goto done; } - GEM_BUG_ON(last->ctx == rq->ctx); + GEM_BUG_ON(last->gem_context == rq->gem_context); if (submit) port_assign(port, last); @@ -1437,7 +1439,7 @@ static void execlists_context_unpin(struct intel_engine_cs *engine, static int execlists_request_alloc(struct i915_request *request) { struct intel_context *ce = - to_intel_context(request->ctx, request->engine); + to_intel_context(request->gem_context, request->engine); int ret; GEM_BUG_ON(!ce->pin_count); @@ -1954,7 +1956,7 @@ static void execlists_reset(struct intel_engine_cs *engine, * future request will be after userspace has had the opportunity * to recreate its own state. */ - regs = to_intel_context(request->ctx, engine)->lrc_reg_state; + regs = to_intel_context(request->gem_context, engine)->lrc_reg_state; if (engine->default_state) { void *defaults; @@ -1967,7 +1969,8 @@ static void execlists_reset(struct intel_engine_cs *engine, i915_gem_object_unpin_map(engine->default_state); } } - execlists_init_reg_state(regs, request->ctx, engine, request->ring); + execlists_init_reg_state(regs, + request->gem_context, engine, request->ring); /* Move the RING_HEAD onto the breadcrumb, past the hanging batch */ regs[CTX_RING_BUFFER_START + 1] = i915_ggtt_offset(request->ring->vma); @@ -1989,7 +1992,7 @@ static void execlists_reset_finish(struct intel_engine_cs *engine) static int intel_logical_ring_emit_pdps(struct i915_request *rq) { - struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; + struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt; struct intel_engine_cs *engine = rq->engine; const int num_lri_cmds = GEN8_3LVL_PDPES * 2; u32 *cs; @@ -2028,15 +2031,15 @@ static int gen8_emit_bb_start(struct i915_request *rq, * it is unsafe in case of lite-restore (because the ctx is * not idle). PML4 is allocated during ppgtt init so this is * not needed in 48-bit.*/ - if (rq->ctx->ppgtt && - (intel_engine_flag(rq->engine) & rq->ctx->ppgtt->pd_dirty_rings) && - !i915_vm_is_48bit(&rq->ctx->ppgtt->base) && + if (rq->gem_context->ppgtt && + (intel_engine_flag(rq->engine) & rq->gem_context->ppgtt->pd_dirty_rings) && + !i915_vm_is_48bit(&rq->gem_context->ppgtt->base) && !intel_vgpu_active(rq->i915)) { ret = intel_logical_ring_emit_pdps(rq); if (ret) return ret; - rq->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(rq->engine); + rq->gem_context->ppgtt->pd_dirty_rings &= ~intel_engine_flag(rq->engine); } cs = intel_ring_begin(rq, 6); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 6f200a747176..53703012ec75 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -571,8 +571,8 @@ static void reset_ring(struct intel_engine_cs *engine, */ if (request) { struct drm_i915_private *dev_priv = request->i915; - struct intel_context *ce = to_intel_context(request->ctx, - engine); + struct intel_context *ce = + to_intel_context(request->gem_context, engine); struct i915_hw_ppgtt *ppgtt; if (ce->state) { @@ -584,7 +584,7 @@ static void reset_ring(struct intel_engine_cs *engine, CCID_EN); } - ppgtt = request->ctx->ppgtt ?: engine->i915->mm.aliasing_ppgtt; + ppgtt = request->gem_context->ppgtt ?: engine->i915->mm.aliasing_ppgtt; if (ppgtt) { u32 pd_offset = ppgtt->pd.base.ggtt_offset << 10; @@ -1458,7 +1458,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags) *cs++ = MI_NOOP; *cs++ = MI_SET_CONTEXT; - *cs++ = i915_ggtt_offset(to_intel_context(rq->ctx, engine)->state) | flags; + *cs++ = i915_ggtt_offset(to_intel_context(rq->gem_context, engine)->state) | flags; /* * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP * WaMiSetContext_Hang:snb,ivb,vlv @@ -1526,7 +1526,7 @@ static int remap_l3(struct i915_request *rq, int slice) static int switch_context(struct i915_request *rq) { struct intel_engine_cs *engine = rq->engine; - struct i915_gem_context *to_ctx = rq->ctx; + struct i915_gem_context *to_ctx = rq->gem_context; struct i915_hw_ppgtt *to_mm = to_ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt; struct i915_gem_context *from_ctx = engine->legacy_active_context; @@ -1597,7 +1597,7 @@ static int ring_request_alloc(struct i915_request *request) { int ret; - GEM_BUG_ON(!to_intel_context(request->ctx, request->engine)->pin_count); + GEM_BUG_ON(!to_intel_context(request->gem_context, request->engine)->pin_count); /* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c index 438e0b045a2c..2c4e77c050dc 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -105,7 +105,10 @@ static int emit_recurse_batch(struct hang *h, struct i915_request *rq) { struct drm_i915_private *i915 = h->i915; - struct i915_address_space *vm = rq->ctx->ppgtt ? &rq->ctx->ppgtt->base : &i915->ggtt.base; + struct i915_address_space *vm = + rq->gem_context->ppgtt ? + &rq->gem_context->ppgtt->base : + &i915->ggtt.base; struct i915_vma *hws, *vma; unsigned int flags; u32 *batch; diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c index 1b8a07125150..68cb9126b3e1 100644 --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c @@ -83,7 +83,7 @@ static int emit_recurse_batch(struct spinner *spin, struct i915_request *rq, u32 arbitration_command) { - struct i915_address_space *vm = &rq->ctx->ppgtt->base; + struct i915_address_space *vm = &rq->gem_context->ppgtt->base; struct i915_vma *hws, *vma; u32 *batch; int err; -- cgit v1.2.3-58-ga151 From 01278cb143955f4b592568d1756f1baf506245c2 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 22:26:31 +0100 Subject: drm/i915: Move fiddling with engine->last_retired_context Move the knowledge about resetting the current context tracking on the engine from inside i915_gem_context.c into intel_engine_cs.c Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_context.c | 12 ++---------- drivers/gpu/drm/i915/intel_engine_cs.c | 23 +++++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 4bf18b5c6f1d..9e70f4dfa703 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -514,16 +514,8 @@ void i915_gem_contexts_lost(struct drm_i915_private *dev_priv) lockdep_assert_held(&dev_priv->drm.struct_mutex); - for_each_engine(engine, dev_priv, id) { - engine->legacy_active_context = NULL; - engine->legacy_active_ppgtt = NULL; - - if (!engine->last_retired_context) - continue; - - intel_context_unpin(engine->last_retired_context, engine); - engine->last_retired_context = NULL; - } + for_each_engine(engine, dev_priv, id) + intel_engine_lost_context(engine); } void i915_gem_contexts_fini(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 7983b8a1ad44..9e618aab6568 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1096,6 +1096,29 @@ void intel_engines_unpark(struct drm_i915_private *i915) } } +/** + * intel_engine_lost_context: called when the GPU is reset into unknown state + * @engine: the engine + * + * We have either reset the GPU or otherwise about to lose state tracking of + * the current GPU logical state (e.g. suspend). On next use, it is therefore + * imperative that we make no presumptions about the current state and load + * from scratch. + */ +void intel_engine_lost_context(struct intel_engine_cs *engine) +{ + struct i915_gem_context *ctx; + + lockdep_assert_held(&engine->i915->drm.struct_mutex); + + engine->legacy_active_context = NULL; + engine->legacy_active_ppgtt = NULL; + + ctx = fetch_and_zero(&engine->last_retired_context); + if (ctx) + intel_context_unpin(ctx, engine); +} + bool intel_engine_can_store_dword(struct intel_engine_cs *engine) { switch (INTEL_GEN(engine->i915)) { diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 61f385a92484..2b16185e36c4 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -1053,6 +1053,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine); bool intel_engines_are_idle(struct drm_i915_private *dev_priv); bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine); +void intel_engine_lost_context(struct intel_engine_cs *engine); void intel_engines_park(struct drm_i915_private *i915); void intel_engines_unpark(struct drm_i915_private *i915); -- cgit v1.2.3-58-ga151 From 1fc44d9b1afb0afe46acd99bdfdf793805a850e1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 22:26:32 +0100 Subject: drm/i915: Store a pointer to intel_context in i915_request To ease the frequent and ugly pointer dance of &request->gem_context->engine[request->engine->id] during request submission, store that pointer as request->hw_context. One major advantage that we will exploit later is that this decouples the logical context state from the engine itself. v2: Set mock_context->ops so we don't crash and burn in selftests. Cleanups from Tvrtko. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Acked-by: Zhenyu Wang Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-3-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gvt/mmio_context.c | 6 +- drivers/gpu/drm/i915/gvt/mmio_context.h | 2 +- drivers/gpu/drm/i915/gvt/scheduler.c | 141 ++++++++++---------------- drivers/gpu/drm/i915/gvt/scheduler.h | 1 - drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 12 +-- drivers/gpu/drm/i915/i915_gem_context.c | 17 ++-- drivers/gpu/drm/i915/i915_gem_context.h | 21 ++-- drivers/gpu/drm/i915/i915_gpu_error.c | 3 +- drivers/gpu/drm/i915/i915_perf.c | 25 ++--- drivers/gpu/drm/i915/i915_request.c | 34 +++---- drivers/gpu/drm/i915/i915_request.h | 1 + drivers/gpu/drm/i915/intel_engine_cs.c | 54 ++++++---- drivers/gpu/drm/i915/intel_guc_submission.c | 10 +- drivers/gpu/drm/i915/intel_lrc.c | 125 +++++++++++++---------- drivers/gpu/drm/i915/intel_lrc.h | 7 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 100 +++++++++++------- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +- drivers/gpu/drm/i915/selftests/mock_context.c | 7 ++ drivers/gpu/drm/i915/selftests/mock_engine.c | 41 +++++--- 20 files changed, 321 insertions(+), 296 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c index 0f949554d118..708170e61625 100644 --- a/drivers/gpu/drm/i915/gvt/mmio_context.c +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c @@ -446,9 +446,9 @@ static void switch_mocs(struct intel_vgpu *pre, struct intel_vgpu *next, #define CTX_CONTEXT_CONTROL_VAL 0x03 -bool is_inhibit_context(struct i915_gem_context *ctx, int ring_id) +bool is_inhibit_context(struct intel_context *ce) { - u32 *reg_state = ctx->__engine[ring_id].lrc_reg_state; + const u32 *reg_state = ce->lrc_reg_state; u32 inhibit_mask = _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); @@ -501,7 +501,7 @@ static void switch_mmio(struct intel_vgpu *pre, * itself. */ if (mmio->in_context && - !is_inhibit_context(s->shadow_ctx, ring_id)) + !is_inhibit_context(&s->shadow_ctx->__engine[ring_id])) continue; if (mmio->mask) diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.h b/drivers/gpu/drm/i915/gvt/mmio_context.h index 0439eb8057a8..5c3b9ff9f96a 100644 --- a/drivers/gpu/drm/i915/gvt/mmio_context.h +++ b/drivers/gpu/drm/i915/gvt/mmio_context.h @@ -49,7 +49,7 @@ void intel_gvt_switch_mmio(struct intel_vgpu *pre, void intel_gvt_init_engine_mmio_context(struct intel_gvt *gvt); -bool is_inhibit_context(struct i915_gem_context *ctx, int ring_id); +bool is_inhibit_context(struct intel_context *ce); int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu, struct i915_request *req); diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 17f9f8d7e148..e1760030dda1 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -54,11 +54,8 @@ static void set_context_pdp_root_pointer( static void update_shadow_pdps(struct intel_vgpu_workload *workload) { - struct intel_vgpu *vgpu = workload->vgpu; - int ring_id = workload->ring_id; - struct i915_gem_context *shadow_ctx = vgpu->submission.shadow_ctx; struct drm_i915_gem_object *ctx_obj = - shadow_ctx->__engine[ring_id].state->obj; + workload->req->hw_context->state->obj; struct execlist_ring_context *shadow_ring_context; struct page *page; @@ -128,9 +125,8 @@ static int populate_shadow_context(struct intel_vgpu_workload *workload) struct intel_vgpu *vgpu = workload->vgpu; struct intel_gvt *gvt = vgpu->gvt; int ring_id = workload->ring_id; - struct i915_gem_context *shadow_ctx = vgpu->submission.shadow_ctx; struct drm_i915_gem_object *ctx_obj = - shadow_ctx->__engine[ring_id].state->obj; + workload->req->hw_context->state->obj; struct execlist_ring_context *shadow_ring_context; struct page *page; void *dst; @@ -280,10 +276,8 @@ static int shadow_context_status_change(struct notifier_block *nb, return NOTIFY_OK; } -static void shadow_context_descriptor_update(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static void shadow_context_descriptor_update(struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); u64 desc = 0; desc = ce->lrc_desc; @@ -292,7 +286,7 @@ static void shadow_context_descriptor_update(struct i915_gem_context *ctx, * like GEN8_CTX_* cached in desc_template */ desc &= U64_MAX << 12; - desc |= ctx->desc_template & ((1ULL << 12) - 1); + desc |= ce->gem_context->desc_template & ((1ULL << 12) - 1); ce->lrc_desc = desc; } @@ -300,12 +294,11 @@ static void shadow_context_descriptor_update(struct i915_gem_context *ctx, static int copy_workload_to_ring_buffer(struct intel_vgpu_workload *workload) { struct intel_vgpu *vgpu = workload->vgpu; + struct i915_request *req = workload->req; void *shadow_ring_buffer_va; u32 *cs; - struct i915_request *req = workload->req; - if (IS_KABYLAKE(req->i915) && - is_inhibit_context(req->gem_context, req->engine->id)) + if (IS_KABYLAKE(req->i915) && is_inhibit_context(req->hw_context)) intel_vgpu_restore_inhibit_context(vgpu, req); /* allocate shadow ring buffer */ @@ -353,60 +346,56 @@ int intel_gvt_scan_and_shadow_workload(struct intel_vgpu_workload *workload) struct intel_vgpu_submission *s = &vgpu->submission; struct i915_gem_context *shadow_ctx = s->shadow_ctx; struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; - int ring_id = workload->ring_id; - struct intel_engine_cs *engine = dev_priv->engine[ring_id]; - struct intel_ring *ring; + struct intel_engine_cs *engine = dev_priv->engine[workload->ring_id]; + struct intel_context *ce; int ret; lockdep_assert_held(&dev_priv->drm.struct_mutex); - if (workload->shadowed) + if (workload->req) return 0; + /* pin shadow context by gvt even the shadow context will be pinned + * when i915 alloc request. That is because gvt will update the guest + * context from shadow context when workload is completed, and at that + * moment, i915 may already unpined the shadow context to make the + * shadow_ctx pages invalid. So gvt need to pin itself. After update + * the guest context, gvt can unpin the shadow_ctx safely. + */ + ce = intel_context_pin(shadow_ctx, engine); + if (IS_ERR(ce)) { + gvt_vgpu_err("fail to pin shadow context\n"); + return PTR_ERR(ce); + } + shadow_ctx->desc_template &= ~(0x3 << GEN8_CTX_ADDRESSING_MODE_SHIFT); shadow_ctx->desc_template |= workload->ctx_desc.addressing_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT; - if (!test_and_set_bit(ring_id, s->shadow_ctx_desc_updated)) - shadow_context_descriptor_update(shadow_ctx, - dev_priv->engine[ring_id]); + if (!test_and_set_bit(workload->ring_id, s->shadow_ctx_desc_updated)) + shadow_context_descriptor_update(ce); ret = intel_gvt_scan_and_shadow_ringbuffer(workload); if (ret) - goto err_scan; + goto err_unpin; if ((workload->ring_id == RCS) && (workload->wa_ctx.indirect_ctx.size != 0)) { ret = intel_gvt_scan_and_shadow_wa_ctx(&workload->wa_ctx); if (ret) - goto err_scan; - } - - /* pin shadow context by gvt even the shadow context will be pinned - * when i915 alloc request. That is because gvt will update the guest - * context from shadow context when workload is completed, and at that - * moment, i915 may already unpined the shadow context to make the - * shadow_ctx pages invalid. So gvt need to pin itself. After update - * the guest context, gvt can unpin the shadow_ctx safely. - */ - ring = intel_context_pin(shadow_ctx, engine); - if (IS_ERR(ring)) { - ret = PTR_ERR(ring); - gvt_vgpu_err("fail to pin shadow context\n"); - goto err_shadow; + goto err_shadow; } ret = populate_shadow_context(workload); if (ret) - goto err_unpin; - workload->shadowed = true; + goto err_shadow; + return 0; -err_unpin: - intel_context_unpin(shadow_ctx, engine); err_shadow: release_shadow_wa_ctx(&workload->wa_ctx); -err_scan: +err_unpin: + intel_context_unpin(ce); return ret; } @@ -414,7 +403,6 @@ static int intel_gvt_generate_request(struct intel_vgpu_workload *workload) { int ring_id = workload->ring_id; struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv; - struct intel_engine_cs *engine = dev_priv->engine[ring_id]; struct i915_request *rq; struct intel_vgpu *vgpu = workload->vgpu; struct intel_vgpu_submission *s = &vgpu->submission; @@ -437,7 +425,6 @@ static int intel_gvt_generate_request(struct intel_vgpu_workload *workload) return 0; err_unpin: - intel_context_unpin(shadow_ctx, engine); release_shadow_wa_ctx(&workload->wa_ctx); return ret; } @@ -517,21 +504,13 @@ err: return ret; } -static int update_wa_ctx_2_shadow_ctx(struct intel_shadow_wa_ctx *wa_ctx) +static void update_wa_ctx_2_shadow_ctx(struct intel_shadow_wa_ctx *wa_ctx) { - struct intel_vgpu_workload *workload = container_of(wa_ctx, - struct intel_vgpu_workload, - wa_ctx); - int ring_id = workload->ring_id; - struct intel_vgpu_submission *s = &workload->vgpu->submission; - struct i915_gem_context *shadow_ctx = s->shadow_ctx; - struct drm_i915_gem_object *ctx_obj = - shadow_ctx->__engine[ring_id].state->obj; - struct execlist_ring_context *shadow_ring_context; - struct page *page; - - page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN); - shadow_ring_context = kmap_atomic(page); + struct intel_vgpu_workload *workload = + container_of(wa_ctx, struct intel_vgpu_workload, wa_ctx); + struct i915_request *rq = workload->req; + struct execlist_ring_context *shadow_ring_context = + (struct execlist_ring_context *)rq->hw_context->lrc_reg_state; shadow_ring_context->bb_per_ctx_ptr.val = (shadow_ring_context->bb_per_ctx_ptr.val & @@ -539,9 +518,6 @@ static int update_wa_ctx_2_shadow_ctx(struct intel_shadow_wa_ctx *wa_ctx) shadow_ring_context->rcs_indirect_ctx.val = (shadow_ring_context->rcs_indirect_ctx.val & (~INDIRECT_CTX_ADDR_MASK)) | wa_ctx->indirect_ctx.shadow_gma; - - kunmap_atomic(shadow_ring_context); - return 0; } static int prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) @@ -670,12 +646,9 @@ err_unpin_mm: static int dispatch_workload(struct intel_vgpu_workload *workload) { struct intel_vgpu *vgpu = workload->vgpu; - struct intel_vgpu_submission *s = &vgpu->submission; - struct i915_gem_context *shadow_ctx = s->shadow_ctx; struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; int ring_id = workload->ring_id; - struct intel_engine_cs *engine = dev_priv->engine[ring_id]; - int ret = 0; + int ret; gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n", ring_id, workload); @@ -687,10 +660,6 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) goto out; ret = prepare_workload(workload); - if (ret) { - intel_context_unpin(shadow_ctx, engine); - goto out; - } out: if (ret) @@ -765,27 +734,23 @@ out: static void update_guest_context(struct intel_vgpu_workload *workload) { + struct i915_request *rq = workload->req; struct intel_vgpu *vgpu = workload->vgpu; struct intel_gvt *gvt = vgpu->gvt; - struct intel_vgpu_submission *s = &vgpu->submission; - struct i915_gem_context *shadow_ctx = s->shadow_ctx; - int ring_id = workload->ring_id; - struct drm_i915_gem_object *ctx_obj = - shadow_ctx->__engine[ring_id].state->obj; + struct drm_i915_gem_object *ctx_obj = rq->hw_context->state->obj; struct execlist_ring_context *shadow_ring_context; struct page *page; void *src; unsigned long context_gpa, context_page_num; int i; - gvt_dbg_sched("ring id %d workload lrca %x\n", ring_id, - workload->ctx_desc.lrca); - - context_page_num = gvt->dev_priv->engine[ring_id]->context_size; + gvt_dbg_sched("ring id %d workload lrca %x\n", rq->engine->id, + workload->ctx_desc.lrca); + context_page_num = rq->engine->context_size; context_page_num = context_page_num >> PAGE_SHIFT; - if (IS_BROADWELL(gvt->dev_priv) && ring_id == RCS) + if (IS_BROADWELL(gvt->dev_priv) && rq->engine->id == RCS) context_page_num = 19; i = 2; @@ -858,6 +823,7 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) scheduler->current_workload[ring_id]; struct intel_vgpu *vgpu = workload->vgpu; struct intel_vgpu_submission *s = &vgpu->submission; + struct i915_request *rq; int event; mutex_lock(&gvt->lock); @@ -866,11 +832,8 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) * switch to make sure request is completed. * For the workload w/o request, directly complete the workload. */ - if (workload->req) { - struct drm_i915_private *dev_priv = - workload->vgpu->gvt->dev_priv; - struct intel_engine_cs *engine = - dev_priv->engine[workload->ring_id]; + rq = fetch_and_zero(&workload->req); + if (rq) { wait_event(workload->shadow_ctx_status_wq, !atomic_read(&workload->shadow_ctx_active)); @@ -886,8 +849,6 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) workload->status = 0; } - i915_request_put(fetch_and_zero(&workload->req)); - if (!workload->status && !(vgpu->resetting_eng & ENGINE_MASK(ring_id))) { update_guest_context(workload); @@ -896,10 +857,13 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) INTEL_GVT_EVENT_MAX) intel_vgpu_trigger_virtual_event(vgpu, event); } - mutex_lock(&dev_priv->drm.struct_mutex); + /* unpin shadow ctx as the shadow_ctx update is done */ - intel_context_unpin(s->shadow_ctx, engine); - mutex_unlock(&dev_priv->drm.struct_mutex); + mutex_lock(&rq->i915->drm.struct_mutex); + intel_context_unpin(rq->hw_context); + mutex_unlock(&rq->i915->drm.struct_mutex); + + i915_request_put(rq); } gvt_dbg_sched("ring id %d complete workload %p status %d\n", @@ -1270,7 +1234,6 @@ alloc_workload(struct intel_vgpu *vgpu) atomic_set(&workload->shadow_ctx_active, 0); workload->status = -EINPROGRESS; - workload->shadowed = false; workload->vgpu = vgpu; return workload; diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h b/drivers/gpu/drm/i915/gvt/scheduler.h index 6c644782193e..21eddab4a9cd 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.h +++ b/drivers/gpu/drm/i915/gvt/scheduler.h @@ -83,7 +83,6 @@ struct intel_vgpu_workload { struct i915_request *req; /* if this workload has been dispatched to i915? */ bool dispatched; - bool shadowed; int status; struct intel_vgpu_mm *shadow_mm; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 34c125e2d90c..e33c380b43e3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1950,6 +1950,7 @@ struct drm_i915_private { */ struct i915_perf_stream *exclusive_stream; + struct intel_context *pinned_ctx; u32 specific_ctx_id; struct hrtimer poll_check_timer; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a20f8db5729d..03874b50ada9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3181,14 +3181,14 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, i915_retire_requests(dev_priv); for_each_engine(engine, dev_priv, id) { - struct i915_gem_context *ctx; + struct intel_context *ce; i915_gem_reset_engine(engine, engine->hangcheck.active_request, stalled_mask & ENGINE_MASK(id)); - ctx = fetch_and_zero(&engine->last_retired_context); - if (ctx) - intel_context_unpin(ctx, engine); + ce = fetch_and_zero(&engine->last_retired_context); + if (ce) + intel_context_unpin(ce); /* * Ostensibily, we always want a context loaded for powersaving, @@ -4897,13 +4897,13 @@ void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj) static void assert_kernel_context_is_current(struct drm_i915_private *i915) { - struct i915_gem_context *kernel_context = i915->kernel_context; + struct i915_gem_context *kctx = i915->kernel_context; struct intel_engine_cs *engine; enum intel_engine_id id; for_each_engine(engine, i915, id) { GEM_BUG_ON(__i915_gem_active_peek(&engine->timeline.last_request)); - GEM_BUG_ON(engine->last_retired_context != kernel_context); + GEM_BUG_ON(engine->last_retired_context->gem_context != kctx); } } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9e70f4dfa703..b69b18ef8120 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -127,14 +127,8 @@ static void i915_gem_context_free(struct i915_gem_context *ctx) for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) { struct intel_context *ce = &ctx->__engine[n]; - if (!ce->state) - continue; - - WARN_ON(ce->pin_count); - if (ce->ring) - intel_ring_free(ce->ring); - - __i915_gem_object_release_unless_active(ce->state->obj); + if (ce->ops) + ce->ops->destroy(ce); } kfree(ctx->name); @@ -266,6 +260,7 @@ __create_hw_context(struct drm_i915_private *dev_priv, struct drm_i915_file_private *file_priv) { struct i915_gem_context *ctx; + unsigned int n; int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); @@ -283,6 +278,12 @@ __create_hw_context(struct drm_i915_private *dev_priv, ctx->i915 = dev_priv; ctx->sched.priority = I915_PRIORITY_NORMAL; + for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) { + struct intel_context *ce = &ctx->__engine[n]; + + ce->gem_context = ctx; + } + INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); INIT_LIST_HEAD(&ctx->handles_list); diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index ace3b129c189..749a4ff566f5 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -45,6 +45,11 @@ struct intel_ring; #define DEFAULT_CONTEXT_HANDLE 0 +struct intel_context_ops { + void (*unpin)(struct intel_context *ce); + void (*destroy)(struct intel_context *ce); +}; + /** * struct i915_gem_context - client state * @@ -144,11 +149,14 @@ struct i915_gem_context { /** engine: per-engine logical HW state */ struct intel_context { + struct i915_gem_context *gem_context; struct i915_vma *state; struct intel_ring *ring; u32 *lrc_reg_state; u64 lrc_desc; int pin_count; + + const struct intel_context_ops *ops; } __engine[I915_NUM_ENGINES]; /** ring_size: size for allocating the per-engine ring buffer */ @@ -263,25 +271,22 @@ to_intel_context(struct i915_gem_context *ctx, return &ctx->__engine[engine->id]; } -static inline struct intel_ring * +static inline struct intel_context * intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine) { return engine->context_pin(engine, ctx); } -static inline void __intel_context_pin(struct i915_gem_context *ctx, - const struct intel_engine_cs *engine) +static inline void __intel_context_pin(struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); - GEM_BUG_ON(!ce->pin_count); ce->pin_count++; } -static inline void intel_context_unpin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static inline void intel_context_unpin(struct intel_context *ce) { - engine->context_unpin(engine, ctx); + GEM_BUG_ON(!ce->ops); + ce->ops->unpin(ce); } /* i915_gem_context.c */ diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 37c9a42654ba..47721437a4c5 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1485,8 +1485,7 @@ static void gem_record_rings(struct i915_gpu_state *error) ee->ctx = i915_error_object_create(i915, - to_intel_context(ctx, - engine)->state); + request->hw_context->state); error->simulated |= i915_gem_context_no_error_capture(ctx); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 019bd2d073ad..4f0eb84b3c00 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1221,7 +1221,7 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) dev_priv->perf.oa.specific_ctx_id = stream->ctx->hw_id; } else { struct intel_engine_cs *engine = dev_priv->engine[RCS]; - struct intel_ring *ring; + struct intel_context *ce; int ret; ret = i915_mutex_lock_interruptible(&dev_priv->drm); @@ -1234,19 +1234,19 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) * * NB: implied RCS engine... */ - ring = intel_context_pin(stream->ctx, engine); + ce = intel_context_pin(stream->ctx, engine); mutex_unlock(&dev_priv->drm.struct_mutex); - if (IS_ERR(ring)) - return PTR_ERR(ring); + if (IS_ERR(ce)) + return PTR_ERR(ce); + dev_priv->perf.oa.pinned_ctx = ce; /* * Explicitly track the ID (instead of calling * i915_ggtt_offset() on the fly) considering the difference * with gen8+ and execlists */ - dev_priv->perf.oa.specific_ctx_id = - i915_ggtt_offset(to_intel_context(stream->ctx, engine)->state); + dev_priv->perf.oa.specific_ctx_id = i915_ggtt_offset(ce->state); } return 0; @@ -1262,17 +1262,14 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) static void oa_put_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; + struct intel_context *ce; - if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) { - dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; - } else { - struct intel_engine_cs *engine = dev_priv->engine[RCS]; + dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; + ce = fetch_and_zero(&dev_priv->perf.oa.pinned_ctx); + if (ce) { mutex_lock(&dev_priv->drm.struct_mutex); - - dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; - intel_context_unpin(stream->ctx, engine); - + intel_context_unpin(ce); mutex_unlock(&dev_priv->drm.struct_mutex); } } diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index fe8810a6a339..fc499bcbd105 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -383,8 +383,8 @@ static void __retire_engine_request(struct intel_engine_cs *engine, * the subsequent request. */ if (engine->last_retired_context) - intel_context_unpin(engine->last_retired_context, engine); - engine->last_retired_context = rq->gem_context; + intel_context_unpin(engine->last_retired_context); + engine->last_retired_context = rq->hw_context; } static void __retire_engine_upto(struct intel_engine_cs *engine, @@ -456,7 +456,7 @@ static void i915_request_retire(struct i915_request *request) /* Retirement decays the ban score as it is a sign of ctx progress */ atomic_dec_if_positive(&request->gem_context->ban_score); - intel_context_unpin(request->gem_context, request->engine); + intel_context_unpin(request->hw_context); __retire_engine_upto(request->engine, request); @@ -657,7 +657,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) { struct drm_i915_private *i915 = engine->i915; struct i915_request *rq; - struct intel_ring *ring; + struct intel_context *ce; int ret; lockdep_assert_held(&i915->drm.struct_mutex); @@ -681,22 +681,21 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) * GGTT space, so do this first before we reserve a seqno for * ourselves. */ - ring = intel_context_pin(ctx, engine); - if (IS_ERR(ring)) - return ERR_CAST(ring); - GEM_BUG_ON(!ring); + ce = intel_context_pin(ctx, engine); + if (IS_ERR(ce)) + return ERR_CAST(ce); ret = reserve_gt(i915); if (ret) goto err_unpin; - ret = intel_ring_wait_for_space(ring, MIN_SPACE_FOR_ADD_REQUEST); + ret = intel_ring_wait_for_space(ce->ring, MIN_SPACE_FOR_ADD_REQUEST); if (ret) goto err_unreserve; /* Move our oldest request to the slab-cache (if not in use!) */ - rq = list_first_entry(&ring->request_list, typeof(*rq), ring_link); - if (!list_is_last(&rq->ring_link, &ring->request_list) && + rq = list_first_entry(&ce->ring->request_list, typeof(*rq), ring_link); + if (!list_is_last(&rq->ring_link, &ce->ring->request_list) && i915_request_completed(rq)) i915_request_retire(rq); @@ -761,8 +760,9 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) rq->i915 = i915; rq->engine = engine; rq->gem_context = ctx; - rq->ring = ring; - rq->timeline = ring->timeline; + rq->hw_context = ce; + rq->ring = ce->ring; + rq->timeline = ce->ring->timeline; GEM_BUG_ON(rq->timeline == &engine->timeline); spin_lock_init(&rq->lock); @@ -814,14 +814,14 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) goto err_unwind; /* Keep a second pin for the dual retirement along engine and ring */ - __intel_context_pin(rq->gem_context, engine); + __intel_context_pin(ce); /* Check that we didn't interrupt ourselves with a new request */ GEM_BUG_ON(rq->timeline->seqno != rq->fence.seqno); return rq; err_unwind: - rq->ring->emit = rq->head; + ce->ring->emit = rq->head; /* Make sure we didn't add ourselves to external state before freeing */ GEM_BUG_ON(!list_empty(&rq->active_list)); @@ -832,7 +832,7 @@ err_unwind: err_unreserve: unreserve_gt(i915); err_unpin: - intel_context_unpin(ctx, engine); + intel_context_unpin(ce); return ERR_PTR(ret); } @@ -1018,8 +1018,8 @@ i915_request_await_object(struct i915_request *to, void __i915_request_add(struct i915_request *request, bool flush_caches) { struct intel_engine_cs *engine = request->engine; - struct intel_ring *ring = request->ring; struct i915_timeline *timeline = request->timeline; + struct intel_ring *ring = request->ring; struct i915_request *prev; u32 *cs; int err; diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h index dddecd9ffd0c..1bbbb7a9fa03 100644 --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -95,6 +95,7 @@ struct i915_request { */ struct i915_gem_context *gem_context; struct intel_engine_cs *engine; + struct intel_context *hw_context; struct intel_ring *ring; struct i915_timeline *timeline; struct intel_signal_node signaling; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 9e618aab6568..26f9f8aab949 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -645,6 +645,12 @@ static int init_phys_status_page(struct intel_engine_cs *engine) return 0; } +static void __intel_context_unpin(struct i915_gem_context *ctx, + struct intel_engine_cs *engine) +{ + intel_context_unpin(to_intel_context(ctx, engine)); +} + /** * intel_engines_init_common - initialize cengine state which might require hw access * @engine: Engine to initialize. @@ -658,7 +664,8 @@ static int init_phys_status_page(struct intel_engine_cs *engine) */ int intel_engine_init_common(struct intel_engine_cs *engine) { - struct intel_ring *ring; + struct drm_i915_private *i915 = engine->i915; + struct intel_context *ce; int ret; engine->set_default_submission(engine); @@ -670,18 +677,18 @@ int intel_engine_init_common(struct intel_engine_cs *engine) * be available. To avoid this we always pin the default * context. */ - ring = intel_context_pin(engine->i915->kernel_context, engine); - if (IS_ERR(ring)) - return PTR_ERR(ring); + ce = intel_context_pin(i915->kernel_context, engine); + if (IS_ERR(ce)) + return PTR_ERR(ce); /* * Similarly the preempt context must always be available so that * we can interrupt the engine at any time. */ - if (engine->i915->preempt_context) { - ring = intel_context_pin(engine->i915->preempt_context, engine); - if (IS_ERR(ring)) { - ret = PTR_ERR(ring); + if (i915->preempt_context) { + ce = intel_context_pin(i915->preempt_context, engine); + if (IS_ERR(ce)) { + ret = PTR_ERR(ce); goto err_unpin_kernel; } } @@ -690,7 +697,7 @@ int intel_engine_init_common(struct intel_engine_cs *engine) if (ret) goto err_unpin_preempt; - if (HWS_NEEDS_PHYSICAL(engine->i915)) + if (HWS_NEEDS_PHYSICAL(i915)) ret = init_phys_status_page(engine); else ret = init_status_page(engine); @@ -702,10 +709,11 @@ int intel_engine_init_common(struct intel_engine_cs *engine) err_breadcrumbs: intel_engine_fini_breadcrumbs(engine); err_unpin_preempt: - if (engine->i915->preempt_context) - intel_context_unpin(engine->i915->preempt_context, engine); + if (i915->preempt_context) + __intel_context_unpin(i915->preempt_context, engine); + err_unpin_kernel: - intel_context_unpin(engine->i915->kernel_context, engine); + __intel_context_unpin(i915->kernel_context, engine); return ret; } @@ -718,6 +726,8 @@ err_unpin_kernel: */ void intel_engine_cleanup_common(struct intel_engine_cs *engine) { + struct drm_i915_private *i915 = engine->i915; + intel_engine_cleanup_scratch(engine); if (HWS_NEEDS_PHYSICAL(engine->i915)) @@ -732,9 +742,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) if (engine->default_state) i915_gem_object_put(engine->default_state); - if (engine->i915->preempt_context) - intel_context_unpin(engine->i915->preempt_context, engine); - intel_context_unpin(engine->i915->kernel_context, engine); + if (i915->preempt_context) + __intel_context_unpin(i915->preempt_context, engine); + __intel_context_unpin(i915->kernel_context, engine); i915_timeline_fini(&engine->timeline); } @@ -1007,8 +1017,8 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv) */ bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine) { - const struct i915_gem_context * const kernel_context = - engine->i915->kernel_context; + const struct intel_context *kernel_context = + to_intel_context(engine->i915->kernel_context, engine); struct i915_request *rq; lockdep_assert_held(&engine->i915->drm.struct_mutex); @@ -1020,7 +1030,7 @@ bool intel_engine_has_kernel_context(const struct intel_engine_cs *engine) */ rq = __i915_gem_active_peek(&engine->timeline.last_request); if (rq) - return rq->gem_context == kernel_context; + return rq->hw_context == kernel_context; else return engine->last_retired_context == kernel_context; } @@ -1107,16 +1117,16 @@ void intel_engines_unpark(struct drm_i915_private *i915) */ void intel_engine_lost_context(struct intel_engine_cs *engine) { - struct i915_gem_context *ctx; + struct intel_context *ce; lockdep_assert_held(&engine->i915->drm.struct_mutex); engine->legacy_active_context = NULL; engine->legacy_active_ppgtt = NULL; - ctx = fetch_and_zero(&engine->last_retired_context); - if (ctx) - intel_context_unpin(ctx, engine); + ce = fetch_and_zero(&engine->last_retired_context); + if (ce) + intel_context_unpin(ce); } bool intel_engine_can_store_dword(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index a432a193f3c4..133367a17863 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -513,9 +513,7 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) { struct intel_guc_client *client = guc->execbuf_client; struct intel_engine_cs *engine = rq->engine; - u32 ctx_desc = - lower_32_bits(intel_lr_context_descriptor(rq->gem_context, - engine)); + u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc); u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64); spin_lock(&client->wq_lock); @@ -553,8 +551,8 @@ static void inject_preempt_context(struct work_struct *work) preempt_work[engine->id]); struct intel_guc_client *client = guc->preempt_client; struct guc_stage_desc *stage_desc = __get_stage_desc(client); - u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(client->owner, - engine)); + u32 ctx_desc = lower_32_bits(to_intel_context(client->owner, + engine)->lrc_desc); u32 data[7]; /* @@ -726,7 +724,7 @@ static bool __guc_dequeue(struct intel_engine_cs *engine) struct i915_request *rq, *rn; list_for_each_entry_safe(rq, rn, &p->requests, sched.link) { - if (last && rq->gem_context != last->gem_context) { + if (last && rq->hw_context != last->hw_context) { if (port == last_port) { __list_del_many(&p->requests, &rq->sched.link); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 1e9cc55d785c..b97c5d4c7877 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -164,7 +164,8 @@ #define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS) static int execlists_context_deferred_alloc(struct i915_gem_context *ctx, - struct intel_engine_cs *engine); + struct intel_engine_cs *engine, + struct intel_context *ce); static void execlists_init_reg_state(u32 *reg_state, struct i915_gem_context *ctx, struct intel_engine_cs *engine, @@ -189,12 +190,7 @@ static inline bool need_preempt(const struct intel_engine_cs *engine, !i915_request_completed(last)); } -/** - * intel_lr_context_descriptor_update() - calculate & cache the descriptor - * descriptor for a pinned context - * @ctx: Context to work on - * @engine: Engine the descriptor will be used with - * +/* * The context descriptor encodes various attributes of a context, * including its GTT address and some flags. Because it's fairly * expensive to calculate, we'll just do it once and cache the result, @@ -222,9 +218,9 @@ static inline bool need_preempt(const struct intel_engine_cs *engine, */ static void intel_lr_context_descriptor_update(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) + struct intel_engine_cs *engine, + struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); u64 desc; BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH))); @@ -418,8 +414,7 @@ execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state) static u64 execlists_update_context(struct i915_request *rq) { - struct intel_context *ce = - to_intel_context(rq->gem_context, rq->engine); + struct intel_context *ce = rq->hw_context; struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt ?: rq->i915->mm.aliasing_ppgtt; u32 *reg_state = ce->lrc_reg_state; @@ -496,14 +491,14 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) execlists_clear_active(execlists, EXECLISTS_ACTIVE_HWACK); } -static bool ctx_single_port_submission(const struct i915_gem_context *ctx) +static bool ctx_single_port_submission(const struct intel_context *ce) { return (IS_ENABLED(CONFIG_DRM_I915_GVT) && - i915_gem_context_force_single_submission(ctx)); + i915_gem_context_force_single_submission(ce->gem_context)); } -static bool can_merge_ctx(const struct i915_gem_context *prev, - const struct i915_gem_context *next) +static bool can_merge_ctx(const struct intel_context *prev, + const struct intel_context *next) { if (prev != next) return false; @@ -680,8 +675,8 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) * second request, and so we never need to tell the * hardware about the first. */ - if (last && !can_merge_ctx(rq->gem_context, - last->gem_context)) { + if (last && + !can_merge_ctx(rq->hw_context, last->hw_context)) { /* * If we are on the second port and cannot * combine this request with the last, then we @@ -700,14 +695,14 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine) * the same context (even though a different * request) to the second port. */ - if (ctx_single_port_submission(last->gem_context) || - ctx_single_port_submission(rq->gem_context)) { + if (ctx_single_port_submission(last->hw_context) || + ctx_single_port_submission(rq->hw_context)) { __list_del_many(&p->requests, &rq->sched.link); goto done; } - GEM_BUG_ON(last->gem_context == rq->gem_context); + GEM_BUG_ON(last->hw_context == rq->hw_context); if (submit) port_assign(port, last); @@ -1339,6 +1334,37 @@ static void execlists_schedule(struct i915_request *request, spin_unlock_irq(&engine->timeline.lock); } +static void execlists_context_destroy(struct intel_context *ce) +{ + GEM_BUG_ON(!ce->state); + GEM_BUG_ON(ce->pin_count); + + intel_ring_free(ce->ring); + __i915_gem_object_release_unless_active(ce->state->obj); +} + +static void __execlists_context_unpin(struct intel_context *ce) +{ + intel_ring_unpin(ce->ring); + + ce->state->obj->pin_global--; + i915_gem_object_unpin_map(ce->state->obj); + i915_vma_unpin(ce->state); + + i915_gem_context_put(ce->gem_context); +} + +static void execlists_context_unpin(struct intel_context *ce) +{ + lockdep_assert_held(&ce->gem_context->i915->drm.struct_mutex); + GEM_BUG_ON(ce->pin_count == 0); + + if (--ce->pin_count) + return; + + __execlists_context_unpin(ce); +} + static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma) { unsigned int flags; @@ -1362,21 +1388,15 @@ static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma) return i915_vma_pin(vma, 0, GEN8_LR_CONTEXT_ALIGN, flags); } -static struct intel_ring * -execlists_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static struct intel_context * +__execlists_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx, + struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); void *vaddr; int ret; - lockdep_assert_held(&ctx->i915->drm.struct_mutex); - - if (likely(ce->pin_count++)) - goto out; - GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ - - ret = execlists_context_deferred_alloc(ctx, engine); + ret = execlists_context_deferred_alloc(ctx, engine, ce); if (ret) goto err; GEM_BUG_ON(!ce->state); @@ -1395,7 +1415,7 @@ execlists_context_pin(struct intel_engine_cs *engine, if (ret) goto unpin_map; - intel_lr_context_descriptor_update(ctx, engine); + intel_lr_context_descriptor_update(ctx, engine, ce); ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE; ce->lrc_reg_state[CTX_RING_BUFFER_START+1] = @@ -1404,8 +1424,7 @@ execlists_context_pin(struct intel_engine_cs *engine, ce->state->obj->pin_global++; i915_gem_context_get(ctx); -out: - return ce->ring; + return ce; unpin_map: i915_gem_object_unpin_map(ce->state->obj); @@ -1416,33 +1435,33 @@ err: return ERR_PTR(ret); } -static void execlists_context_unpin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static const struct intel_context_ops execlists_context_ops = { + .unpin = execlists_context_unpin, + .destroy = execlists_context_destroy, +}; + +static struct intel_context * +execlists_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = to_intel_context(ctx, engine); lockdep_assert_held(&ctx->i915->drm.struct_mutex); - GEM_BUG_ON(ce->pin_count == 0); - if (--ce->pin_count) - return; - - intel_ring_unpin(ce->ring); + if (likely(ce->pin_count++)) + return ce; + GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ - ce->state->obj->pin_global--; - i915_gem_object_unpin_map(ce->state->obj); - i915_vma_unpin(ce->state); + ce->ops = &execlists_context_ops; - i915_gem_context_put(ctx); + return __execlists_context_pin(engine, ctx, ce); } static int execlists_request_alloc(struct i915_request *request) { - struct intel_context *ce = - to_intel_context(request->gem_context, request->engine); int ret; - GEM_BUG_ON(!ce->pin_count); + GEM_BUG_ON(!request->hw_context->pin_count); /* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just @@ -1956,7 +1975,7 @@ static void execlists_reset(struct intel_engine_cs *engine, * future request will be after userspace has had the opportunity * to recreate its own state. */ - regs = to_intel_context(request->gem_context, engine)->lrc_reg_state; + regs = request->hw_context->lrc_reg_state; if (engine->default_state) { void *defaults; @@ -2327,8 +2346,6 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) engine->reset.finish = execlists_reset_finish; engine->context_pin = execlists_context_pin; - engine->context_unpin = execlists_context_unpin; - engine->request_alloc = execlists_request_alloc; engine->emit_flush = gen8_emit_flush; @@ -2563,7 +2580,7 @@ static void execlists_init_reg_state(u32 *regs, struct drm_i915_private *dev_priv = engine->i915; struct i915_hw_ppgtt *ppgtt = ctx->ppgtt ?: dev_priv->mm.aliasing_ppgtt; u32 base = engine->mmio_base; - bool rcs = engine->id == RCS; + bool rcs = engine->class == RENDER_CLASS; /* A context is actually a big batch buffer with several * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The @@ -2710,10 +2727,10 @@ err_unpin_ctx: } static int execlists_context_deferred_alloc(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) + struct intel_engine_cs *engine, + struct intel_context *ce) { struct drm_i915_gem_object *ctx_obj; - struct intel_context *ce = to_intel_context(ctx, engine); struct i915_vma *vma; uint32_t context_size; struct intel_ring *ring; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 4ec7d8dd13c8..1593194e930c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -104,11 +104,4 @@ struct i915_gem_context; void intel_lr_context_resume(struct drm_i915_private *dev_priv); -static inline uint64_t -intel_lr_context_descriptor(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) -{ - return to_intel_context(ctx, engine)->lrc_desc; -} - #endif /* _INTEL_LRC_H_ */ diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 53703012ec75..0c0c9f531e4e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -571,8 +571,7 @@ static void reset_ring(struct intel_engine_cs *engine, */ if (request) { struct drm_i915_private *dev_priv = request->i915; - struct intel_context *ce = - to_intel_context(request->gem_context, engine); + struct intel_context *ce = request->hw_context; struct i915_hw_ppgtt *ppgtt; if (ce->state) { @@ -1186,7 +1185,31 @@ intel_ring_free(struct intel_ring *ring) kfree(ring); } -static int context_pin(struct intel_context *ce) +static void intel_ring_context_destroy(struct intel_context *ce) +{ + GEM_BUG_ON(ce->pin_count); + + if (ce->state) + __i915_gem_object_release_unless_active(ce->state->obj); +} + +static void intel_ring_context_unpin(struct intel_context *ce) +{ + lockdep_assert_held(&ce->gem_context->i915->drm.struct_mutex); + GEM_BUG_ON(ce->pin_count == 0); + + if (--ce->pin_count) + return; + + if (ce->state) { + ce->state->obj->pin_global--; + i915_vma_unpin(ce->state); + } + + i915_gem_context_put(ce->gem_context); +} + +static int __context_pin(struct intel_context *ce) { struct i915_vma *vma = ce->state; int ret; @@ -1275,25 +1298,19 @@ err_obj: return ERR_PTR(err); } -static struct intel_ring * -intel_ring_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static struct intel_context * +__ring_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx, + struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); - int ret; - - lockdep_assert_held(&ctx->i915->drm.struct_mutex); - - if (likely(ce->pin_count++)) - goto out; - GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ + int err; if (!ce->state && engine->context_size) { struct i915_vma *vma; vma = alloc_context_vma(engine); if (IS_ERR(vma)) { - ret = PTR_ERR(vma); + err = PTR_ERR(vma); goto err; } @@ -1301,8 +1318,8 @@ intel_ring_context_pin(struct intel_engine_cs *engine, } if (ce->state) { - ret = context_pin(ce); - if (ret) + err = __context_pin(ce); + if (err) goto err; ce->state->obj->pin_global++; @@ -1310,32 +1327,37 @@ intel_ring_context_pin(struct intel_engine_cs *engine, i915_gem_context_get(ctx); -out: /* One ringbuffer to rule them all */ - return engine->buffer; + GEM_BUG_ON(!engine->buffer); + ce->ring = engine->buffer; + + return ce; err: ce->pin_count = 0; - return ERR_PTR(ret); + return ERR_PTR(err); } -static void intel_ring_context_unpin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static const struct intel_context_ops ring_context_ops = { + .unpin = intel_ring_context_unpin, + .destroy = intel_ring_context_destroy, +}; + +static struct intel_context * +intel_ring_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = to_intel_context(ctx, engine); lockdep_assert_held(&ctx->i915->drm.struct_mutex); - GEM_BUG_ON(ce->pin_count == 0); - if (--ce->pin_count) - return; + if (likely(ce->pin_count++)) + return ce; + GEM_BUG_ON(!ce->pin_count); /* no overflow please! */ - if (ce->state) { - ce->state->obj->pin_global--; - i915_vma_unpin(ce->state); - } + ce->ops = &ring_context_ops; - i915_gem_context_put(ctx); + return __ring_context_pin(engine, ctx, ce); } static int intel_init_ring_buffer(struct intel_engine_cs *engine) @@ -1346,10 +1368,6 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine) intel_engine_setup_common(engine); - err = intel_engine_init_common(engine); - if (err) - goto err; - timeline = i915_timeline_create(engine->i915, engine->name); if (IS_ERR(timeline)) { err = PTR_ERR(timeline); @@ -1371,8 +1389,14 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine) GEM_BUG_ON(engine->buffer); engine->buffer = ring; + err = intel_engine_init_common(engine); + if (err) + goto err_unpin; + return 0; +err_unpin: + intel_ring_unpin(ring); err_ring: intel_ring_free(ring); err: @@ -1458,7 +1482,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags) *cs++ = MI_NOOP; *cs++ = MI_SET_CONTEXT; - *cs++ = i915_ggtt_offset(to_intel_context(rq->gem_context, engine)->state) | flags; + *cs++ = i915_ggtt_offset(rq->hw_context->state) | flags; /* * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP * WaMiSetContext_Hang:snb,ivb,vlv @@ -1549,7 +1573,7 @@ static int switch_context(struct i915_request *rq) hw_flags = MI_FORCE_RESTORE; } - if (to_intel_context(to_ctx, engine)->state && + if (rq->hw_context->state && (to_ctx != from_ctx || hw_flags & MI_FORCE_RESTORE)) { GEM_BUG_ON(engine->id != RCS); @@ -1597,7 +1621,7 @@ static int ring_request_alloc(struct i915_request *request) { int ret; - GEM_BUG_ON(!to_intel_context(request->gem_context, request->engine)->pin_count); + GEM_BUG_ON(!request->hw_context->pin_count); /* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just @@ -2028,8 +2052,6 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv, engine->reset.finish = reset_finish; engine->context_pin = intel_ring_context_pin; - engine->context_unpin = intel_ring_context_unpin; - engine->request_alloc = ring_request_alloc; engine->emit_breadcrumb = i9xx_emit_breadcrumb; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 2b16185e36c4..20c4e13efc0d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -436,10 +436,9 @@ struct intel_engine_cs { void (*set_default_submission)(struct intel_engine_cs *engine); - 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); + struct intel_context *(*context_pin)(struct intel_engine_cs *engine, + struct i915_gem_context *ctx); + int (*request_alloc)(struct i915_request *rq); int (*init_context)(struct i915_request *rq); @@ -555,7 +554,7 @@ struct intel_engine_cs { * to the kernel context and trash it as the save may not happen * before the hardware is powered down. */ - struct i915_gem_context *last_retired_context; + struct intel_context *last_retired_context; /* We track the current MI_SET_CONTEXT in order to eliminate * redudant context switches. This presumes that requests are not diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c index 501becc47c0c..8904f1ce64e3 100644 --- a/drivers/gpu/drm/i915/selftests/mock_context.c +++ b/drivers/gpu/drm/i915/selftests/mock_context.c @@ -30,6 +30,7 @@ mock_context(struct drm_i915_private *i915, const char *name) { struct i915_gem_context *ctx; + unsigned int n; int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); @@ -43,6 +44,12 @@ mock_context(struct drm_i915_private *i915, INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); INIT_LIST_HEAD(&ctx->handles_list); + for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) { + struct intel_context *ce = &ctx->__engine[n]; + + ce->gem_context = ctx; + } + ret = ida_simple_get(&i915->contexts.hw_ida, 0, MAX_CONTEXT_HW_ID, GFP_KERNEL); if (ret < 0) diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index 26bf29d97007..33eddfc1f8ce 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -72,25 +72,37 @@ static void hw_delay_complete(struct timer_list *t) spin_unlock(&engine->hw_lock); } -static struct intel_ring * -mock_context_pin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static void mock_context_unpin(struct intel_context *ce) { - struct intel_context *ce = to_intel_context(ctx, engine); + if (--ce->pin_count) + return; - if (!ce->pin_count++) - i915_gem_context_get(ctx); + i915_gem_context_put(ce->gem_context); +} - return engine->buffer; +static void mock_context_destroy(struct intel_context *ce) +{ + GEM_BUG_ON(ce->pin_count); } -static void mock_context_unpin(struct intel_engine_cs *engine, - struct i915_gem_context *ctx) +static const struct intel_context_ops mock_context_ops = { + .unpin = mock_context_unpin, + .destroy = mock_context_destroy, +}; + +static struct intel_context * +mock_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = to_intel_context(ctx, engine); - if (!--ce->pin_count) - i915_gem_context_put(ctx); + if (!ce->pin_count++) { + i915_gem_context_get(ctx); + ce->ring = engine->buffer; + ce->ops = &mock_context_ops; + } + + return ce; } static int mock_request_alloc(struct i915_request *request) @@ -185,7 +197,6 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, engine->base.status_page.page_addr = (void *)(engine + 1); engine->base.context_pin = mock_context_pin; - engine->base.context_unpin = mock_context_unpin; engine->base.request_alloc = mock_request_alloc; engine->base.emit_flush = mock_emit_flush; engine->base.emit_breadcrumb = mock_emit_breadcrumb; @@ -238,11 +249,13 @@ void mock_engine_free(struct intel_engine_cs *engine) { struct mock_engine *mock = container_of(engine, typeof(*mock), base); + struct intel_context *ce; GEM_BUG_ON(timer_pending(&mock->hw_delay)); - if (engine->last_retired_context) - intel_context_unpin(engine->last_retired_context, engine); + ce = fetch_and_zero(&engine->last_retired_context); + if (ce) + intel_context_unpin(ce); mock_ring_free(engine->buffer); -- cgit v1.2.3-58-ga151 From 867985d4a4319819a971645a262f3340f7d30343 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 17 May 2018 22:26:33 +0100 Subject: drm/i915: Pull the context->pin_count dec into the common intel_context_unpin As all backends implement the same pin_count mechanism and do a dec-and-test as their first step, pull that into the common intel_context_unpin(). This also pulls into the caller, eliminating the indirect call in the usual steady state case. The intel_context_pin() side is a little more complicated as it combines the lookup/alloc as well as pinning the state, and so is left for a later date. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-4-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_context.h | 4 ++++ drivers/gpu/drm/i915/intel_lrc.c | 13 +------------ drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ------ drivers/gpu/drm/i915/selftests/mock_engine.c | 3 --- 4 files changed, 5 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 749a4ff566f5..c3262b4dd2ee 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -285,6 +285,10 @@ static inline void __intel_context_pin(struct intel_context *ce) static inline void intel_context_unpin(struct intel_context *ce) { + GEM_BUG_ON(!ce->pin_count); + if (--ce->pin_count) + return; + GEM_BUG_ON(!ce->ops); ce->ops->unpin(ce); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b97c5d4c7877..9abd26f22b47 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1343,7 +1343,7 @@ static void execlists_context_destroy(struct intel_context *ce) __i915_gem_object_release_unless_active(ce->state->obj); } -static void __execlists_context_unpin(struct intel_context *ce) +static void execlists_context_unpin(struct intel_context *ce) { intel_ring_unpin(ce->ring); @@ -1354,17 +1354,6 @@ static void __execlists_context_unpin(struct intel_context *ce) i915_gem_context_put(ce->gem_context); } -static void execlists_context_unpin(struct intel_context *ce) -{ - lockdep_assert_held(&ce->gem_context->i915->drm.struct_mutex); - GEM_BUG_ON(ce->pin_count == 0); - - if (--ce->pin_count) - return; - - __execlists_context_unpin(ce); -} - static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma) { unsigned int flags; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0c0c9f531e4e..001cf6bcb349 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1195,12 +1195,6 @@ static void intel_ring_context_destroy(struct intel_context *ce) static void intel_ring_context_unpin(struct intel_context *ce) { - lockdep_assert_held(&ce->gem_context->i915->drm.struct_mutex); - GEM_BUG_ON(ce->pin_count == 0); - - if (--ce->pin_count) - return; - if (ce->state) { ce->state->obj->pin_global--; i915_vma_unpin(ce->state); diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index 33eddfc1f8ce..f1ac7453053e 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -74,9 +74,6 @@ static void hw_delay_complete(struct timer_list *t) static void mock_context_unpin(struct intel_context *ce) { - if (--ce->pin_count) - return; - i915_gem_context_put(ce->gem_context); } -- cgit v1.2.3-58-ga151 From 38992c57c9c8425dc9cb75efe6f9b9255ea627a0 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Thu, 26 Apr 2018 11:06:59 +0300 Subject: drm/panel: Remove drm_panel_detach() calls from all panel drivers Remove all drm_panel_detach() calls from all panel drivers and update the kerneldoc for drm_panel_detach(). Setting the connector and drm to NULL when the DRM panel device is going away hardly serves any purpose. Usually the whole memory structure is freed right after the remove call. However, calling the detach function from the master DRM device, and setting the connector pointer to NULL, has the logic of marking the panel again as available for another DRM master to attach. The usual situation would be the same DRM master device binding again. Signed-off-by: Jyri Sarha Reviewed-by: Daniel Vetter Signed-off-by: Thierry Reding Link: https://patchwork.freedesktop.org/patch/msgid/464b8d330d6b4c94cfb5aad2ca9ea7eb2c52d934.1524727888.git.jsarha@ti.com --- drivers/gpu/drm/drm_panel.c | 6 ++++++ drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 - drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 1 - drivers/gpu/drm/panel/panel-lvds.c | 1 - drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 1 - drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 - drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 - drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 - drivers/gpu/drm/panel/panel-simple.c | 1 - drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 1 - 10 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 308d442a531b..71e407574276 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -94,6 +94,9 @@ EXPORT_SYMBOL(drm_panel_remove); * * An error is returned if the panel is already attached to another connector. * + * When unloading, the driver should detach from the panel by calling + * drm_panel_detach(). + * * Return: 0 on success or a negative error code on failure. */ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector) @@ -115,6 +118,9 @@ EXPORT_SYMBOL(drm_panel_attach); * Detaches a panel from the connector it is attached to. If a panel is not * attached to any connector this is effectively a no-op. * + * This function should not be called by the panel device itself. It + * is only for the drm device that called drm_panel_attach(). + * * Return: 0 on success or a negative error code on failure. */ int drm_panel_detach(struct drm_panel *panel) diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c index 57df39b5c589..bb53e0850764 100644 --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c @@ -292,7 +292,6 @@ static int innolux_panel_remove(struct mipi_dsi_device *dsi) DRM_DEV_ERROR(&dsi->dev, "failed to detach from DSI host: %d\n", err); - drm_panel_detach(&innolux->base); innolux_panel_del(innolux); return 0; diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c index 0a94ab79a6c0..99caa7835e7b 100644 --- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c +++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c @@ -500,7 +500,6 @@ static int jdi_panel_remove(struct mipi_dsi_device *dsi) dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret); - drm_panel_detach(&jdi->base); jdi_panel_del(jdi); return 0; diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 5185819c5b79..8a1687887ae9 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -282,7 +282,6 @@ static int panel_lvds_remove(struct platform_device *pdev) { struct panel_lvds *lvds = dev_get_drvdata(&pdev->dev); - drm_panel_detach(&lvds->panel); drm_panel_remove(&lvds->panel); panel_lvds_disable(&lvds->panel); diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c index 74a806121f80..cb4dfb98be0f 100644 --- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c +++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c @@ -299,7 +299,6 @@ static int wuxga_nt_panel_remove(struct mipi_dsi_device *dsi) if (ret < 0) dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret); - drm_panel_detach(&wuxga_nt->base); wuxga_nt_panel_del(wuxga_nt); return 0; diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c index 71c09ed436ae..75f925390551 100644 --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c @@ -292,7 +292,6 @@ static int seiko_panel_remove(struct platform_device *pdev) { struct seiko_panel *panel = dev_get_drvdata(&pdev->dev); - drm_panel_detach(&panel->base); drm_panel_remove(&panel->base); seiko_panel_disable(&panel->base); diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c index 6bf8730f1a21..02fc0f5423d4 100644 --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c @@ -418,7 +418,6 @@ static int sharp_panel_remove(struct mipi_dsi_device *dsi) if (err < 0) dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err); - drm_panel_detach(&sharp->base); sharp_panel_del(sharp); return 0; diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c index 494aa9b1628a..e5cae0050f52 100644 --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c @@ -327,7 +327,6 @@ static int sharp_nt_panel_remove(struct mipi_dsi_device *dsi) if (ret < 0) dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret); - drm_panel_detach(&sharp_nt->base); sharp_nt_panel_del(sharp_nt); return 0; diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index cbf1ab404ee7..062b6b416b89 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -364,7 +364,6 @@ static int panel_simple_remove(struct device *dev) { struct panel_simple *panel = dev_get_drvdata(dev); - drm_panel_detach(&panel->base); drm_panel_remove(&panel->base); panel_simple_disable(&panel->base); diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c index 358c64ef1922..74284e5afc5d 100644 --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c @@ -419,7 +419,6 @@ static int st7789v_remove(struct spi_device *spi) { struct st7789v *ctx = spi_get_drvdata(spi); - drm_panel_detach(&ctx->panel); drm_panel_remove(&ctx->panel); if (ctx->backlight) -- cgit v1.2.3-58-ga151 From 0c08754b59da5557532d946599854e6df28edc22 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Thu, 26 Apr 2018 11:07:00 +0300 Subject: drm/panel: Add device_link from panel device to DRM device Add device_link from panel device (supplier) to DRM device (consumer) when drm_panel_attach() is called. This patch should protect the master DRM driver if an attached panel driver unbinds while it is in use. The device_link should make sure the DRM device is unbound before the panel driver becomes unavailable. The device_link is removed when drm_panel_detach() is called. The drm_panel_detach() should be called by the consumer DRM driver, not the panel driver, otherwise both drivers are racing to delete the same link. Signed-off-by: Jyri Sarha Reviewed-by: Eric Anholt Signed-off-by: Thierry Reding Link: https://patchwork.freedesktop.org/patch/msgid/b53584fd988d045c13de22d81825395b0ae0aad7.1524727888.git.jsarha@ti.com --- drivers/gpu/drm/drm_panel.c | 10 ++++++++++ include/drm/drm_panel.h | 1 + 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 71e407574276..965530a6f4cd 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -104,6 +105,13 @@ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector) if (panel->connector) return -EBUSY; + panel->link = device_link_add(connector->dev->dev, panel->dev, 0); + if (!panel->link) { + dev_err(panel->dev, "failed to link panel to %s\n", + dev_name(connector->dev->dev)); + return -EINVAL; + } + panel->connector = connector; panel->drm = connector->dev; @@ -125,6 +133,8 @@ EXPORT_SYMBOL(drm_panel_attach); */ int drm_panel_detach(struct drm_panel *panel) { + device_link_del(panel->link); + panel->connector = NULL; panel->drm = NULL; diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 14ac240a1f64..26a1b5fd8796 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -89,6 +89,7 @@ struct drm_panel { struct drm_device *drm; struct drm_connector *connector; struct device *dev; + struct device_link *link; const struct drm_panel_funcs *funcs; -- cgit v1.2.3-58-ga151 From d9f9565c1f7f51904a5009671595a50b8ab0462b Mon Sep 17 00:00:00 2001 From: Philippe CORNU Date: Mon, 23 Apr 2018 16:10:50 +0200 Subject: drm/panel: otm8009a: Fix backlight updates Backlight updates was not working anymore since the good implementation of the DSI low-power mode in the DSI host driver. After a longer analysis, the backlight updates in DSI video mode require the DSI high- speed mode. Note: it is important to keep the DSI low-power mode for the rest of the driver as init sequence, sleep in/out... DSI commands work in low-power mode. Signed-off-by: Philippe Cornu Signed-off-by: Thierry Reding Link: https://patchwork.freedesktop.org/patch/msgid/20180423141054.13128-2-philippe.cornu@st.com --- drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c index 90f1ae4af93c..0fd2e0144d2b 100644 --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c @@ -98,6 +98,20 @@ static void otm8009a_dcs_write_buf(struct otm8009a *ctx, const void *data, DRM_WARN("mipi dsi dcs write buffer failed\n"); } +static void otm8009a_dcs_write_buf_hs(struct otm8009a *ctx, const void *data, + size_t len) +{ + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + + /* data will be sent in dsi hs mode (ie. no lpm) */ + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + + otm8009a_dcs_write_buf(ctx, data, len); + + /* restore back the dsi lpm mode */ + dsi->mode_flags |= MIPI_DSI_MODE_LPM; +} + #define dcs_write_seq(ctx, seq...) \ ({ \ static const u8 d[] = { seq }; \ @@ -387,7 +401,7 @@ static int otm8009a_backlight_update_status(struct backlight_device *bd) */ data[0] = MIPI_DCS_SET_DISPLAY_BRIGHTNESS; data[1] = bd->props.brightness; - otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data)); + otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data)); /* set Brightness Control & Backlight on */ data[1] = 0x24; @@ -399,7 +413,7 @@ static int otm8009a_backlight_update_status(struct backlight_device *bd) /* Update Brightness Control & Backlight */ data[0] = MIPI_DCS_WRITE_CONTROL_DISPLAY; - otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data)); + otm8009a_dcs_write_buf_hs(ctx, data, ARRAY_SIZE(data)); return 0; } -- cgit v1.2.3-58-ga151 From 36830ce4eb2b5f6fe796c457b4043160c0e1e1fe Mon Sep 17 00:00:00 2001 From: Philippe CORNU Date: Mon, 23 Apr 2018 16:10:51 +0200 Subject: drm/panel: otm8009a: Fix glitches by moving backlight enable to otm8009a_enable() The backlight 1st update was in the otm8009a_prepare() function for a bad reason: backlight was not working in video mode and the otm8009a_prepare() is in command mode for the init sequence. As the backlight is now fixed (no low-power mode), it is good to put it back in the otm8009a_enable() function, avoiding also image glitches visible on some "slow" devices. Signed-off-by: Philippe Cornu Signed-off-by: Thierry Reding Link: https://patchwork.freedesktop.org/patch/msgid/20180423141054.13128-3-philippe.cornu@st.com --- drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c index 0fd2e0144d2b..de4a16d5275c 100644 --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c @@ -330,13 +330,6 @@ static int otm8009a_prepare(struct drm_panel *panel) ctx->prepared = true; - /* - * Power on the backlight. Note: end-user still controls brightness - * Note: ctx->prepared must be true before updating the backlight. - */ - ctx->bl_dev->props.power = FB_BLANK_UNBLANK; - backlight_update_status(ctx->bl_dev); - return 0; } @@ -344,6 +337,16 @@ static int otm8009a_enable(struct drm_panel *panel) { struct otm8009a *ctx = panel_to_otm8009a(panel); + if (ctx->enabled) + return 0; + + /* + * Power on the backlight. Note: end-user still controls brightness + * Note: ctx->prepared must be true before updating the backlight. + */ + ctx->bl_dev->props.power = FB_BLANK_UNBLANK; + backlight_update_status(ctx->bl_dev); + ctx->enabled = true; return 0; -- cgit v1.2.3-58-ga151 From 6982b943123d5867997a6d0f8b5976ac3ddb2fa6 Mon Sep 17 00:00:00 2001 From: Philippe CORNU Date: Mon, 23 Apr 2018 16:10:52 +0200 Subject: drm/panel: otm8009a: No message if probe success Remove the message in case of probe success. This comes from a suggestion followed in the recent integration of the raydium rm68200 panel. Signed-off-by: Philippe Cornu Signed-off-by: Thierry Reding Link: https://patchwork.freedesktop.org/patch/msgid/20180423141054.13128-4-philippe.cornu@st.com --- drivers/gpu/drm/panel/panel-orisetech-otm8009a.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c index de4a16d5275c..4c638b7b9943 100644 --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c @@ -14,8 +14,6 @@ #include #include