From f43aa31fe7dc43b808ec619b0d407180cd3725c0 Mon Sep 17 00:00:00 2001 From: fred gao Date: Mon, 17 Jul 2017 14:52:05 +0800 Subject: drm/i915/gvt: Fix the vblank timer close issue after shutdown VMs in reverse Once the Windows guest is shutdown, the display pipe will be disabled and intel_gvt_check_vblank_emulation will be called to check if the vblank timer is turned off. Given the scenario of creating VM1 ,VM2, destoying VM2 in current code, VM1 has pipe enabled and continues to check VM2, the flag have_enabled_pipe is always false since all the VM2 pipes are disabled, so the vblank timer will be canceled and TDR happens in Windows VM1 guest due to the vsync timeout. In this patch the vblank timer will be never canceled once one pipe is enabled. v2: - remove have_enabled_pipe flag and check pipe enabled directly. (Zhenyu) Cc: Wang Hongbo Signed-off-by: fred gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index 2deb05f618fb..7cb0818a13de 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -323,27 +323,27 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt) { struct intel_gvt_irq *irq = &gvt->irq; struct intel_vgpu *vgpu; - bool have_enabled_pipe = false; int pipe, id; if (WARN_ON(!mutex_is_locked(&gvt->lock))) return; - hrtimer_cancel(&irq->vblank_timer.timer); - for_each_active_vgpu(gvt, vgpu, id) { for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) { - have_enabled_pipe = - pipe_is_enabled(vgpu, pipe); - if (have_enabled_pipe) - break; + if (pipe_is_enabled(vgpu, pipe)) + goto out; } } - if (have_enabled_pipe) - hrtimer_start(&irq->vblank_timer.timer, - ktime_add_ns(ktime_get(), irq->vblank_timer.period), - HRTIMER_MODE_ABS); + /* 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); + } static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe) -- cgit v1.2.3 From 26a201a2ba82a801973ce29e1004b64742e81e7e Mon Sep 17 00:00:00 2001 From: Jian Jun Chen Date: Wed, 19 Jul 2017 13:16:56 +0800 Subject: drm/i915/gvt: Extend KBL platform support in GVT-g Extend KBL platform support in GVT-g. Validation tests are done on KBL server and KBL NUC. Both show the same quality. Signed-off-by: Jian Jun Chen Cc: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/intel_gvt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c index 52d5b82790d9..c17ed0e62b67 100644 --- a/drivers/gpu/drm/i915/intel_gvt.c +++ b/drivers/gpu/drm/i915/intel_gvt.c @@ -45,7 +45,7 @@ static bool is_supported_device(struct drm_i915_private *dev_priv) return true; if (IS_SKYLAKE(dev_priv)) return true; - if (IS_KABYLAKE(dev_priv) && INTEL_DEVID(dev_priv) == 0x591D) + if (IS_KABYLAKE(dev_priv)) return true; return false; } -- cgit v1.2.3 From 7728124af30e04b9c2c3e3125e28963b065867c2 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Fri, 14 Jul 2017 18:12:42 +0300 Subject: drm/i915: Fix user ptr check size in eb_relocate_vma() Fix the sizeof(ptr) vs. sizeof(*ptr) typo. Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") Cc: Chris Wilson Cc: Joonas Lahtinen Signed-off-by: Imre Deak Reviewed-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/20170714151242.517-2-imre.deak@intel.com (cherry picked from commit edd9003f7f9dddd28fdd768e6e7569d996c769cb) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 054b2e54cdaf..8d6a783b06ba 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1458,7 +1458,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma) * to read. However, if the array is not writable the user loses * the updated relocation values. */ - if (unlikely(!access_ok(VERIFY_READ, urelocs, remain*sizeof(urelocs)))) + if (unlikely(!access_ok(VERIFY_READ, urelocs, remain*sizeof(*urelocs)))) return -EFAULT; do { -- cgit v1.2.3 From d34cfebbf9cca8308e7bba3636a1a0fd79131051 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 14 Jul 2017 18:52:27 +0300 Subject: drm/i915: Fix cursor updates on some platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out that just writing CURPOS isn't sufficient to move the cursor on some platforms. My 830 works just fine, but eg. 945 and PNV don't. On those platforms we need to arm even the CURPOS update with a CURBASE write. Even worse, a write to any of the cursor register apart from CURBASE will cancel an already pending cursor update. So if we have armed a CURCNTR/CURBASE update, a subsequent CURPOS write prior to vblank would cancel that armed update. Thus we're left with a cursor that doesn't appear to move, or even change shape. Fix the problem by always performing the CURBASE write after a CURPOS write. Bspec is somewhat unclear which platforms actually require this CURBASE write and which don't. So to keep it simple and to make sure we really fix the problem across all supported devices, let's just perform the CURBASE write unconditionally. Cc: Paul Menzel Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101790 Fixes: 75343a44c901 ("drm/i915: Drop useless posting reads from cursor commit") Signed-off-by: Ville Syrjälä Tested-by: Paul Menzel Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20170714155227.6089-1-ville.syrjala@linux.intel.com (cherry picked from commit 8753d2bc5e49daad301ce65f5dada57ed924fad6) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dec9e58545a1..ae4fb72a3394 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9540,7 +9540,16 @@ static void i9xx_update_cursor(struct intel_plane *plane, * On some platforms writing CURCNTR first will also * cause CURPOS to be armed by the CURBASE write. * Without the CURCNTR write the CURPOS write would - * arm itself. + * arm itself. Thus we always start the full update + * with a CURCNTR write. + * + * On other platforms CURPOS always requires the + * CURBASE write to arm the update. Additonally + * a write to any of the cursor register will cancel + * an already armed cursor update. Thus leaving out + * the CURBASE write after CURPOS could lead to a + * cursor that doesn't appear to move, or even change + * shape. Thus we always write CURBASE. * * CURCNTR and CUR_FBC_CTL are always * armed by the CURBASE write only. @@ -9559,6 +9568,7 @@ static void i9xx_update_cursor(struct intel_plane *plane, plane->cursor.cntl = cntl; } else { I915_WRITE_FW(CURPOS(pipe), pos); + I915_WRITE_FW(CURBASE(pipe), base); } POSTING_READ_FW(CURBASE(pipe)); -- cgit v1.2.3 From 4706ca779a723913469a47bbdd63ede01681f0c4 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 19 Jul 2017 14:54:55 +0200 Subject: drm/i915: Unbreak gpu reset vs. modeset locking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Taking the modeset locks unconditionally isn't the greatest idea, because atm that part is still broken and times out (and then atomic keels over). And there's really no reason to do so, the old code didn't do that either. To make the patch a bit simpler let's also nuke 2 cases that are only around for the old mmioflip paths. Atomic nonblocking workers will not die (minus bugs) when a gpu reset happens. And of course this doesn't fix any of the gpu reset vs. modeset deadlock fun, but it at least stop modern CI machines from keeling over all over the place for no reason at all. And we still have the explicit testcases to run the fake gpu reset, so coverage isn't that much worse. v2: Split out additional changes on top, restrict this to purely reducing the critical section of modeset locks. v2: Review from Maarten - update comments - don't oops when state is NULL in intel_finish_reset, but try to at least still drop locks properly. The hw is going to be toast anyway. Fixes: 739748939974 ("drm/i915: Fix modeset handling during gpu reset, v5.") Cc: Maarten Lankhorst Reviewed-by: Maarten Lankhorst Cc: Ville Syrjälä Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20170719125502.25696-3-daniel.vetter@ffwll.ch (cherry picked from commit ce87ea15ebc60a9f8f156b2549f7b2cf7fe48d04) Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++------------------------- 1 file changed, 18 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ae4fb72a3394..e35daae00137 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3427,26 +3427,6 @@ static void intel_complete_page_flips(struct drm_i915_private *dev_priv) intel_finish_page_flip_cs(dev_priv, crtc->pipe); } -static void intel_update_primary_planes(struct drm_device *dev) -{ - struct drm_crtc *crtc; - - for_each_crtc(dev, crtc) { - struct intel_plane *plane = to_intel_plane(crtc->primary); - struct intel_plane_state *plane_state = - to_intel_plane_state(plane->base.state); - - if (plane_state->base.visible) { - trace_intel_update_plane(&plane->base, - to_intel_crtc(crtc)); - - plane->update_plane(plane, - to_intel_crtc_state(crtc->state), - plane_state); - } - } -} - static int __intel_display_resume(struct drm_device *dev, struct drm_atomic_state *state, @@ -3499,6 +3479,12 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv) struct drm_atomic_state *state; int ret; + + /* reset doesn't touch the display */ + if (!i915.force_reset_modeset_test && + !gpu_reset_clobbers_display(dev_priv)) + return; + /* * Need mode_config.mutex so that we don't * trample ongoing ->detect() and whatnot. @@ -3512,12 +3498,6 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv) drm_modeset_backoff(ctx); } - - /* reset doesn't touch the display, but flips might get nuked anyway, */ - if (!i915.force_reset_modeset_test && - !gpu_reset_clobbers_display(dev_priv)) - return; - /* * Disabling the crtcs gracefully seems nicer. Also the * g33 docs say we should at least disable all the planes. @@ -3547,6 +3527,14 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) struct drm_atomic_state *state = dev_priv->modeset_restore_state; int ret; + /* reset doesn't touch the display */ + if (!i915.force_reset_modeset_test && + !gpu_reset_clobbers_display(dev_priv)) + return; + + if (!state) + goto unlock; + /* * Flips in the rings will be nuked by the reset, * so complete all pending flips so that user space @@ -3558,22 +3546,10 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) /* reset doesn't touch the display */ if (!gpu_reset_clobbers_display(dev_priv)) { - if (!state) { - /* - * Flips in the rings have been nuked by the reset, - * so update the base address of all primary - * planes to the the last fb to make sure we're - * showing the correct fb after a reset. - * - * FIXME: Atomic will make this obsolete since we won't schedule - * CS-based flips (which might get lost in gpu resets) any more. - */ - intel_update_primary_planes(dev); - } else { - ret = __intel_display_resume(dev, state, ctx); + /* for testing only restore the display */ + ret = __intel_display_resume(dev, state, ctx); if (ret) DRM_ERROR("Restoring old state failed with %i\n", ret); - } } else { /* * The display has been reset as well, @@ -3597,8 +3573,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) intel_hpd_init(dev_priv); } - if (state) - drm_atomic_state_put(state); + drm_atomic_state_put(state); +unlock: drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); mutex_unlock(&dev->mode_config.mutex); -- cgit v1.2.3 From 3d91142528db7d4ea34e3233c261acc48c164b9a Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Thu, 20 Jul 2017 00:35:03 +0200 Subject: drm/i915/selftests: Fix an error handling path in 'mock_gem_device()' Goto the right label in case of error, otherwise there is a leak. This has been introduced by c5cf9a9147ff. In this patch a goto has not been updated. Fixes: c5cf9a9147ff ("drm/i915: Create a kmem_cache to allocate struct i915_priolist from") Signed-off-by: Christophe JAILLET Link: https://patchwork.freedesktop.org/patch/msgid/20170719223503.30580-1-christophe.jaillet@wanadoo.fr Reviewed-by: Tvrtko Ursulin Signed-off-by: Chris Wilson (cherry picked from commit a5ec7fe81a6ec38cb8b8a798d0552cbcadce7aa9) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 627e2aa09766..8cdec455cf7d 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -206,7 +206,7 @@ struct drm_i915_private *mock_gem_device(void) mkwrite_device_info(i915)->ring_mask = BIT(0); i915->engine[RCS] = mock_engine(i915, "mock"); if (!i915->engine[RCS]) - goto err_dependencies; + goto err_priorities; i915->kernel_context = mock_context(i915, NULL); if (!i915->kernel_context) -- cgit v1.2.3 From 283d6860d64f5091565bf729b0a6d6af14ae6c27 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Thu, 20 Jul 2017 14:28:20 +0300 Subject: drm/i915: Fix scaler init during CRTC HW state readout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scaler allocation code depends on a non-zero default value for the crtc scaler_id, so make sure we initialize the scaler state accordingly even if the crtc is off. This fixes at least an initial YUV420 modeset (added in a follow-up patchset by Shashank) when booting with the screen off: after the initial HW readout and modeset which enables the scaler a subsequent modeset will disable the scaler which isn't properly allocated. This results in a funky HW state where the pipe scaler HW registers can't be modified and the normally black screen is grey and shifted to the right or jitters. The problem was revealed by Shashank's YUV420 patchset and first reported by Ville. v2: - In the stable tag also include versions which need backporting (Jani) Cc: Jani Nikula Cc: Shashank Sharma Cc: Ville Syrjälä Cc: Chandra Konduru Cc: Matt Roper Cc: # 4.2.x Reported-by: Ville Syrjälä Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers") Signed-off-by: Imre Deak Reviewed-by: Mahesh Kumar Link: https://patchwork.freedesktop.org/patch/msgid/20170720112820.26816-1-imre.deak@intel.com Signed-off-by: Daniel Vetter (cherry picked from commit 5fb9dadf336f3590c799e8cbde348215dccc2aa2) Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e35daae00137..9471c88d449e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9093,6 +9093,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, u64 power_domain_mask; bool active; + if (INTEL_GEN(dev_priv) >= 9) { + intel_crtc_init_scalers(crtc, pipe_config); + + pipe_config->scaler_state.scaler_id = -1; + pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX); + } + power_domain = POWER_DOMAIN_PIPE(crtc->pipe); if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) return false; @@ -9121,13 +9128,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK; - if (INTEL_GEN(dev_priv) >= 9) { - intel_crtc_init_scalers(crtc, pipe_config); - - pipe_config->scaler_state.scaler_id = -1; - pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX); - } - power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe); if (intel_display_power_get_if_enabled(dev_priv, power_domain)) { power_domain_mask |= BIT_ULL(power_domain); -- cgit v1.2.3 From 5846a73f26a1efa45e2c2edd36aa2ed0a6ad380a Mon Sep 17 00:00:00 2001 From: "Navare, Manasi D" Date: Mon, 17 Jul 2017 15:05:22 -0700 Subject: drm/i915/cnl: Fix loadgen select programming on ddi vswing sequence The condition for setting the Loadgen Select bit of PORT_TX_DW4 register during DDI Vswing Sequence should be Bit rate <=6 GHz whereas the existing code checks only Bit Rate < 6GHz. This patch fixes this condition. While at it also remove the redundant paranthesis. Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.") Cc: Paulo Zanoni Cc: Rodrigo Vivi Signed-off-by: Manasi Navare Reviewed-by: Paulo Zanoni Signed-off-by: Paulo Zanoni Link: https://patchwork.freedesktop.org/patch/msgid/1500329122-32662-1-git-send-email-manasi.d.navare@intel.com Signed-off-by: Daniel Vetter (cherry picked from commit a8e45a1c42d11597e975f3e5f2fe182f90cdaa7f) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 80e96f1f49d2..9edeaaef77ad 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1896,8 +1896,8 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level) val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln)); val &= ~LOADGEN_SELECT; - if (((rate < 600000) && (width == 4) && (ln >= 1)) || - ((rate < 600000) && (width < 4) && ((ln == 1) || (ln == 2)))) { + if ((rate <= 600000 && width == 4 && ln >= 1) || + (rate <= 600000 && width < 4 && (ln == 1 || ln == 2))) { val |= LOADGEN_SELECT; } I915_WRITE(CNL_PORT_TX_DW4_LN(port, ln), val); -- cgit v1.2.3 From a6c00779b2b03f2568c2c697afb1c2cd7f90cac6 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Jul 2017 15:50:34 +0100 Subject: drm/i915: Remove assertion from raw __i915_vma_unpin() After we detect a i915_vma pin overflow, we call __i915_vma_unpin to cleanup. However, on an overflow the pin_count bitfield will be zero, triggering an assertion, even though we the intention is to merely warn and report the error back to the user (as historically the culprit has be a leak in the display code). Fixes: 20dfbde463c8 ("drm/i915: Wrap vma->pin_count accessors with small inline helpers") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-2-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen Signed-off-by: Daniel Vetter (cherry picked from commit 67fddd902b8e37b15a905c287ce4e40f52a564af) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_vma.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index 4a673fc1a432..20cf272c97b1 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -284,12 +284,12 @@ static inline void __i915_vma_pin(struct i915_vma *vma) static inline void __i915_vma_unpin(struct i915_vma *vma) { - GEM_BUG_ON(!i915_vma_is_pinned(vma)); vma->flags--; } static inline void i915_vma_unpin(struct i915_vma *vma) { + GEM_BUG_ON(!i915_vma_is_pinned(vma)); GEM_BUG_ON(!drm_mm_node_allocated(&vma->node)); __i915_vma_unpin(vma); } -- cgit v1.2.3 From bed8d1c840f0b3ddefc5c27a983a1d587cf873e9 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Jul 2017 15:50:35 +0100 Subject: drm/i915: Only mark the execobject as pinned on success If we fail to acquire a fence (for old school fenced GPU access) then we unwind the vma reservation, including its pin. However, we were making the execobject as holding the pin before erring out, leading to a double unpin: [ 3193.991802] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:287! [ 3193.998131] invalid opcode: 0000 [#1] PREEMPT SMP [ 3194.002816] Modules linked in: snd_hda_intel i915 vgem snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich mei_me e1000e mei prime_numbers ptp pps_core [last unloaded: i915] [ 3194.022841] CPU: 0 PID: 8123 Comm: kms_flip Tainted: G U 4.13.0-rc1-CI-CI_DRM_471+ #1 [ 3194.031765] Hardware name: Dell Inc. OptiPlex 755 /0PU052, BIOS A04 11/05/2007 [ 3194.040343] task: ffff8800785d4c40 task.stack: ffffc90001768000 [ 3194.046339] RIP: 0010:eb_release_vmas.isra.6+0x119/0x180 [i915] [ 3194.052234] RSP: 0018:ffffc9000176ba80 EFLAGS: 00010246 [ 3194.057439] RAX: 00000000000003c0 RBX: ffff8800710fc2d8 RCX: ffff8800588e4f48 [ 3194.064546] RDX: ffffffff1fffffff RSI: 00000000ffffffff RDI: ffff8800588e00d0 [ 3194.071654] RBP: ffffc9000176bab0 R08: 0000000000000000 R09: 0000000000000000 [ 3194.078761] R10: 0000000000000040 R11: 0000000000000001 R12: ffff880060822f00 [ 3194.085867] R13: 0000000000000310 R14: 00000000000003b8 R15: ffffc9000176bbb0 [ 3194.092975] FS: 00007fd2b94aba40(0000) GS:ffff88007d200000(0000) knlGS:0000000000000000 [ 3194.101033] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 3194.106754] CR2: 00007ffbec3ff000 CR3: 0000000074e67000 CR4: 00000000000006f0 [ 3194.113861] Call Trace: [ 3194.116321] eb_relocate_slow+0x67/0x4e0 [i915] [ 3194.120861] i915_gem_do_execbuffer+0x429/0x1260 [i915] [ 3194.126070] ? lock_acquire+0xb5/0x210 [ 3194.129803] ? __might_fault+0x39/0x90 [ 3194.133563] i915_gem_execbuffer2+0x9b/0x1b0 [i915] [ 3194.138447] ? i915_gem_execbuffer+0x2b0/0x2b0 [i915] [ 3194.143478] drm_ioctl_kernel+0x64/0xb0 [ 3194.147298] drm_ioctl+0x2cd/0x390 [ 3194.150710] ? i915_gem_execbuffer+0x2b0/0x2b0 [i915] [ 3194.155741] ? finish_task_switch+0xa5/0x210 [ 3194.159993] ? finish_task_switch+0x6a/0x210 [ 3194.164247] do_vfs_ioctl+0x90/0x670 [ 3194.167806] ? entry_SYSCALL_64_fastpath+0x5/0xb1 [ 3194.172492] ? __this_cpu_preempt_check+0x13/0x20 [ 3194.177176] ? trace_hardirqs_on_caller+0xe7/0x1c0 [ 3194.181946] SyS_ioctl+0x3c/0x70 [ 3194.185159] entry_SYSCALL_64_fastpath+0x1c/0xb1 [ 3194.189756] RIP: 0033:0x7fd2b76a8587 [ 3194.193314] RSP: 002b:00007fff074845b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 3194.200855] RAX: ffffffffffffffda RBX: ffffffff8146da43 RCX: 00007fd2b76a8587 [ 3194.207962] RDX: 00007fff074846e0 RSI: 0000000040406469 RDI: 0000000000000003 [ 3194.215068] RBP: ffffc9000176bf88 R08: 0000000000000000 R09: 0000000000000003 [ 3194.222175] R10: 00007fd2b796bb58 R11: 0000000000000246 R12: 00007fff07484880 [ 3194.229280] R13: 0000000000000003 R14: 0000000040406469 R15: 0000000000000000 [ 3194.236386] ? __this_cpu_preempt_check+0x13/0x20 [ 3194.241070] Code: 24 b0 00 00 00 48 85 c9 0f 84 6c ff ff ff 8b 41 20 85 c0 7e 73 83 e8 01 89 41 20 41 8b 84 24 e8 00 00 00 a8 0f 0f 85 5f ff ff ff <0f> 0b 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d f3 c3 49 8b 84 [ 3194.259943] RIP: eb_release_vmas.isra.6+0x119/0x180 [i915] RSP: ffffc9000176ba80 [ 3194.268047] ---[ end trace 1d7348c6575d8800 ]--- [ 3673.658819] softdog: Initiating panic [ 3673.662471] Kernel panic - not syncing: Software Watchdog Timer expired [ 3673.669066] Kernel Offset: disabled [ 3673.672541] Rebooting in 1 seconds.. Reported-by: Tomi Sarvela Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-3-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen Signed-off-by: Daniel Vetter (cherry picked from commit 1da7b54c46bcfe5484af0b27d8c9003b238031b0) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 8d6a783b06ba..8125bb1c745e 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -560,9 +560,6 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb, eb->args->flags |= __EXEC_HAS_RELOC; } - entry->flags |= __EXEC_OBJECT_HAS_PIN; - GEM_BUG_ON(eb_vma_misplaced(entry, vma)); - if (unlikely(entry->flags & EXEC_OBJECT_NEEDS_FENCE)) { err = i915_vma_get_fence(vma); if (unlikely(err)) { @@ -574,6 +571,9 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb, entry->flags |= __EXEC_OBJECT_HAS_FENCE; } + entry->flags |= __EXEC_OBJECT_HAS_PIN; + GEM_BUG_ON(eb_vma_misplaced(entry, vma)); + return 0; } -- cgit v1.2.3 From adf27835a583470707debc55554b638dd85cb8a9 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Jul 2017 15:50:36 +0100 Subject: drm/i915: Only skip updating execobject.offset after error I was being overly paranoid in not updating the execobject.offset after performing the fallback copy where we set reloc.presumed_offset to -1. The thinking was to ensure that a subsequent NORELOC execbuf would be forced to process the invalid relocations. However this is overkill so long as we *only* update the execobject.offset following a successful update of the relocation value witin the batch. If we have to repeat the execbuf due to a later interruption, then we may skip the relocations on the second pass (honouring NORELOC) since the execobject.offset match the actual offsets (even though reloc.presumed_offset is garbage). Subsequent calls to execbuf with NORELOC should themselves ensure that the reloc.presumed_offset have been corrected in case of future migration. Reporting back the actual execobject.offset, even when reloc.presumed_offset is garbage, ensures that reuse of those objects use the latest information to avoid relocations. Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101635 Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-4-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen Signed-off-by: Daniel Vetter (cherry picked from commit 1f727d9e725a408ef58d159c20fb2e51818ff153) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 8125bb1c745e..d70ac429106f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1775,7 +1775,7 @@ out: } } - return err ?: have_copy; + return err; } static int eb_relocate(struct i915_execbuffer *eb) @@ -2209,7 +2209,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_unlock; err = eb_relocate(&eb); - if (err) + if (err) { /* * If the user expects the execobject.offset and * reloc.presumed_offset to be an exact match, @@ -2218,8 +2218,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, * relocation. */ args->flags &= ~__EXEC_HAS_RELOC; - if (err < 0) goto err_vma; + } if (unlikely(eb.batch->exec_entry->flags & EXEC_OBJECT_WRITE)) { DRM_DEBUG("Attempting to use self-modifying batch buffer\n"); -- cgit v1.2.3 From 7b98da66174416a1c7ada93e1eb382e8a4424f87 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Jul 2017 15:50:37 +0100 Subject: drm/i915: Force CPU synchronisation even if userspace requests ASYNC The goal here was to minimise doing any thing or any check inside the kernel that was not strictly required. For a userspace that assumes complete control over the cache domains, the kernel is usually using outdated information and may trigger clflushes where none were required. However, swapping is a situation where userspace has no knowledge of the domain transfer, and will leave the object in the CPU cache. The kernel must flush this out to the backing storage prior to use with the GPU. As we use an asynchronous task tracked by an implicit fence for this, we also need to cancel the ASYNC flag on the object so that the object will wait for the clflush to complete before being executed. This also absolves userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing") that its needed to ensure that the object was out of the CPU cache prior to use on the GPU. Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571 Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Jason Ekstrand Reviewed-by: Jason Ekstrand Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-5-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen Signed-off-by: Daniel Vetter (cherry picked from commit 0f46daa1a273779a0b73d768a788ca3f04238f9c) Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_clflush.c | 7 ++++--- drivers/gpu/drm/i915/i915_gem_clflush.h | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index 152f16c11878..348b29a845c9 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -114,7 +114,7 @@ i915_clflush_notify(struct i915_sw_fence *fence, return NOTIFY_DONE; } -void i915_gem_clflush_object(struct drm_i915_gem_object *obj, +bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, unsigned int flags) { struct clflush *clflush; @@ -128,7 +128,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, */ if (!i915_gem_object_has_struct_page(obj)) { obj->cache_dirty = false; - return; + return false; } /* If the GPU is snooping the contents of the CPU cache, @@ -140,7 +140,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, * tracking. */ if (!(flags & I915_CLFLUSH_FORCE) && obj->cache_coherent) - return; + return false; trace_i915_gem_object_clflush(obj); @@ -179,4 +179,5 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, } obj->cache_dirty = false; + return true; } diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.h b/drivers/gpu/drm/i915/i915_gem_clflush.h index 2455a7820937..f390247561b3 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.h +++ b/drivers/gpu/drm/i915/i915_gem_clflush.h @@ -28,7 +28,7 @@ struct drm_i915_private; struct drm_i915_gem_object; -void i915_gem_clflush_object(struct drm_i915_gem_object *obj, +bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, unsigned int flags); #define I915_CLFLUSH_FORCE BIT(0) #define I915_CLFLUSH_SYNC BIT(1) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d70ac429106f..e9503f6d1100 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1825,7 +1825,7 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) int err; for (i = 0; i < count; i++) { - const struct drm_i915_gem_exec_object2 *entry = &eb->exec[i]; + struct drm_i915_gem_exec_object2 *entry = &eb->exec[i]; struct i915_vma *vma = exec_to_vma(entry); struct drm_i915_gem_object *obj = vma->obj; @@ -1841,12 +1841,14 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) eb->request->capture_list = capture; } + if (unlikely(obj->cache_dirty && !obj->cache_coherent)) { + if (i915_gem_clflush_object(obj, 0)) + entry->flags &= ~EXEC_OBJECT_ASYNC; + } + if (entry->flags & EXEC_OBJECT_ASYNC) goto skip_flushes; - if (unlikely(obj->cache_dirty && !obj->cache_coherent)) - i915_gem_clflush_object(obj, 0); - err = i915_gem_request_await_object (eb->request, obj, entry->flags & EXEC_OBJECT_WRITE); if (err) -- cgit v1.2.3 From e9ba4032a2568ef5b360de95eb6e09195a1b2efd Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 17 Jul 2017 13:13:55 +0200 Subject: drm/i915: Fix bad comparison in skl_compute_plane_wm. ddb_allocation && ddb_allocation / blocks_per_line >= 1 is the same as ddb_allocation >= blocks_per_line, so use the latter to simplify this. This fixes the following compiler warning: drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Signed-off-by: Maarten Lankhorst Fixes: d555cb5827d6 ("drm/i915/skl+: use linetime latency if ddb size is not available") Cc: "Mahesh Kumar" Reported-by: David Binderman Cc: David Binderman Cc: # v4.13-rc1+ Reviewed-by: Mahesh Kumar (cherry picked from commit 54d20ed1fff23c7d2633f01fc788111bf9c51c5d) Signed-off-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/20170717111355.4523-1-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 48ea0fca1f72..40b224b44d1b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4463,8 +4463,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= + fixed_16_16_to_u32_round_up(plane_blocks_per_line)) selected_result = min_fixed_16_16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed_16_16(method1, method2); -- cgit v1.2.3