Message ID | 1374865055-17919-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, 26 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Otherwise we get flooded by the kernel warning us that we are doing > long sequences of IO without serialisation. For example, > > WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 vlv_sideband_rw+0x48/0x1ef() > Modules linked in: > CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: G W 3.11.0-rc2+ #4 > Call Trace: > [<c2028564>] ? warn_slowpath_common+0x63/0x78 > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > [<c20285dd>] ? warn_slowpath_null+0xf/0x13 > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > [<c227b060>] ? vlv_dpio_write+0x1c/0x21 > [<c2262b3b>] ? intel_dp_set_signal_levels+0x24a/0x385 > [<c2264909>] ? intel_dp_complete_link_train+0x25/0x1d1 > [<c2264c55>] ? intel_dp_check_link_status+0xf7/0x106 > [<c2238ced>] ? i915_hotplug_work_func+0x17b/0x221 > [<c203a204>] ? process_one_work+0x12e/0x210 > [<c203a5e4>] ? worker_thread+0x116/0x1ad > [<c203a4ce>] ? rescuer_thread+0x1cb/0x1cb > [<c203d8f5>] ? kthread+0x67/0x6c > [<c2457ebb>] ? ret_from_kernel_thread+0x1b/0x30 > [<c203d88e>] ? init_completion+0x18/0x18 > > v2: Retire the locking in vlv_crtc_enable() and do it close to the > meat. Grumble about throwing the fix and the refactoring together for no real reason, and having a slightly misleading subject. But since we have the warnings in place, the patch is small, and the end result is what we want, I'll let it pass. Just this once. ;) Reviewed-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 4 ---- > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ > drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++ > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b3389d7..6bb9017 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3653,8 +3653,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > intel_crtc->active = true; > intel_update_watermarks(dev); > > - mutex_lock(&dev_priv->dpio_lock); > - > for_each_encoder_on_crtc(dev, crtc, encoder) > if (encoder->pre_pll_enable) > encoder->pre_pll_enable(encoder); > @@ -3679,8 +3677,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > intel_crtc_update_cursor(crtc, true); > > intel_update_fbc(dev); > - > - mutex_unlock(&dev_priv->dpio_lock); > } > > static void i9xx_crtc_enable(struct drm_crtc *crtc) > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 4a7ba5e..2a455b8 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1719,6 +1719,7 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder) > int pipe = intel_crtc->pipe; > u32 val; > > + mutex_lock(&dev_priv->dpio_lock); > val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port)); > val = 0; > if (pipe) > @@ -1732,6 +1733,7 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder) > 0x00760018); > vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), > 0x00400888); > + mutex_unlock(&dev_priv->dpio_lock); > } > } > > @@ -1746,6 +1748,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder) > return; > > /* Program Tx lane resets to default */ > + mutex_lock(&dev_priv->dpio_lock); > vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), > DPIO_PCS_TX_LANE2_RESET | > DPIO_PCS_TX_LANE1_RESET); > @@ -1759,6 +1762,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder) > vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00); > vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500); > vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000); > + mutex_unlock(&dev_priv->dpio_lock); > } > > /* > @@ -1970,6 +1974,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp) > return 0; > } > > + mutex_lock(&dev_priv->dpio_lock); > vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000); > vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value); > vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port), > @@ -1978,6 +1983,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp) > vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000); > vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value); > vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000); > + mutex_unlock(&dev_priv->dpio_lock); > > return 0; > } > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 044d11d..5fa3035 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1033,6 +1033,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder) > return; > > /* Enable clock channels for this port */ > + mutex_lock(&dev_priv->dpio_lock); > val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port)); > val = 0; > if (pipe) > @@ -1063,6 +1064,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder) > 0x00760018); > vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), > 0x00400888); > + mutex_unlock(&dev_priv->dpio_lock); > } > > static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) > @@ -1076,6 +1078,7 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) > return; > > /* Program Tx lane resets to default */ > + mutex_lock(&dev_priv->dpio_lock); > vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), > DPIO_PCS_TX_LANE2_RESET | > DPIO_PCS_TX_LANE1_RESET); > @@ -1094,6 +1097,7 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) > 0x00002000); > vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), > DPIO_TX_OCALINIT_EN); > + mutex_lock(&dev_priv->dpio_lock); > } > > static void intel_hdmi_post_disable(struct intel_encoder *encoder) > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jul 29, 2013 at 04:20:28PM +0300, Jani Nikula wrote: > On Fri, 26 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Otherwise we get flooded by the kernel warning us that we are doing > > long sequences of IO without serialisation. For example, > > > > WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 vlv_sideband_rw+0x48/0x1ef() > > Modules linked in: > > CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: G W 3.11.0-rc2+ #4 > > Call Trace: > > [<c2028564>] ? warn_slowpath_common+0x63/0x78 > > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > > [<c20285dd>] ? warn_slowpath_null+0xf/0x13 > > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > > [<c227b060>] ? vlv_dpio_write+0x1c/0x21 > > [<c2262b3b>] ? intel_dp_set_signal_levels+0x24a/0x385 > > [<c2264909>] ? intel_dp_complete_link_train+0x25/0x1d1 > > [<c2264c55>] ? intel_dp_check_link_status+0xf7/0x106 > > [<c2238ced>] ? i915_hotplug_work_func+0x17b/0x221 > > [<c203a204>] ? process_one_work+0x12e/0x210 > > [<c203a5e4>] ? worker_thread+0x116/0x1ad > > [<c203a4ce>] ? rescuer_thread+0x1cb/0x1cb > > [<c203d8f5>] ? kthread+0x67/0x6c > > [<c2457ebb>] ? ret_from_kernel_thread+0x1b/0x30 > > [<c203d88e>] ? init_completion+0x18/0x18 > > > > v2: Retire the locking in vlv_crtc_enable() and do it close to the > > meat. > > Grumble about throwing the fix and the refactoring together for no real > reason, and having a slightly misleading subject. But since we have the > warnings in place, the patch is small, and the end result is what we > want, I'll let it pass. Just this once. ;) Meh, easier than working which paths were covered by the current locking scheme and which weren't - which I suppect conflict anyway. The intention was just to do as the subject said, but then I had to fix the deadlock presented by the patch... -Chris
On Mon, Jul 29, 2013 at 03:50:14PM +0100, Chris Wilson wrote: > On Mon, Jul 29, 2013 at 04:20:28PM +0300, Jani Nikula wrote: > > On Fri, 26 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Otherwise we get flooded by the kernel warning us that we are doing > > > long sequences of IO without serialisation. For example, > > > > > > WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 vlv_sideband_rw+0x48/0x1ef() > > > Modules linked in: > > > CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: G W 3.11.0-rc2+ #4 > > > Call Trace: > > > [<c2028564>] ? warn_slowpath_common+0x63/0x78 > > > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > > > [<c20285dd>] ? warn_slowpath_null+0xf/0x13 > > > [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef > > > [<c227b060>] ? vlv_dpio_write+0x1c/0x21 > > > [<c2262b3b>] ? intel_dp_set_signal_levels+0x24a/0x385 > > > [<c2264909>] ? intel_dp_complete_link_train+0x25/0x1d1 > > > [<c2264c55>] ? intel_dp_check_link_status+0xf7/0x106 > > > [<c2238ced>] ? i915_hotplug_work_func+0x17b/0x221 > > > [<c203a204>] ? process_one_work+0x12e/0x210 > > > [<c203a5e4>] ? worker_thread+0x116/0x1ad > > > [<c203a4ce>] ? rescuer_thread+0x1cb/0x1cb > > > [<c203d8f5>] ? kthread+0x67/0x6c > > > [<c2457ebb>] ? ret_from_kernel_thread+0x1b/0x30 > > > [<c203d88e>] ? init_completion+0x18/0x18 > > > > > > v2: Retire the locking in vlv_crtc_enable() and do it close to the > > > meat. > > > > Grumble about throwing the fix and the refactoring together for no real > > reason, and having a slightly misleading subject. But since we have the > > warnings in place, the patch is small, and the end result is what we > > want, I'll let it pass. Just this once. ;) > > Meh, easier than working which paths were covered by the current locking > scheme and which weren't - which I suppect conflict anyway. The > intention was just to do as the subject said, but then I had to fix the > deadlock presented by the patch... Queued for -next, thanks for the patch. And I agree that in this case splitting stuff further is just more awkward ... -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b3389d7..6bb9017 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3653,8 +3653,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_crtc->active = true; intel_update_watermarks(dev); - mutex_lock(&dev_priv->dpio_lock); - for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder->pre_pll_enable) encoder->pre_pll_enable(encoder); @@ -3679,8 +3677,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_crtc_update_cursor(crtc, true); intel_update_fbc(dev); - - mutex_unlock(&dev_priv->dpio_lock); } static void i9xx_crtc_enable(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4a7ba5e..2a455b8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1719,6 +1719,7 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder) int pipe = intel_crtc->pipe; u32 val; + mutex_lock(&dev_priv->dpio_lock); val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port)); val = 0; if (pipe) @@ -1732,6 +1733,7 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder) 0x00760018); vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888); + mutex_unlock(&dev_priv->dpio_lock); } } @@ -1746,6 +1748,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder) return; /* Program Tx lane resets to default */ + mutex_lock(&dev_priv->dpio_lock); vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET); @@ -1759,6 +1762,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder) vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00); vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500); vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000); + mutex_unlock(&dev_priv->dpio_lock); } /* @@ -1970,6 +1974,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp) return 0; } + mutex_lock(&dev_priv->dpio_lock); vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000); vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value); vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port), @@ -1978,6 +1983,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp) vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000); vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value); vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000); + mutex_unlock(&dev_priv->dpio_lock); return 0; } diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 044d11d..5fa3035 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1033,6 +1033,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder) return; /* Enable clock channels for this port */ + mutex_lock(&dev_priv->dpio_lock); val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port)); val = 0; if (pipe) @@ -1063,6 +1064,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder) 0x00760018); vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888); + mutex_unlock(&dev_priv->dpio_lock); } static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) @@ -1076,6 +1078,7 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) return; /* Program Tx lane resets to default */ + mutex_lock(&dev_priv->dpio_lock); vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET); @@ -1094,6 +1097,7 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder) 0x00002000); vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), DPIO_TX_OCALINIT_EN); + mutex_lock(&dev_priv->dpio_lock); } static void intel_hdmi_post_disable(struct intel_encoder *encoder)
Otherwise we get flooded by the kernel warning us that we are doing long sequences of IO without serialisation. For example, WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 vlv_sideband_rw+0x48/0x1ef() Modules linked in: CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: G W 3.11.0-rc2+ #4 Call Trace: [<c2028564>] ? warn_slowpath_common+0x63/0x78 [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef [<c20285dd>] ? warn_slowpath_null+0xf/0x13 [<c227ad43>] ? vlv_sideband_rw+0x48/0x1ef [<c227b060>] ? vlv_dpio_write+0x1c/0x21 [<c2262b3b>] ? intel_dp_set_signal_levels+0x24a/0x385 [<c2264909>] ? intel_dp_complete_link_train+0x25/0x1d1 [<c2264c55>] ? intel_dp_check_link_status+0xf7/0x106 [<c2238ced>] ? i915_hotplug_work_func+0x17b/0x221 [<c203a204>] ? process_one_work+0x12e/0x210 [<c203a5e4>] ? worker_thread+0x116/0x1ad [<c203a4ce>] ? rescuer_thread+0x1cb/0x1cb [<c203d8f5>] ? kthread+0x67/0x6c [<c2457ebb>] ? ret_from_kernel_thread+0x1b/0x30 [<c203d88e>] ? init_completion+0x18/0x18 v2: Retire the locking in vlv_crtc_enable() and do it close to the meat. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 4 ---- drivers/gpu/drm/i915/intel_dp.c | 6 ++++++ drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-)