From d9df2c0943811f3880393be738ab86675029f3d0 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Wed, 27 Dec 2017 05:01:16 +0800 Subject: [PATCH 01/21] drm/i915/gvt: Fix gen8/9_render_mmio_list[0] don't take effect while(mmio++) increase mmio to next, mmio[0] never take effect in while loop. This patch change while to for and fix the above issue. v2: Correct Fixes format.(Zhenyu) v3: Rebase to latest staging.(Zhenyu) Fixes: 83164886e455("drm/i915/gvt: Select appropriate mmio list at initialization time") Signed-off-by: Xiong Zhang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/mmio_context.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c index 74834395dd89..48ee61ac9bc5 100644 --- a/drivers/gpu/drm/i915/gvt/mmio_context.c +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c @@ -80,7 +80,7 @@ static struct engine_mmio gen8_engine_mmio_list[] __cacheline_aligned = { {BCS, RING_INSTPM(BLT_RING_BASE), 0xffff, false}, /* 0x220c0 */ {BCS, RING_HWSTAM(BLT_RING_BASE), 0x0, false}, /* 0x22098 */ {BCS, RING_EXCC(BLT_RING_BASE), 0x0, false}, /* 0x22028 */ - { /* Terminated */ } + {RCS, INVALID_MMIO_REG, 0, false } /* Terminated */ }; static struct engine_mmio gen9_engine_mmio_list[] __cacheline_aligned = { @@ -146,7 +146,7 @@ static struct engine_mmio gen9_engine_mmio_list[] __cacheline_aligned = { {RCS, GEN8_GARBCNTL, 0x0, false}, /* 0xb004 */ {RCS, GEN7_FF_THREAD_MODE, 0x0, false}, /* 0x20a0 */ {RCS, FF_SLICE_CS_CHICKEN2, 0xffff, false}, /* 0x20e4 */ - { /* Terminated */ } + {RCS, INVALID_MMIO_REG, 0, false } /* Terminated */ }; static struct { @@ -310,8 +310,8 @@ static void switch_mmio(struct intel_vgpu *pre, if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) switch_mocs(pre, next, ring_id); - mmio = dev_priv->gvt->engine_mmio_list; - while (i915_mmio_reg_offset((mmio++)->reg)) { + for (mmio = dev_priv->gvt->engine_mmio_list; + i915_mmio_reg_valid(mmio->reg); mmio++) { if (mmio->ring_id != ring_id) continue; // save From 03fa9350c30b5db827e9b155715df09660ba28b1 Mon Sep 17 00:00:00 2001 From: Pei Zhang Date: Tue, 26 Dec 2017 14:31:25 +0800 Subject: [PATCH 02/21] drm/i915/gvt: add PLANE_KEYMAX regs to mmio track list Running 4.15 Linux kernel in VM will cause host GVT reports 'untrack mmio 0x701a0' errror, which identifies the PLANE_KEYMAX registers. Add them to track list. v2: rebase to latest staging code. Signed-off-by: Pei Zhang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/handlers.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 92d6468daeee..38f3b00d3a7a 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -2843,6 +2843,9 @@ static int init_skl_mmio_info(struct intel_gvt *gvt) MMIO_D(_MMIO(_PLANE_KEYVAL_1(PIPE_A)), D_SKL_PLUS); MMIO_D(_MMIO(_PLANE_KEYVAL_1(PIPE_B)), D_SKL_PLUS); MMIO_D(_MMIO(_PLANE_KEYVAL_1(PIPE_C)), D_SKL_PLUS); + MMIO_D(_MMIO(_PLANE_KEYMAX_1(PIPE_A)), D_SKL_PLUS); + MMIO_D(_MMIO(_PLANE_KEYMAX_1(PIPE_B)), D_SKL_PLUS); + MMIO_D(_MMIO(_PLANE_KEYMAX_1(PIPE_C)), D_SKL_PLUS); MMIO_D(_MMIO(_PLANE_KEYMSK_1(PIPE_A)), D_SKL_PLUS); MMIO_D(_MMIO(_PLANE_KEYMSK_1(PIPE_B)), D_SKL_PLUS); MMIO_D(_MMIO(_PLANE_KEYMSK_1(PIPE_C)), D_SKL_PLUS); From cc753fbe1ac47560e1517e3e11fb0c8a3c95eef5 Mon Sep 17 00:00:00 2001 From: Hang Yuan Date: Fri, 22 Dec 2017 18:06:31 +0800 Subject: [PATCH 03/21] drm/i915/gvt: validate gfn before set shadow page entry GVT may receive partial write on one guest PTE update. Validate gfn not to translate incomplete gfn. This avoids some unnecessary error messages incurred by the incomplete gfn translating. Also fix the bug that the whole PPGTT shadow page update is aborted on any invalid gfn entry. gfn validation relys on hypervisor's help. Add one MPT module function to provide the function. Signed-off-by: Hang Yuan Reviewed-by: Zhi Wang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/gtt.c | 24 +++++++++++++++++++----- drivers/gpu/drm/i915/gvt/hypercall.h | 1 + drivers/gpu/drm/i915/gvt/kvmgt.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/gvt/mpt.h | 17 +++++++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index a529d2bd393c..8d5317d0122d 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -997,9 +997,11 @@ static inline void ppgtt_generate_shadow_entry(struct intel_gvt_gtt_entry *se, static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt) { struct intel_vgpu *vgpu = spt->vgpu; + struct intel_gvt *gvt = vgpu->gvt; + struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops; struct intel_vgpu_ppgtt_spt *s; struct intel_gvt_gtt_entry se, ge; - unsigned long i; + unsigned long gfn, i; int ret; trace_spt_change(spt->vgpu->id, "born", spt, @@ -1007,9 +1009,10 @@ static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt) if (gtt_type_is_pte_pt(spt->shadow_page.type)) { for_each_present_guest_entry(spt, &ge, i) { - ret = gtt_entry_p2m(vgpu, &ge, &se); - if (ret) - goto fail; + gfn = ops->get_pfn(&ge); + if (!intel_gvt_hypervisor_is_valid_gfn(vgpu, gfn) || + gtt_entry_p2m(vgpu, &ge, &se)) + ops->set_pfn(&se, gvt->gtt.scratch_mfn); ppgtt_set_shadow_entry(spt, &se, i); } return 0; @@ -1906,7 +1909,7 @@ static int emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, struct intel_vgpu_mm *ggtt_mm = vgpu->gtt.ggtt_mm; struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops; unsigned long g_gtt_index = off >> info->gtt_entry_size_shift; - unsigned long gma; + unsigned long gma, gfn; struct intel_gvt_gtt_entry e, m; int ret; @@ -1925,6 +1928,16 @@ static int emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, bytes); if (ops->test_present(&e)) { + gfn = ops->get_pfn(&e); + + /* one PTE update may be issued in multiple writes and the + * first write may not construct a valid gfn + */ + if (!intel_gvt_hypervisor_is_valid_gfn(vgpu, gfn)) { + ops->set_pfn(&m, gvt->gtt.scratch_mfn); + goto out; + } + ret = gtt_entry_p2m(vgpu, &e, &m); if (ret) { gvt_vgpu_err("fail to translate guest gtt entry\n"); @@ -1939,6 +1952,7 @@ static int emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, ops->set_pfn(&m, gvt->gtt.scratch_mfn); } +out: ggtt_set_shadow_entry(ggtt_mm, &m, g_gtt_index); gtt_invalidate(gvt->dev_priv); ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index); diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h index a1bd82feb827..f8e77e166246 100644 --- a/drivers/gpu/drm/i915/gvt/hypercall.h +++ b/drivers/gpu/drm/i915/gvt/hypercall.h @@ -58,6 +58,7 @@ struct intel_gvt_mpt { int (*set_opregion)(void *vgpu); int (*get_vfio_device)(void *vgpu); void (*put_vfio_device)(void *vgpu); + bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn); }; extern struct intel_gvt_mpt xengt_mpt; diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 45bab5a6290b..eb92572056c3 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1570,6 +1570,21 @@ static unsigned long kvmgt_virt_to_pfn(void *addr) return PFN_DOWN(__pa(addr)); } +static bool kvmgt_is_valid_gfn(unsigned long handle, unsigned long gfn) +{ + struct kvmgt_guest_info *info; + struct kvm *kvm; + + if (!handle_valid(handle)) + return false; + + info = (struct kvmgt_guest_info *)handle; + kvm = info->kvm; + + return kvm_is_visible_gfn(kvm, gfn); + +} + struct intel_gvt_mpt kvmgt_mpt = { .host_init = kvmgt_host_init, .host_exit = kvmgt_host_exit, @@ -1585,6 +1600,7 @@ struct intel_gvt_mpt kvmgt_mpt = { .set_opregion = kvmgt_set_opregion, .get_vfio_device = kvmgt_get_vfio_device, .put_vfio_device = kvmgt_put_vfio_device, + .is_valid_gfn = kvmgt_is_valid_gfn, }; EXPORT_SYMBOL_GPL(kvmgt_mpt); diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h index ca8005a6d5fa..81aff4eacbfe 100644 --- a/drivers/gpu/drm/i915/gvt/mpt.h +++ b/drivers/gpu/drm/i915/gvt/mpt.h @@ -339,4 +339,21 @@ static inline void intel_gvt_hypervisor_put_vfio_device(struct intel_vgpu *vgpu) intel_gvt_host.mpt->put_vfio_device(vgpu); } +/** + * intel_gvt_hypervisor_is_valid_gfn - check if a visible gfn + * @vgpu: a vGPU + * @gfn: guest PFN + * + * Returns: + * true on valid gfn, false on not. + */ +static inline bool intel_gvt_hypervisor_is_valid_gfn( + struct intel_vgpu *vgpu, unsigned long gfn) +{ + if (!intel_gvt_host.mpt->is_valid_gfn) + return true; + + return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn); +} + #endif /* _GVT_MPT_H_ */ From 8466169ab9fe9f0c3412881116aca895feea6e70 Mon Sep 17 00:00:00 2001 From: Michel Thierry Date: Mon, 8 Jan 2018 10:37:34 -0800 Subject: [PATCH 04/21] drm/i915/gvt: Do not use I915_NUM_ENGINES to iterate over the mocs regs array The mocs reg array is defined locally but then we iterate over its elements using I915_NUM_ENGINES. There is no 'hard' connection between I915_NUM_ENGINES and the regs array and there will be problems if either of them increases. Use the size of the mocs reg array instead to safely iterate over it. Signed-off-by: Michel Thierry Cc: Weinan Li Cc: Zhenyu Wang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c index 48ee61ac9bc5..73ad6e90e49d 100644 --- a/drivers/gpu/drm/i915/gvt/mmio_context.c +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c @@ -167,7 +167,7 @@ static void load_render_mocs(struct drm_i915_private *dev_priv) }; int ring_id, i; - for (ring_id = 0; ring_id < I915_NUM_ENGINES; ring_id++) { + for (ring_id = 0; ring_id < ARRAY_SIZE(regs); ring_id++) { offset.reg = regs[ring_id]; for (i = 0; i < 64; i++) { gen9_render_mocs.control_table[ring_id][i] = From 2afba81c7909ac259720c0d3e7616cf54d4a5368 Mon Sep 17 00:00:00 2001 From: Anusha Srivatsa Date: Wed, 3 Jan 2018 11:03:45 -0800 Subject: [PATCH 05/21] drm/i915/glk: Disable Guc and HuC on GLK Since the firmwares are not yet released to public repo, disable them on Geminilake. v2: Remove the firmware versions (Michal) v3: Remove unwanted defines (Rodrigo) Correct commit message (Michal) Cc: Michal Wajdeczko Cc: Rodrigo Vivi Cc: Signed-off-by: Anusha Srivatsa Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") Reviewed-by: Michal Wajdeczko Signed-off-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/1515006225-13003-1-git-send-email-anusha.srivatsa@intel.com (cherry picked from commit a76050a4837860fcadb6ca11d69d41e08f4090d8) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_guc_fw.c | 9 --------- drivers/gpu/drm/i915/intel_huc.c | 11 ----------- 2 files changed, 20 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index cbc51c960425..3b0932942857 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -39,9 +39,6 @@ #define KBL_FW_MAJOR 9 #define KBL_FW_MINOR 39 -#define GLK_FW_MAJOR 10 -#define GLK_FW_MINOR 56 - #define GUC_FW_PATH(platform, major, minor) \ "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin" @@ -54,8 +51,6 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE); #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR) MODULE_FIRMWARE(I915_KBL_GUC_UCODE); -#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR) - static void guc_fw_select(struct intel_uc_fw *guc_fw) { struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); @@ -82,10 +77,6 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) guc_fw->path = I915_KBL_GUC_UCODE; guc_fw->major_ver_wanted = KBL_FW_MAJOR; guc_fw->minor_ver_wanted = KBL_FW_MINOR; - } else if (IS_GEMINILAKE(dev_priv)) { - guc_fw->path = I915_GLK_GUC_UCODE; - guc_fw->major_ver_wanted = GLK_FW_MAJOR; - guc_fw->minor_ver_wanted = GLK_FW_MINOR; } else { DRM_WARN("%s: No firmware known for this platform!\n", intel_uc_fw_type_repr(guc_fw->type)); diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index 974be3defa70..8ed05182f944 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -54,10 +54,6 @@ #define KBL_HUC_FW_MINOR 00 #define KBL_BLD_NUM 1810 -#define GLK_HUC_FW_MAJOR 02 -#define GLK_HUC_FW_MINOR 00 -#define GLK_BLD_NUM 1748 - #define HUC_FW_PATH(platform, major, minor, bld_num) \ "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ __stringify(minor) "_" __stringify(bld_num) ".bin" @@ -74,9 +70,6 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE); KBL_HUC_FW_MINOR, KBL_BLD_NUM) MODULE_FIRMWARE(I915_KBL_HUC_UCODE); -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \ - GLK_HUC_FW_MINOR, GLK_BLD_NUM) - static void huc_fw_select(struct intel_uc_fw *huc_fw) { struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); @@ -103,10 +96,6 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) huc_fw->path = I915_KBL_HUC_UCODE; huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; - } else if (IS_GEMINILAKE(dev_priv)) { - huc_fw->path = I915_GLK_HUC_UCODE; - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; } else { DRM_WARN("%s: No firmware known for this platform!\n", intel_uc_fw_type_repr(huc_fw->type)); From 90024a5951029685acc5396258f1b0de9b23cf4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Sun, 31 Dec 2017 23:34:54 +0100 Subject: [PATCH 06/21] drm/i915: Try EDID bitbanging on HDMI after failed read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ACK/NACK implementation as found in e.g. the G965 has the falling clock edge and the release of the data line after the ACK for the received byte happen at the same time. This is conformant with the I2C specification, which allows a zero hold time, see footnote [3]: "A device must internally provide a hold time of at least 300 ns for the SDA signal (with respect to the V IH(min) of the SCL signal) to bridge the undefined region of the falling edge of SCL." Some HDMI-to-VGA converters apparently fail to adhere to this requirement and latch SDA at the falling clock edge, so instead of an ACK sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the transfer. The bitbanging releases the data line for the ACK only 1/4 bit time after the falling clock edge, so a slave will see the correct value no matter if it samples at the rising or the falling clock edge or in the center. Fallback to bitbanging is already done for the CRT connector. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92685 Signed-off-by: Stefan Brüns Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/a39f080b-81a5-4c93-b3f7-7cb0a58daca3@rwthex-w2-a.rwth-ad.de (cherry picked from commit cfb926e148e99acc02351d72e8b85e32b5f786ef) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bced7b954d93..179d0ad3889d 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1595,12 +1595,20 @@ intel_hdmi_set_edid(struct drm_connector *connector) struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); struct edid *edid; bool connected = false; + struct i2c_adapter *i2c; intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - edid = drm_get_edid(connector, - intel_gmbus_get_adapter(dev_priv, - intel_hdmi->ddc_bus)); + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus); + + edid = drm_get_edid(connector, i2c); + + if (!edid && !intel_gmbus_is_forced_bit(i2c)) { + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n"); + intel_gmbus_force_bit(i2c, true); + edid = drm_get_edid(connector, i2c); + intel_gmbus_force_bit(i2c, false); + } intel_hdmi_dp_dual_mode_detect(connector, edid != NULL); From 6389902072db658366ec53711a83d711364b7330 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 9 Jan 2018 14:20:40 +0200 Subject: [PATCH 07/21] drm/i915: Fix using BIT_ULL() vs. BIT() for power domain masks The power domain masks are 64 bit wide, so we need BIT_ULL() when setting bits in them, these ones were missed during converting from 32 to 64 bit masks. All 3 enums are <32 atm, so this didn't cause a real problem. Fixes: d8fc70b7367b ("drm/i915: Make power domain masks 64 bit long") Cc: Joonas Lahtinen Reported-by: Fengguang Wu Signed-off-by: Imre Deak Reviewed-by: Chris Wilson Reviewed-by: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20180109122040.19425-1-imre.deak@intel.com (cherry picked from commit 17bd6e66d8f677b1b250d098097fbed6e70175ef) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0cd355978ab4..f288bcc7be22 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5661,8 +5661,8 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc, if (!crtc_state->base.active) return 0; - mask = BIT(POWER_DOMAIN_PIPE(pipe)); - mask |= BIT(POWER_DOMAIN_TRANSCODER(transcoder)); + mask = BIT_ULL(POWER_DOMAIN_PIPE(pipe)); + mask |= BIT_ULL(POWER_DOMAIN_TRANSCODER(transcoder)); if (crtc_state->pch_pfit.enabled || crtc_state->pch_pfit.force_thru) mask |= BIT_ULL(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe)); @@ -5674,7 +5674,7 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc, } if (HAS_DDI(dev_priv) && crtc_state->has_audio) - mask |= BIT(POWER_DOMAIN_AUDIO); + mask |= BIT_ULL(POWER_DOMAIN_AUDIO); if (crtc_state->shared_dpll) mask |= BIT_ULL(POWER_DOMAIN_PLLS); From c950af50e5983a119bebedceed94ef505df8d552 Mon Sep 17 00:00:00 2001 From: Sagar Arun Kamble Date: Wed, 10 Jan 2018 18:24:16 +0530 Subject: [PATCH 08/21] drm/i915/guc: Add uc_fini_wq in gem_init unwind path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While moving code around for solving lockdep issue for GuC log relay, spotted that uc_fini_wq is not being called in failure path in gem_init. Missed in the below commit. Add it. v2: Removed GEM_BUG_ON(!HAS_GUC()) from intel_uc_fini_wq as init happens only based on enable_guc module parameter and does not consider has_guc capability. (Michal) Signed-off-by: Sagar Arun Kamble Fixes: 3176ff49bc3e ("drm/i915/guc: Move GuC workqueue allocations outside of the mutex") Cc: Michał Winiarski Cc: Chris Wilson Reviewed-by: Michał Winiarski Link: https://patchwork.freedesktop.org/patch/msgid/1515588857-10283-1-git-send-email-sagar.a.kamble@intel.com Signed-off-by: Chris Wilson (cherry picked from commit da943b5ab071584fcb9cfa896dc8c643d376f362) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/intel_uc.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8bc3283484be..1135a77b383a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5283,6 +5283,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv) intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); mutex_unlock(&dev_priv->drm.struct_mutex); + intel_uc_fini_wq(dev_priv); + if (ret != -EIO) i915_gem_cleanup_userptr(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 907deac6e3fa..d82ca0f438f5 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -209,8 +209,6 @@ void intel_uc_fini_wq(struct drm_i915_private *dev_priv) if (!USES_GUC(dev_priv)) return; - GEM_BUG_ON(!HAS_GUC(dev_priv)); - intel_guc_fini_wq(&dev_priv->guc); } From 25da77f83068bfc95d08ddfb8e6c53983901e278 Mon Sep 17 00:00:00 2001 From: Oscar Mateo Date: Fri, 22 Dec 2017 14:38:49 -0800 Subject: [PATCH 09/21] drm/i915: Stop getting the fault address from RING_FAULT_REG This register does not contain it. Instead, we have to look into FAULT_TLB_DATA0 & 1 (where, by the way, we can also get the address space). v2: Right formatting v3: - Use 12 (as per the register format) instead of PAGE_SIZE (Chris) - s/BITS_44_TO_47/HIGHBITS (Chris) - Right formatting, this time for real Fixes: b03ec3d67ab8 ("drm/i915: There is only one fault register from GEN8 onwards") Signed-off-by: Oscar Mateo Cc: Michel Thierry Cc: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/1513982329-32191-1-git-send-email-oscar.mateo@intel.com Reviewed-by: Michel Thierry Signed-off-by: Chris Wilson (cherry picked from commit 5a3f58dfd1420347be838546e00a4a9e847bda30) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem_gtt.c | 15 +++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index c5f393870532..0de4f3f13fc4 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2287,12 +2287,23 @@ static void gen8_check_and_clear_faults(struct drm_i915_private *dev_priv) u32 fault = I915_READ(GEN8_RING_FAULT_REG); if (fault & RING_FAULT_VALID) { + u32 fault_data0, fault_data1; + u64 fault_addr; + + fault_data0 = I915_READ(GEN8_FAULT_TLB_DATA0); + fault_data1 = I915_READ(GEN8_FAULT_TLB_DATA1); + fault_addr = ((u64)(fault_data1 & FAULT_VA_HIGH_BITS) << 44) | + ((u64)fault_data0 << 12); + DRM_DEBUG_DRIVER("Unexpected fault\n" - "\tAddr: 0x%08lx\n" + "\tAddr: 0x%08x_%08x\n" + "\tAddress space: %s\n" "\tEngine ID: %d\n" "\tSource ID: %d\n" "\tType: %d\n", - fault & PAGE_MASK, + upper_32_bits(fault_addr), + lower_32_bits(fault_addr), + fault_data1 & FAULT_GTT_SEL ? "GGTT" : "PPGTT", GEN8_RING_FAULT_ENGINE_ID(fault), RING_FAULT_SRCID(fault), RING_FAULT_FAULT_TYPE(fault)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 505c605eff98..a2108e35c599 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2489,6 +2489,8 @@ enum i915_power_well_id { #define GEN8_FAULT_TLB_DATA0 _MMIO(0x4b10) #define GEN8_FAULT_TLB_DATA1 _MMIO(0x4b14) +#define FAULT_VA_HIGH_BITS (0xf << 0) +#define FAULT_GTT_SEL (1 << 4) #define FPGA_DBG _MMIO(0x42300) #define FPGA_DBG_RM_NOCLAIM (1<<31) From cc4f8fc72eb949889caa1d574d3db65fa9d71a12 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 11 Jan 2018 07:30:31 +0000 Subject: [PATCH 10/21] drm/i915/pmu: Reconstruct active state on starting busy-stats We have a hole in our busy-stat accounting if the pmu is enabled during a long running batch, the pmu will not start accumulating busy-time until the next context switch. This then fails tests that are only sampling a single batch. v2: Count each active port just once (context in/out events are only on the first and last assignment to a port). v3: Avoid hardcoding knowledge of 2 submission ports Fixes: 30e17b7847f5 ("drm/i915: Engine busy time tracking") Testcase: igt/perf_pmu/busy-start Testcase: igt/perf_pmu/busy-double-start Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20180111073031.14614-1-chris@chris-wilson.co.uk (cherry picked from commit 4900727d35bb20028f9bd83146ec4bf78afffe30) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_engine_cs.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 6bb51a502b8b..d790bdc227ff 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1951,8 +1951,22 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) spin_lock_irqsave(&engine->stats.lock, flags); if (engine->stats.enabled == ~0) goto busy; - if (engine->stats.enabled++ == 0) + if (engine->stats.enabled++ == 0) { + struct intel_engine_execlists *execlists = &engine->execlists; + const struct execlist_port *port = execlists->port; + unsigned int num_ports = execlists_num_ports(execlists); + engine->stats.enabled_at = ktime_get(); + + /* XXX submission method oblivious? */ + while (num_ports-- && port_isset(port)) { + engine->stats.active++; + port++; + } + + if (engine->stats.active) + engine->stats.start = engine->stats.enabled_at; + } spin_unlock_irqrestore(&engine->stats.lock, flags); return 0; From 412718a10926f77052d8107cb3c5d9dbc96cf8c9 Mon Sep 17 00:00:00 2001 From: Tina Zhang Date: Mon, 15 Jan 2018 14:41:42 +0800 Subject: [PATCH 11/21] drm/i915/gvt: Keep obj->dma_buf link NULL during exporting According to commit (319c933c71f3dbdb2b3274d1634d3494c70efa06) Author: Daniel Vetter Date: Thu Aug 15 00:02:46 2013 +0200 drm/prime: proper locking+refcounting for obj->dma_buf link obj->dma_buf link should be reinstated at import time. Gvt-g dma-buf buffer exposeing might be simpler, as there won't be much racing during Gvt-g dma-buf exposing. In other words, Gvt-g dma-buf exposing can guarantee exposing happens before gem close ioctl, and Gvt-g is the only exporter of the guest framebuffer. But following the drm prime scheme can give Gvt-g a chance to increase a dma-buf reference count during importing. Otherwise, we have to increase the reference during exposing, which will break the case that the only reference userspace has held was through the dma-buf fd and the reference count is one. Signed-off-by: Tina Zhang Cc: Daniel Vetter Cc: Zhenyu Wang Cc: Zhi Wang Cc: Hang Yuan Cc: Gerd Hoffmann Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/dmabuf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c index 2ab584f97dfb..2fb7b34ef561 100644 --- a/drivers/gpu/drm/i915/gvt/dmabuf.c +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c @@ -472,7 +472,6 @@ int intel_vgpu_get_dmabuf(struct intel_vgpu *vgpu, unsigned int dmabuf_id) ret = PTR_ERR(dmabuf); goto out_free_gem; } - obj->base.dma_buf = dmabuf; i915_gem_object_put(obj); From 14b4434bff1aab69f3785bc67276efbff7fffa3d Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Mon, 15 Jan 2018 16:36:11 +0800 Subject: [PATCH 12/21] drm/i915/gvt: cancel virtual vblank timer when no vGPU exists Stop irq timer for virtual vblank timer emulation if no vGPU exists, otherwise it will keep gvt service thread busy to handle virtual vblank but no use. Reviewed-by: Zhi Wang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index 4688619f6a1c..a8784fa91289 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -258,6 +258,8 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu) 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); From 61a669473f82cd5cac66146644b829b370c791ca Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Mon, 15 Jan 2018 16:36:12 +0800 Subject: [PATCH 13/21] drm/i915/gvt: cancel scheduler timer when no vGPU exists Stop gvt scheduler timer if no vGPU exists, otherwise it keeps gvt service thread busy to handle request schedule event but no actual schedule activity required. Reviewed-by: Zhi Wang Signed-off-by: Zhenyu Wang Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/gvt/sched_policy.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index eea1a2f92099..d031f6486ce3 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -308,8 +308,15 @@ static int tbs_sched_init_vgpu(struct intel_vgpu *vgpu) static void tbs_sched_clean_vgpu(struct intel_vgpu *vgpu) { + struct intel_gvt *gvt = vgpu->gvt; + struct gvt_sched_data *sched_data = gvt->scheduler.sched_data; + kfree(vgpu->sched_data); vgpu->sched_data = NULL; + + /* this vgpu id has been removed */ + if (idr_is_empty(&gvt->vgpu_idr)) + hrtimer_cancel(&sched_data->timer); } static void tbs_sched_start_schedule(struct intel_vgpu *vgpu) From d13a8479f3584613b6aacbb793eae64578b8f69a Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Tue, 16 Jan 2018 16:53:24 +0100 Subject: [PATCH 14/21] drm/i915: Always call to intel_display_set_init_power() in resume_early. intel_power_domains_init_hw() calls set_init_power, but when using runtime power management this call is skipped. This prevents hw readout from taking place. Signed-off-by: Maarten Lankhorst Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172 Link: https://patchwork.freedesktop.org/patch/msgid/20180116155324.75120-1-maarten.lankhorst@linux.intel.com Fixes: bc87229f323e ("drm/i915/skl: enable PC9/10 power states during suspend-to-idle") Cc: Nivedita Swaminathan Cc: Imre Deak Cc: Patrik Jakobsson Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: # v4.5+ Reviewed-by: Imre Deak (cherry picked from commit ac25dfed15d470d7f23dd817e965b54aa3f94a1e) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c8da9d20c33..173d0095e3b2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1842,6 +1842,8 @@ static int i915_drm_resume_early(struct drm_device *dev) if (IS_GEN9_LP(dev_priv) || !(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload)) intel_power_domains_init_hw(dev_priv, true); + else + intel_display_set_init_power(dev_priv, true); i915_gem_sanitize(dev_priv); From c5bd1fc9a6c843c85a5cea5765cdc997f832df3c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 15 Jan 2018 21:24:46 +0000 Subject: [PATCH 15/21] drm/i915: Only attempt to scan the requested number of shrinker slabs Since commit 4e773c3a8a69 ("drm/i915: Wire up shrinkctl->nr_scanned"), we track the number of objects we scan and do not wish to exceed that as it will overly penalise our own slabs under mempressure. Given that we now know the target number of objects to scan, use that as our guide for deciding to shrink as opposed to the number of objects we manage to shrink (which doesn't correspond to the numbers we report to shrinkctl). Fixes: 4e773c3a8a69 ("drm/i915: Wire up shrinkctl->nr_scanned") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20180115212455.24046-2-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin (cherry picked from commit 29d384e34c55d696cf37bd4159e05f4b14d45da0) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 9029ed04879c..0e158f9287c4 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -363,13 +363,13 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE); - if (freed < sc->nr_to_scan) + if (sc->nr_scanned < sc->nr_to_scan) freed += i915_gem_shrink(i915, sc->nr_to_scan - sc->nr_scanned, &sc->nr_scanned, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND); - if (freed < sc->nr_to_scan && current_is_kswapd()) { + if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) { intel_runtime_pm_get(i915); freed += i915_gem_shrink(i915, sc->nr_to_scan - sc->nr_scanned, From 124804c4c4b2e84e956cedd17878562e230a907f Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 21 Jan 2018 17:31:43 +0000 Subject: [PATCH 16/21] drm/i915: Protect WC stash allocation against direct reclaim As we attempt to allocate pages for use in a new WC stash, direct reclaim may run underneath us and fill up the WC stash. We have to be careful then not to overflow the pvec. Fixes: 66df1014efba ("drm/i915: Keep a small stash of preallocated WC pages") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103109 Signed-off-by: Chris Wilson Cc: Matthew Auld Cc: Joonas Lahtinen Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20180121173143.17090-1-chris@chris-wilson.co.uk (cherry picked from commit 073cd7816685ac77c6d8b4d321a5586c9177b76a) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem_gtt.c | 32 ++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0de4f3f13fc4..133cb9590003 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -377,6 +377,7 @@ static gen6_pte_t iris_pte_encode(dma_addr_t addr, static struct page *vm_alloc_page(struct i915_address_space *vm, gfp_t gfp) { struct pagevec *pvec = &vm->free_pages; + struct pagevec stash; if (I915_SELFTEST_ONLY(should_fail(&vm->fault_attr, 1))) i915_gem_shrink_all(vm->i915); @@ -395,7 +396,15 @@ static struct page *vm_alloc_page(struct i915_address_space *vm, gfp_t gfp) if (likely(pvec->nr)) return pvec->pages[--pvec->nr]; - /* Otherwise batch allocate pages to amoritize cost of set_pages_wc. */ + /* + * Otherwise batch allocate pages to amoritize cost of set_pages_wc. + * + * We have to be careful as page allocation may trigger the shrinker + * (via direct reclaim) which will fill up the WC stash underneath us. + * So we add our WB pages into a temporary pvec on the stack and merge + * them into the WC stash after all the allocations are complete. + */ + pagevec_init(&stash); do { struct page *page; @@ -403,15 +412,24 @@ static struct page *vm_alloc_page(struct i915_address_space *vm, gfp_t gfp) if (unlikely(!page)) break; - pvec->pages[pvec->nr++] = page; - } while (pagevec_space(pvec)); + stash.pages[stash.nr++] = page; + } while (stash.nr < pagevec_space(pvec)); - if (unlikely(!pvec->nr)) - return NULL; + if (stash.nr) { + int nr = min_t(int, stash.nr, pagevec_space(pvec)); + struct page **pages = stash.pages + stash.nr - nr; - set_pages_array_wc(pvec->pages, pvec->nr); + if (nr && !set_pages_array_wc(pages, nr)) { + memcpy(pvec->pages + pvec->nr, + pages, sizeof(pages[0]) * nr); + pvec->nr += nr; + stash.nr -= nr; + } - return pvec->pages[--pvec->nr]; + pagevec_release(&stash); + } + + return likely(pvec->nr) ? pvec->pages[--pvec->nr] : NULL; } static void vm_free_pages_release(struct i915_address_space *vm, From 9965db26ac0548648309f506dc155a92daa2158f Mon Sep 17 00:00:00 2001 From: Mika Kahola Date: Mon, 18 Dec 2017 10:04:03 +0200 Subject: [PATCH 17/21] drm/i915: Check for fused or unused pipes We may have fused or unused pipes in our system. Let's check that the pipe in question is within limits of accessible pipes. In case, that we are not able to access the pipe, we return early with a warning. v2: Rephrasing of the commit message (Jani) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103206 Reported-by: Thomas Gleixner Tested-by: Jaswinder Singh Rajput Suggested-by: Jani Nikula Reviewed-by: Jani Nikula Signed-off-by: Mika Kahola Signed-off-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/1513584243-12607-1-git-send-email-mika.kahola@intel.com (cherry picked from commit 0b7029b7e43fda1304c181a3ade0b429b9edcd9d) Signed-off-by: Rodrigo Vivi Cc: # v4.10+ --- drivers/gpu/drm/i915/intel_audio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index f1502a0188eb..522d54fecb53 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -779,7 +779,7 @@ static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv, { struct intel_encoder *encoder; - if (WARN_ON(pipe >= INTEL_INFO(dev_priv)->num_pipes)) + if (WARN_ON(pipe >= ARRAY_SIZE(dev_priv->av_enc_map))) return NULL; /* MST */ From a306343bcd7df89d9d45a601929e26866e7b7a81 Mon Sep 17 00:00:00 2001 From: Manasi Navare Date: Thu, 12 Oct 2017 12:13:38 -0700 Subject: [PATCH 18/21] drm/i915/edp: Do not do link training fallback or prune modes on EDP In case of eDP because the panel has a fixed mode, the link rate and lane count at which it is trained corresponds to the link BW required to support the native resolution of the panel. In case of panles with lower resolutions where fewer lanes are hooked up internally, that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. So it is pointless to fallback to lower link rate/lane count in case of link training failure on eDP connector since the lower link BW will not support the native resolution of the panel and we cannot prune the preferred mode on the eDP connector. In case of Link training failure on the eDP panel, something is wrong in the HW internally and hence driver errors out with a loud and clear DRM_ERROR message. v2: * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) Cc: Clinton Taylor Cc: Jim Bride Cc: Jani Nikula Cc: Ville Syrjala Cc: Dave Airlie Cc: Daniel Vetter Signed-off-by: Manasi Navare Reviewed-by: Ville Syrjala Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103369 Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/1507835618-23051-1-git-send-email-manasi.d.navare@intel.com (cherry picked from commit c0cfb10d9e1de490e36d3b9d4228c0ea0ca30677) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_dp_link_training.c | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 05907fa8a553..cf8fef8b6f58 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) return; failure_handling: - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", - intel_connector->base.base.id, - intel_connector->base.name, - intel_dp->link_rate, intel_dp->lane_count); - if (!intel_dp_get_link_train_fallback_values(intel_dp, - intel_dp->link_rate, - intel_dp->lane_count)) - /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); + /* Dont fallback and prune modes if its eDP */ + if (!intel_dp_is_edp(intel_dp)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", + intel_connector->base.base.id, + intel_connector->base.name, + intel_dp->link_rate, intel_dp->lane_count); + if (!intel_dp_get_link_train_fallback_values(intel_dp, + intel_dp->link_rate, + intel_dp->lane_count)) + /* Schedule a Hotplug Uevent to userspace to start modeset */ + schedule_work(&intel_connector->modeset_retry_work); + } else { + DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", + intel_connector->base.base.id, + intel_connector->base.name, + intel_dp->link_rate, intel_dp->lane_count); + } return; } From b5a756a722286af9702d565501e1f690d075d16b Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Mon, 29 Jan 2018 08:33:46 +0000 Subject: [PATCH 19/21] Revert "drm/i915: mark all device info struct with __initconst" This reverts commit 5b54eddd3920e9f6f1a6d972454baf350cbae77e. Conflicts: drivers/gpu/drm/i915/i915_pci.c Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104805 Signed-off-by: Lionel Landwerlin Reviewed-by: Chris Wilson Fixes: 5b54eddd3920 ("drm/i915: mark all device info struct with __initconst") Link: https://patchwork.freedesktop.org/patch/msgid/20180129083346.29173-1-lionel.g.landwerlin@intel.com (cherry picked from commit 5db47e37b38755c5e26e6b8fbc1a32ce73495940) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_pci.c | 94 ++++++++++++++++----------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 36d48422b475..1c30c688f23a 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -74,19 +74,19 @@ GEN_DEFAULT_PAGE_SIZES, \ CURSOR_OFFSETS -static const struct intel_device_info intel_i830_info __initconst = { +static const struct intel_device_info intel_i830_info = { GEN2_FEATURES, .platform = INTEL_I830, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, /* legal, last one wins */ }; -static const struct intel_device_info intel_i845g_info __initconst = { +static const struct intel_device_info intel_i845g_info = { GEN2_FEATURES, .platform = INTEL_I845G, }; -static const struct intel_device_info intel_i85x_info __initconst = { +static const struct intel_device_info intel_i85x_info = { GEN2_FEATURES, .platform = INTEL_I85X, .is_mobile = 1, .num_pipes = 2, /* legal, last one wins */ @@ -94,7 +94,7 @@ static const struct intel_device_info intel_i85x_info __initconst = { .has_fbc = 1, }; -static const struct intel_device_info intel_i865g_info __initconst = { +static const struct intel_device_info intel_i865g_info = { GEN2_FEATURES, .platform = INTEL_I865G, }; @@ -108,7 +108,7 @@ static const struct intel_device_info intel_i865g_info __initconst = { GEN_DEFAULT_PAGE_SIZES, \ CURSOR_OFFSETS -static const struct intel_device_info intel_i915g_info __initconst = { +static const struct intel_device_info intel_i915g_info = { GEN3_FEATURES, .platform = INTEL_I915G, .cursor_needs_physical = 1, .has_overlay = 1, .overlay_needs_physical = 1, @@ -116,7 +116,7 @@ static const struct intel_device_info intel_i915g_info __initconst = { .unfenced_needs_alignment = 1, }; -static const struct intel_device_info intel_i915gm_info __initconst = { +static const struct intel_device_info intel_i915gm_info = { GEN3_FEATURES, .platform = INTEL_I915GM, .is_mobile = 1, @@ -128,7 +128,7 @@ static const struct intel_device_info intel_i915gm_info __initconst = { .unfenced_needs_alignment = 1, }; -static const struct intel_device_info intel_i945g_info __initconst = { +static const struct intel_device_info intel_i945g_info = { GEN3_FEATURES, .platform = INTEL_I945G, .has_hotplug = 1, .cursor_needs_physical = 1, @@ -137,7 +137,7 @@ static const struct intel_device_info intel_i945g_info __initconst = { .unfenced_needs_alignment = 1, }; -static const struct intel_device_info intel_i945gm_info __initconst = { +static const struct intel_device_info intel_i945gm_info = { GEN3_FEATURES, .platform = INTEL_I945GM, .is_mobile = 1, .has_hotplug = 1, .cursor_needs_physical = 1, @@ -148,14 +148,14 @@ static const struct intel_device_info intel_i945gm_info __initconst = { .unfenced_needs_alignment = 1, }; -static const struct intel_device_info intel_g33_info __initconst = { +static const struct intel_device_info intel_g33_info = { GEN3_FEATURES, .platform = INTEL_G33, .has_hotplug = 1, .has_overlay = 1, }; -static const struct intel_device_info intel_pineview_info __initconst = { +static const struct intel_device_info intel_pineview_info = { GEN3_FEATURES, .platform = INTEL_PINEVIEW, .is_mobile = 1, .has_hotplug = 1, @@ -172,7 +172,7 @@ static const struct intel_device_info intel_pineview_info __initconst = { GEN_DEFAULT_PAGE_SIZES, \ CURSOR_OFFSETS -static const struct intel_device_info intel_i965g_info __initconst = { +static const struct intel_device_info intel_i965g_info = { GEN4_FEATURES, .platform = INTEL_I965G, .has_overlay = 1, @@ -180,7 +180,7 @@ static const struct intel_device_info intel_i965g_info __initconst = { .has_snoop = false, }; -static const struct intel_device_info intel_i965gm_info __initconst = { +static const struct intel_device_info intel_i965gm_info = { GEN4_FEATURES, .platform = INTEL_I965GM, .is_mobile = 1, .has_fbc = 1, @@ -190,13 +190,13 @@ static const struct intel_device_info intel_i965gm_info __initconst = { .has_snoop = false, }; -static const struct intel_device_info intel_g45_info __initconst = { +static const struct intel_device_info intel_g45_info = { GEN4_FEATURES, .platform = INTEL_G45, .ring_mask = RENDER_RING | BSD_RING, }; -static const struct intel_device_info intel_gm45_info __initconst = { +static const struct intel_device_info intel_gm45_info = { GEN4_FEATURES, .platform = INTEL_GM45, .is_mobile = 1, .has_fbc = 1, @@ -215,12 +215,12 @@ static const struct intel_device_info intel_gm45_info __initconst = { GEN_DEFAULT_PAGE_SIZES, \ CURSOR_OFFSETS -static const struct intel_device_info intel_ironlake_d_info __initconst = { +static const struct intel_device_info intel_ironlake_d_info = { GEN5_FEATURES, .platform = INTEL_IRONLAKE, }; -static const struct intel_device_info intel_ironlake_m_info __initconst = { +static const struct intel_device_info intel_ironlake_m_info = { GEN5_FEATURES, .platform = INTEL_IRONLAKE, .is_mobile = 1, .has_fbc = 1, @@ -243,12 +243,12 @@ static const struct intel_device_info intel_ironlake_m_info __initconst = { GEN6_FEATURES, \ .platform = INTEL_SANDYBRIDGE -static const struct intel_device_info intel_sandybridge_d_gt1_info __initconst = { +static const struct intel_device_info intel_sandybridge_d_gt1_info = { SNB_D_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_sandybridge_d_gt2_info __initconst = { +static const struct intel_device_info intel_sandybridge_d_gt2_info = { SNB_D_PLATFORM, .gt = 2, }; @@ -259,12 +259,12 @@ static const struct intel_device_info intel_sandybridge_d_gt2_info __initconst = .is_mobile = 1 -static const struct intel_device_info intel_sandybridge_m_gt1_info __initconst = { +static const struct intel_device_info intel_sandybridge_m_gt1_info = { SNB_M_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_sandybridge_m_gt2_info __initconst = { +static const struct intel_device_info intel_sandybridge_m_gt2_info = { SNB_M_PLATFORM, .gt = 2, }; @@ -288,12 +288,12 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info __initconst = .platform = INTEL_IVYBRIDGE, \ .has_l3_dpf = 1 -static const struct intel_device_info intel_ivybridge_d_gt1_info __initconst = { +static const struct intel_device_info intel_ivybridge_d_gt1_info = { IVB_D_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_ivybridge_d_gt2_info __initconst = { +static const struct intel_device_info intel_ivybridge_d_gt2_info = { IVB_D_PLATFORM, .gt = 2, }; @@ -304,17 +304,17 @@ static const struct intel_device_info intel_ivybridge_d_gt2_info __initconst = { .is_mobile = 1, \ .has_l3_dpf = 1 -static const struct intel_device_info intel_ivybridge_m_gt1_info __initconst = { +static const struct intel_device_info intel_ivybridge_m_gt1_info = { IVB_M_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_ivybridge_m_gt2_info __initconst = { +static const struct intel_device_info intel_ivybridge_m_gt2_info = { IVB_M_PLATFORM, .gt = 2, }; -static const struct intel_device_info intel_ivybridge_q_info __initconst = { +static const struct intel_device_info intel_ivybridge_q_info = { GEN7_FEATURES, .platform = INTEL_IVYBRIDGE, .gt = 2, @@ -322,7 +322,7 @@ static const struct intel_device_info intel_ivybridge_q_info __initconst = { .has_l3_dpf = 1, }; -static const struct intel_device_info intel_valleyview_info __initconst = { +static const struct intel_device_info intel_valleyview_info = { .platform = INTEL_VALLEYVIEW, .gen = 7, .is_lp = 1, @@ -358,17 +358,17 @@ static const struct intel_device_info intel_valleyview_info __initconst = { .platform = INTEL_HASWELL, \ .has_l3_dpf = 1 -static const struct intel_device_info intel_haswell_gt1_info __initconst = { +static const struct intel_device_info intel_haswell_gt1_info = { HSW_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_haswell_gt2_info __initconst = { +static const struct intel_device_info intel_haswell_gt2_info = { HSW_PLATFORM, .gt = 2, }; -static const struct intel_device_info intel_haswell_gt3_info __initconst = { +static const struct intel_device_info intel_haswell_gt3_info = { HSW_PLATFORM, .gt = 3, }; @@ -388,17 +388,17 @@ static const struct intel_device_info intel_haswell_gt3_info __initconst = { .gen = 8, \ .platform = INTEL_BROADWELL -static const struct intel_device_info intel_broadwell_gt1_info __initconst = { +static const struct intel_device_info intel_broadwell_gt1_info = { BDW_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_broadwell_gt2_info __initconst = { +static const struct intel_device_info intel_broadwell_gt2_info = { BDW_PLATFORM, .gt = 2, }; -static const struct intel_device_info intel_broadwell_rsvd_info __initconst = { +static const struct intel_device_info intel_broadwell_rsvd_info = { BDW_PLATFORM, .gt = 3, /* According to the device ID those devices are GT3, they were @@ -406,13 +406,13 @@ static const struct intel_device_info intel_broadwell_rsvd_info __initconst = { */ }; -static const struct intel_device_info intel_broadwell_gt3_info __initconst = { +static const struct intel_device_info intel_broadwell_gt3_info = { BDW_PLATFORM, .gt = 3, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, }; -static const struct intel_device_info intel_cherryview_info __initconst = { +static const struct intel_device_info intel_cherryview_info = { .gen = 8, .num_pipes = 3, .has_hotplug = 1, .is_lp = 1, @@ -455,12 +455,12 @@ static const struct intel_device_info intel_cherryview_info __initconst = { .gen = 9, \ .platform = INTEL_SKYLAKE -static const struct intel_device_info intel_skylake_gt1_info __initconst = { +static const struct intel_device_info intel_skylake_gt1_info = { SKL_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_skylake_gt2_info __initconst = { +static const struct intel_device_info intel_skylake_gt2_info = { SKL_PLATFORM, .gt = 2, }; @@ -470,12 +470,12 @@ static const struct intel_device_info intel_skylake_gt2_info __initconst = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING -static const struct intel_device_info intel_skylake_gt3_info __initconst = { +static const struct intel_device_info intel_skylake_gt3_info = { SKL_GT3_PLUS_PLATFORM, .gt = 3, }; -static const struct intel_device_info intel_skylake_gt4_info __initconst = { +static const struct intel_device_info intel_skylake_gt4_info = { SKL_GT3_PLUS_PLATFORM, .gt = 4, }; @@ -511,13 +511,13 @@ static const struct intel_device_info intel_skylake_gt4_info __initconst = { IVB_CURSOR_OFFSETS, \ BDW_COLORS -static const struct intel_device_info intel_broxton_info __initconst = { +static const struct intel_device_info intel_broxton_info = { GEN9_LP_FEATURES, .platform = INTEL_BROXTON, .ddb_size = 512, }; -static const struct intel_device_info intel_geminilake_info __initconst = { +static const struct intel_device_info intel_geminilake_info = { GEN9_LP_FEATURES, .platform = INTEL_GEMINILAKE, .ddb_size = 1024, @@ -529,17 +529,17 @@ static const struct intel_device_info intel_geminilake_info __initconst = { .gen = 9, \ .platform = INTEL_KABYLAKE -static const struct intel_device_info intel_kabylake_gt1_info __initconst = { +static const struct intel_device_info intel_kabylake_gt1_info = { KBL_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_kabylake_gt2_info __initconst = { +static const struct intel_device_info intel_kabylake_gt2_info = { KBL_PLATFORM, .gt = 2, }; -static const struct intel_device_info intel_kabylake_gt3_info __initconst = { +static const struct intel_device_info intel_kabylake_gt3_info = { KBL_PLATFORM, .gt = 3, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, @@ -550,17 +550,17 @@ static const struct intel_device_info intel_kabylake_gt3_info __initconst = { .gen = 9, \ .platform = INTEL_COFFEELAKE -static const struct intel_device_info intel_coffeelake_gt1_info __initconst = { +static const struct intel_device_info intel_coffeelake_gt1_info = { CFL_PLATFORM, .gt = 1, }; -static const struct intel_device_info intel_coffeelake_gt2_info __initconst = { +static const struct intel_device_info intel_coffeelake_gt2_info = { CFL_PLATFORM, .gt = 2, }; -static const struct intel_device_info intel_coffeelake_gt3_info __initconst = { +static const struct intel_device_info intel_coffeelake_gt3_info = { CFL_PLATFORM, .gt = 3, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = { .ddb_size = 1024, \ GLK_COLORS -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = { +static const struct intel_device_info intel_cannonlake_gt2_info = { GEN10_FEATURES, .is_alpha_support = 1, .platform = INTEL_CANNONLAKE, From b26a32a82a901f894c79d1ec8b1ac94dde83e83c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 29 Jan 2018 14:41:04 +0000 Subject: [PATCH 20/21] drm/i915: Always run hangcheck while the GPU is busy Previously, we relied on only running the hangcheck while somebody was waiting on the GPU, in order to minimise the amount of time hangcheck had to run. (If nobody was watching the GPU, nobody would notice if the GPU wasn't responding -- eventually somebody would care and so kick hangcheck into action.) However, this falls apart from around commit 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion"), as not all waiters declare themselves to hangcheck and so we could switch off hangcheck and miss GPU hangs even when waiting under the struct_mutex. If we enable hangcheck from the first request submission, and let it run until the GPU is idle again, we forgo all the complexity involved with only enabling around waiters. We just have to remember to be careful that we do not declare a GPU hang when idly waiting for the next request to be come ready, as we will run hangcheck continuously even when the engines are stalled waiting for external events. This should be true already as we should only be tracking requests submitted to hardware for execution as an indicator that the engine is busy. Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for request completion" Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104840 Signed-off-by: Chris Wilson Cc: Chris Wilson Cc: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20180129144104.3921-1-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala (cherry picked from commit 889230489b6b138ba97ba2f13fc9644a3d16d0d2) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem.c | 7 +++---- drivers/gpu/drm/i915/i915_gem_request.c | 2 ++ drivers/gpu/drm/i915/intel_breadcrumbs.c | 11 ----------- drivers/gpu/drm/i915/intel_hangcheck.c | 7 +------ 4 files changed, 6 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1135a77b383a..dd89abd2263d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3323,16 +3323,15 @@ i915_gem_retire_work_handler(struct work_struct *work) mutex_unlock(&dev->struct_mutex); } - /* Keep the retire handler running until we are finally idle. + /* + * Keep the retire handler running until we are finally idle. * We do not need to do this test under locking as in the worst-case * we queue the retire worker once too often. */ - if (READ_ONCE(dev_priv->gt.awake)) { - i915_queue_hangcheck(dev_priv); + if (READ_ONCE(dev_priv->gt.awake)) queue_delayed_work(dev_priv->wq, &dev_priv->gt.retire_work, round_jiffies_up_relative(HZ)); - } } static inline bool diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index d575109f7a7f..e09d18df8b7f 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -276,6 +276,8 @@ static void mark_busy(struct drm_i915_private *i915) intel_engines_unpark(i915); + i915_queue_hangcheck(i915); + queue_delayed_work(i915->wq, &i915->gt.retire_work, round_jiffies_up_relative(HZ)); diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 58c624f982d9..bd40fea16b4f 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -149,17 +149,6 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t) return; mod_timer(&b->fake_irq, jiffies + 1); - - /* Ensure that even if the GPU hangs, we get woken up. - * - * However, note that if no one is waiting, we never notice - * a gpu hang. Eventually, we will have to wait for a resource - * held by the GPU and so trigger a hangcheck. In the most - * pathological case, this will be upon memory starvation! To - * prevent this, we also queue the hangcheck from the retire - * worker. - */ - i915_queue_hangcheck(engine->i915); } static void irq_enable(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 31f01d64c021..348a4f7ffb67 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -411,7 +411,6 @@ static void i915_hangcheck_elapsed(struct work_struct *work) struct intel_engine_cs *engine; enum intel_engine_id id; unsigned int hung = 0, stuck = 0; - int busy_count = 0; if (!i915_modparams.enable_hangcheck) return; @@ -429,7 +428,6 @@ static void i915_hangcheck_elapsed(struct work_struct *work) intel_uncore_arm_unclaimed_mmio_detection(dev_priv); for_each_engine(engine, dev_priv, id) { - const bool busy = intel_engine_has_waiter(engine); struct intel_engine_hangcheck hc; semaphore_clear_deadlocks(dev_priv); @@ -443,16 +441,13 @@ static void i915_hangcheck_elapsed(struct work_struct *work) if (hc.action != ENGINE_DEAD) stuck |= intel_engine_flag(engine); } - - busy_count += busy; } if (hung) hangcheck_declare_hang(dev_priv, hung, stuck); /* Reset timer in case GPU hangs without another request being added */ - if (busy_count) - i915_queue_hangcheck(dev_priv); + i915_queue_hangcheck(dev_priv); } void intel_engine_init_hangcheck(struct intel_engine_cs *engine) From 751b01cb07ebf0dbbe4a4fbfeaa509388e4a2b0a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 31 Jan 2018 21:44:39 +0000 Subject: [PATCH 21/21] drm/i915/ppgtt: Pin page directories before allocation Commit e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes") believed that because it did not insert its freshly allocated page directory into the pd tree, it was safe from the shrinker. I failed to heed the lesson learnt from commit dd19674bacba ("drm/i915: Remove bitmap tracking for used-ptes") that we need to pin all the levels in the tree before hitting the shrinker or else the shrinker may free an upper layer as we proceed to allocate the tree. Thus leaving dangling pointers everywhere and a GPF should we hit direct reclaim at just the wrong moment. CPU: 0 PID: 7374 Comm: chromium Tainted: P O 4.14.13-1-ARCH #1 Hardware name: Apple Inc. MacBookPro12,1/Mac-E43C1C25D4880AD6, BIOS MBP121.88Z.0167.B33.1706181928 06/18/2017 task: ffff994f696c2c40 task.stack: ffffb1a789d4c000 RIP: 0010:gen8_ppgtt_set_pde.isra.40+0x48/0x70 [i915] RSP: 0018:ffffb1a789d4f940 EFLAGS: 00010206 RAX: 81c1788cc4f68138 RBX: ffff994f54db8000 RCX: ffff994f696c2c40 RDX: 000000023bc73003 RSI: ffff994d598b6b80 RDI: ffff994f54db8000 RBP: ffff994d598b6b80 R08: 0000000000000000 R09: 0000000000000000 R10: ffffb1a789d4f550 R11: ffff994eaf3c3208 R12: 0000000000000027 R13: 0000000000005000 R14: 0000000004e8f000 R15: ffff994f54dba000 FS: 00007f585886aa00(0000) GS:ffff994faec00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000004ac8e8 CR3: 00000002552c8004 CR4: 00000000003606f0 Call Trace: gen8_ppgtt_alloc_pdp+0x178/0x320 [i915] gen8_ppgtt_alloc_4lvl+0x5f/0x150 [i915] ppgtt_bind_vma+0x30/0x70 [i915] i915_vma_bind+0x68/0xd0 [i915] __i915_vma_do_pin+0x2d6/0x3a0 [i915] eb_lookup_vmas+0x7a2/0xb50 [i915] i915_gem_do_execbuffer+0x4d7/0x10e0 [i915] ? sock_wfree+0x34/0x60 ? unix_stream_read_generic+0x1f9/0x7e0 ? import_iovec+0x37/0xd0 ? i915_gem_execbuffer2+0x5d/0x390 [i915] i915_gem_execbuffer2+0x1b7/0x390 [i915] ? i915_gem_execbuffer+0x2d0/0x2d0 [i915] drm_ioctl_kernel+0x59/0xb0 [drm] drm_ioctl+0x2d5/0x370 [drm] ? i915_gem_execbuffer+0x2d0/0x2d0 [i915] ? __seccomp_filter+0x3b/0x260 do_vfs_ioctl+0xa1/0x610 ? syscall_trace_enter+0xdb/0x2b0 SyS_ioctl+0x74/0x80 do_syscall_64+0x55/0x110 entry_SYSCALL64_slow_path+0x25/0x25 RIP: 0033:0x7f584fa82d27 RSP: 002b:00007ffee14a7828 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 000003b0126a1030 RCX: 00007f584fa82d27 RDX: 00007ffee14a7870 RSI: 0000000040406469 RDI: 0000000000000080 RBP: 00007ffee14a7870 R08: 0000000000000002 R09: 0000000000000077 R10: 00007f5839f2b780 R11: 0000000000000246 R12: 0000000040406469 R13: 0000000000000080 R14: 00007f5842b00040 R15: 0000000000000000 Code: 01 00 83 81 58 0a 00 00 01 48 2b 05 13 9d fd c9 48 c1 f8 06 48 c1 e0 0c 48 8d 04 d0 48 8b 56 08 48 03 05 0c 9d fd c9 48 83 ca 03 <48> 89 10 83 a9 58 0a 00 00 01 65 ff 0d 37 03 fb 3e 74 02 f3 c3 RIP: gen8_ppgtt_set_pde.isra.40+0x48/0x70 [i915] RSP: ffffb1a789d4f940 Reported-by: Eric Blau Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104773 Fixes: e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes") References: dd19674bacba ("drm/i915: Remove bitmap tracking for used-ptes") Testcase: igt/drv_selftest/live_gtt (igt_ppgtt_shrink_boom) Signed-off-by: Chris Wilson Cc: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20180131214440.7141-1-chris@chris-wilson.co.uk Reviewed-by: Matthew Auld (cherry picked from commit b715a2f0c7714a399e7f8e951cc8dea9cd4eeb4b) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 133cb9590003..7e403eaa9e0f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1359,15 +1359,18 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm, int count = gen8_pte_count(start, length); if (pt == vm->scratch_pt) { + pd->used_pdes++; + pt = alloc_pt(vm); - if (IS_ERR(pt)) + if (IS_ERR(pt)) { + pd->used_pdes--; goto unwind; + } if (count < GEN8_PTES || intel_vgpu_active(vm->i915)) gen8_initialize_pt(vm, pt); gen8_ppgtt_set_pde(vm, pd, pt, pde); - pd->used_pdes++; GEM_BUG_ON(pd->used_pdes > I915_PDES); } @@ -1391,13 +1394,16 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm, gen8_for_each_pdpe(pd, pdp, start, length, pdpe) { if (pd == vm->scratch_pd) { + pdp->used_pdpes++; + pd = alloc_pd(vm); - if (IS_ERR(pd)) + if (IS_ERR(pd)) { + pdp->used_pdpes--; goto unwind; + } gen8_initialize_pd(vm, pd); gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe); - pdp->used_pdpes++; GEM_BUG_ON(pdp->used_pdpes > i915_pdpes_per_pdp(vm)); mark_tlbs_dirty(i915_vm_to_ppgtt(vm));