Re: Your kernel commit 2f4f649a69a9eb51f6e98130e19dd90a260a4145
On Tue, Nov 27, 2012 at 10:58 PM, Magnus R wrote: > I run the 3.7 release candidates on my retina Macbook Pro and starting with > rc6 the graphics have become garbled. I have narrowed it down to your above > mentioned commit in drivers/gpu/drm/i915/intel_display.c (when I revert that > commit the graphics is not garbled). Please point me in the right direction > if you would like me to file bug in a more systematic manner, or let me know > if I can assist with any data of my configuration. Should be fixed with 9a30a61f3516871c5c638f from the drm-intel-fixes tree from http://cgit.freedesktop.org/~danvet/drm-intel The pull request for that is currently pending with Dave Airlie. Cheers, Daniel PS: When reporting regressions, please always include relevant mailing list, so that other people see them to (and maybe also because your dear maintainer may miss a mail every once a while ...). -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PULL] last drm-intel-next for 3.8
Hi Dave, Besides the big item of lifting the "preliminary hw support" tag from the Haswell code, just small bits&pieces all over: - Leftover Haswell patches and some fixes from Paulo - LyncPoint PCH support (for hsw) - OOM handling improvements from Chris Wilson - connector property and send_vblank_event refactorings from Rob Clark - random pile of small fixes Note that the send_vblank refactorings will cause some locking WARNs to show up. Imre has fixed that up, but since all the driver changes outside of the drm core have been for exonys, those four patches are merged through the exonys-next tree. Yours, Daniel The following changes since commit bf6f036848ab2151c2498f11cb7d31a52a95dd5c: drm/vmwgfx: Make vmw_dmabuf_unreference handle NULL objects (2012-11-20 16:19:59 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-next-2012-11-23 for you to fetch changes up to 70b12bb415463c1bd146b67c5fbf6784fd046ad9: drm/i915: promote Haswell to full support (2012-11-23 19:40:14 +0100) Ben Widawsky (2): drm/i915: Fix warning in i915_gem_chipset_flush drm/i915: Use pci_resource functions for BARs. Chris Wilson (11): drm/i915: Use LRI to update the semaphore registers drm/i915: Remove save/restore of physical HWS_PGA register drm/i915: Remove bogus test for a present execbuffer drm/i915: Guard pages being reaped by OOM whilst binding-to-GTT drm/i915: Pin the object whilst faulting it in drm/i915: Flush outstanding unpin tasks before pageflipping drm/i915: Apply the IBX transcoder A w/a for HDMI to SDVO as well drm/i915: Defer assignment of obj->gtt_space until after all possible mallocs drm/i915: Borrow our struct_mutex for the direct reclaim drm/i915: LVDS fallback to fixed-mode if EDID not present drm/i915: Report the origin of the LVDS fixed panel mode Damien Lespiau (1): drm/i915: Only check for valid PP_{ON, OFF}_DELAYS on pre ILK hardware Daniel Vetter (2): drm/i915: drop buggy write to FDI_RX_CHICKEN register drm/i915: resurrect panel lid handling Jani Nikula (1): drm/i915/sdvo: kfree the intel_sdvo_connector, not drm_connector, on destroy Jean Delvare (1): drm/i915: Optimize DIV_ROUND_CLOSEST call Paulo Zanoni (10): drm/i915: don't limit Haswell CRT encoder to pipe A drm/i915: use cpu/pch transcoder on intel_enable_pipe drm/i915: fix false positive "Unclaimed write" messages drm/i915: make DP work on LPT-LP machines drm/i915: don't intel_crt_init if DDI A has 4 lanes drm/i915: make the panel fitter work on pipes B and C on IVB drm/i915: make the panel fitter work on pipes B and C on Haswell drm/i915: fix intel_ddi_get_cdclk_freq for ULT machines drm/i915: implement WaMbcDriverBootEnable on Haswell drm/i915: promote Haswell to full support Rob Clark (2): drm/i915: use drm_send_vblank_event() helper drm/i915: drm_connector_property -> drm_object_property Takashi Iwai (1): drm/i915: Enable DP audio for Haswell Wei Shun Chang (1): drm/i915: add LynxPoint-LP PCH ID drivers/gpu/drm/i915/i915_drv.c| 26 --- drivers/gpu/drm/i915/i915_drv.h| 13 +++- drivers/gpu/drm/i915/i915_gem.c| 102 drivers/gpu/drm/i915/i915_gem_execbuffer.c |9 --- drivers/gpu/drm/i915/i915_gem_gtt.c|8 +-- drivers/gpu/drm/i915/i915_reg.h|4 ++ drivers/gpu/drm/i915/i915_suspend.c|8 --- drivers/gpu/drm/i915/intel_bios.c |3 +- drivers/gpu/drm/i915/intel_crt.c |2 +- drivers/gpu/drm/i915/intel_ddi.c | 11 +++ drivers/gpu/drm/i915/intel_display.c | 80 +++--- drivers/gpu/drm/i915/intel_dp.c|2 +- drivers/gpu/drm/i915/intel_drv.h |4 +- drivers/gpu/drm/i915/intel_hdmi.c |2 +- drivers/gpu/drm/i915/intel_lvds.c | 24 --- drivers/gpu/drm/i915/intel_modes.c |4 +- drivers/gpu/drm/i915/intel_panel.c | 25 +++ drivers/gpu/drm/i915/intel_pm.c| 25 ++- drivers/gpu/drm/i915/intel_ringbuffer.c|7 +- drivers/gpu/drm/i915/intel_sdvo.c | 64 + drivers/gpu/drm/i915/intel_tv.c| 14 ++-- 21 files changed, 260 insertions(+), 177 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PULL] last drm-intel-next for 3.8
On Thu, Nov 29, 2012 at 11:12:00AM +0100, Daniel Vetter wrote: > Besides the big item of lifting the "preliminary hw support" tag from the > Haswell code, just small bits&pieces all over: > - Leftover Haswell patches and some fixes from Paulo > - LyncPoint PCH support (for hsw) > - OOM handling improvements from Chris Wilson > - connector property and send_vblank_event refactorings from Rob Clark > - random pile of small fixes > > Note that the send_vblank refactorings will cause some locking WARNs to > show up. Imre has fixed that up, but since all the driver changes outside > of the drm core have been for exonys, those four patches are merged > through the exonys-next tree. Meh, I've forgotten to cherry-pick an important fix from Ben for a regression in the 3.8 gen6+ gtt code. New pull request below. While I'm at it, the hdmi VIC patch for the drm edid code is still in my queue, I'll send you that in the next 3.8-fixes pull. Yours, Daniel The following changes since commit bf6f036848ab2151c2498f11cb7d31a52a95dd5c: drm/vmwgfx: Make vmw_dmabuf_unreference handle NULL objects (2012-11-20 16:19:59 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel for-airlied for you to fetch changes up to 2ff4aeac39dbdcac934694413767f09a27965e11: drm/i915: Fix pte updates in ggtt clear range (2012-11-29 11:14:44 +0100) Ben Widawsky (3): drm/i915: Fix warning in i915_gem_chipset_flush drm/i915: Use pci_resource functions for BARs. drm/i915: Fix pte updates in ggtt clear range Chris Wilson (11): drm/i915: Use LRI to update the semaphore registers drm/i915: Remove save/restore of physical HWS_PGA register drm/i915: Remove bogus test for a present execbuffer drm/i915: Guard pages being reaped by OOM whilst binding-to-GTT drm/i915: Pin the object whilst faulting it in drm/i915: Flush outstanding unpin tasks before pageflipping drm/i915: Apply the IBX transcoder A w/a for HDMI to SDVO as well drm/i915: Defer assignment of obj->gtt_space until after all possible mallocs drm/i915: Borrow our struct_mutex for the direct reclaim drm/i915: LVDS fallback to fixed-mode if EDID not present drm/i915: Report the origin of the LVDS fixed panel mode Damien Lespiau (1): drm/i915: Only check for valid PP_{ON, OFF}_DELAYS on pre ILK hardware Daniel Vetter (2): drm/i915: drop buggy write to FDI_RX_CHICKEN register drm/i915: resurrect panel lid handling Jani Nikula (1): drm/i915/sdvo: kfree the intel_sdvo_connector, not drm_connector, on destroy Jean Delvare (1): drm/i915: Optimize DIV_ROUND_CLOSEST call Paulo Zanoni (10): drm/i915: don't limit Haswell CRT encoder to pipe A drm/i915: use cpu/pch transcoder on intel_enable_pipe drm/i915: fix false positive "Unclaimed write" messages drm/i915: make DP work on LPT-LP machines drm/i915: don't intel_crt_init if DDI A has 4 lanes drm/i915: make the panel fitter work on pipes B and C on IVB drm/i915: make the panel fitter work on pipes B and C on Haswell drm/i915: fix intel_ddi_get_cdclk_freq for ULT machines drm/i915: implement WaMbcDriverBootEnable on Haswell drm/i915: promote Haswell to full support Rob Clark (2): drm/i915: use drm_send_vblank_event() helper drm/i915: drm_connector_property -> drm_object_property Takashi Iwai (1): drm/i915: Enable DP audio for Haswell Wei Shun Chang (1): drm/i915: add LynxPoint-LP PCH ID drivers/gpu/drm/i915/i915_drv.c| 26 --- drivers/gpu/drm/i915/i915_drv.h| 13 +++- drivers/gpu/drm/i915/i915_gem.c| 102 drivers/gpu/drm/i915/i915_gem_execbuffer.c |9 --- drivers/gpu/drm/i915/i915_gem_gtt.c| 14 ++-- drivers/gpu/drm/i915/i915_reg.h|4 ++ drivers/gpu/drm/i915/i915_suspend.c|8 --- drivers/gpu/drm/i915/intel_bios.c |3 +- drivers/gpu/drm/i915/intel_crt.c |2 +- drivers/gpu/drm/i915/intel_ddi.c | 11 +++ drivers/gpu/drm/i915/intel_display.c | 80 +++--- drivers/gpu/drm/i915/intel_dp.c|2 +- drivers/gpu/drm/i915/intel_drv.h |4 +- drivers/gpu/drm/i915/intel_hdmi.c |2 +- drivers/gpu/drm/i915/intel_lvds.c | 24 --- drivers/gpu/drm/i915/intel_modes.c |4 +- drivers/gpu/drm/i915/intel_panel.c | 25 +++ drivers/gpu/drm/i915/intel_pm.c| 25 ++- drivers/gpu/drm/i915/intel_ringbuffer.c|7 +- drivers/gpu/drm/i915/intel_sdvo.c | 64 + drivers/gpu/drm/i915/intel_tv.c | 14 ++-- 21 files changed, 264 insertions(+), 179 dele
Re: [PATCH 2/6] i915: convert struct spinlock to spinlock_t
warning: symbol > 'intel_prepare_ddi_buffers' was not declared. Should it be static? > drivers/gpu/drm/i915/intel_ddi.c:1036:34: warning: mixing different enum types > drivers/gpu/drm/i915/intel_ddi.c:1036:34: int enum pipe versus > drivers/gpu/drm/i915/intel_ddi.c:1036:34: int enum transcoder > CC [M] drivers/gpu/drm/i915/intel_ddi.o > drivers/gpu/drm/i915/intel_ddi.c: In function ‘intel_ddi_setup_hw_pll_state’: > drivers/gpu/drm/i915/intel_ddi.c:1129:2: warning: ‘port’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > drivers/gpu/drm/i915/intel_ddi.c::12: note: ‘port’ was declared here > CHECK drivers/gpu/drm/i915/intel_dp.c > CC [M] drivers/gpu/drm/i915/intel_dp.o > CHECK drivers/gpu/drm/i915/intel_hdmi.c > CC [M] drivers/gpu/drm/i915/intel_hdmi.o > CHECK drivers/gpu/drm/i915/intel_sdvo.c > CC [M] drivers/gpu/drm/i915/intel_sdvo.o > CHECK drivers/gpu/drm/i915/intel_modes.c > CC [M] drivers/gpu/drm/i915/intel_modes.o > CHECK drivers/gpu/drm/i915/intel_panel.c > CC [M] drivers/gpu/drm/i915/intel_panel.o > CHECK drivers/gpu/drm/i915/intel_pm.c > drivers/gpu/drm/i915/intel_pm.c:2173:1: warning: symbol 'mchdev_lock' was not > declared. Should it be static? > CC [M] drivers/gpu/drm/i915/intel_pm.o > CHECK drivers/gpu/drm/i915/intel_i2c.c > CC [M] drivers/gpu/drm/i915/intel_i2c.o > CHECK drivers/gpu/drm/i915/intel_fb.c > CC [M] drivers/gpu/drm/i915/intel_fb.o > CHECK drivers/gpu/drm/i915/intel_tv.c > CC [M] drivers/gpu/drm/i915/intel_tv.o > CHECK drivers/gpu/drm/i915/intel_dvo.c > CC [M] drivers/gpu/drm/i915/intel_dvo.o > CHECK drivers/gpu/drm/i915/intel_ringbuffer.c > CC [M] drivers/gpu/drm/i915/intel_ringbuffer.o > CHECK drivers/gpu/drm/i915/intel_overlay.c > CC [M] drivers/gpu/drm/i915/intel_overlay.o > CHECK drivers/gpu/drm/i915/intel_sprite.c > CC [M] drivers/gpu/drm/i915/intel_sprite.o > CHECK drivers/gpu/drm/i915/intel_opregion.c > CC [M] drivers/gpu/drm/i915/intel_opregion.o > CHECK drivers/gpu/drm/i915/dvo_ch7xxx.c > CC [M] drivers/gpu/drm/i915/dvo_ch7xxx.o > CHECK drivers/gpu/drm/i915/dvo_ch7017.c > CC [M] drivers/gpu/drm/i915/dvo_ch7017.o > CHECK drivers/gpu/drm/i915/dvo_ivch.c > CC [M] drivers/gpu/drm/i915/dvo_ivch.o > CHECK drivers/gpu/drm/i915/dvo_tfp410.c > CC [M] drivers/gpu/drm/i915/dvo_tfp410.o > CHECK drivers/gpu/drm/i915/dvo_sil164.c > CC [M] drivers/gpu/drm/i915/dvo_sil164.o > CHECK drivers/gpu/drm/i915/dvo_ns2501.c > CC [M] drivers/gpu/drm/i915/dvo_ns2501.o > CHECK drivers/gpu/drm/i915/i915_gem_dmabuf.c > CC [M] drivers/gpu/drm/i915/i915_gem_dmabuf.o > CHECK drivers/gpu/drm/i915/i915_ioc32.c > CC [M] drivers/gpu/drm/i915/i915_ioc32.o > CHECK drivers/gpu/drm/i915/intel_acpi.c > CC [M] drivers/gpu/drm/i915/intel_acpi.o > LD [M] drivers/gpu/drm/i915/i915.o > Building modules, stage 2. > MODPOST 1 modules > CC drivers/gpu/drm/i915/i915.mod.o > LD [M] drivers/gpu/drm/i915/i915.ko > > Cc: Daniel Vetter > Cc: intel-...@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Reported-by: Hauke Mehrtens > Signed-off-by: Luis R. Rodriguez Queued for -next (3.9), thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 3.8.2->3.8.3 i915 regression: GMBUS [i915 gmbus dpb] timed out, falling back to bit banging on pin 5
On Sat, Mar 16, 2013 at 11:35 AM, Arkadiusz Miskiewicz wrote: > On Thursday 14 of March 2013, Arkadiusz Miskiewicz wrote: >> Hello. >> >> After upgrading from 3.8.2 to 3.8.3 I'm getting regression : > > More people hits this: > https://bugzilla.redhat.com/show_bug.cgi?id=922304 > https://bugs.archlinux.org/task/34327 > (seems always GM45 gpu in these reports) > > archlinux people also noticed that xrandr reports VGA1 as connected (while in > reality nothing is connected to VGA1) Can you please test whether 3.9-rc kernels are affected, too? We need to know this since stable rules mandate that a regression is fixed on upstream first. Once that's figured out we can backport a fix (if 3.9-rc works) or start working on a fix for 3.8-rc kernels first and backport afterwards. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [pull] drm-intel-next
On Fri, Mar 15, 2013 at 3:11 AM, Stéphane Marchesin wrote: >> drm/i915: read out the modeset hw state at load and resume time > This commit regresses modeset on the samsung series 5 chromebook (it > is basically a pineview machine with an lvds panel). I don't seem to > be able to set any mode on it any longer. Does that mean the kernel refuses to set the mode, or that you get a black screen? For starters I guess we need: - drm.debug=0xe dmesg from just before that commit - same for latest 3.9-rc kernels, presuming it's not broken there Latest upstream has a minor chance to work better I think since we've improved the pfit handling in the setup and teardown sequence a bit. Generally lvds has been hit&miss on way too many machines unfortunately with things randomly breaking and getting fixed again (e.g. one of Chris' machines works again with the new code ...). And the commit above doesn't really change much in the code itself but it does change the order (and timing) of the different enable/disable codepaths. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer
On Sat, Mar 16, 2013 at 11:19 AM, Chris Wilson wrote: > On Fri, Mar 15, 2013 at 04:49:42PM -0700, Ben Widawsky wrote: >> On Fri, Mar 15, 2013 at 10:06:19PM +, Chris Wilson wrote: >> > On Fri, Mar 15, 2013 at 09:36:07AM -0700, Ben Widawsky wrote: >> > > On Fri, Mar 15, 2013 at 08:24:03AM +, Chris Wilson wrote: >> > > > That's what I thought too. Looking at the stack trace, the empirical >> > > > evidence is that we need the check. >> > > > -Chris >> > > >> > > I think we need to investigate the issue more then, or put a BUG_ON() in >> > > the drm code and run it through trinity. We have other places where arg >> > > can't/shouldn't be NULL and we don't check. >> > >> > Actually we are both wrong. drm_ioctl() does not validate that the >> > user struct matches the expected size, just ensures that if that user >> > cmd specifies that the arg is to be used that it only up to the known >> > size is copied. >> > >> > A hostile userspace can bludgen a NULL pointer through drm_ioctl() into >> > the driver->ioctl->func(). >> >> > > > + if (args == NULL) >> > > > + return -EINVAL; >> > > > + >> >> I must be failing to see the obvious, but I'm still not getting how args >> can ever be NULL. kdata which is passed to us as "data" and cast to >> "args' is either always some stack variable, or some kmalloc'd memory. I >> see how the arguments themselves can be crap which is really only when >> user size != drv_size. >> >> So tell me, which case can result in a NULL arg? >> 1. user size == drv_size // better not be this one >> 2. user size < driver size >> 3. user size > driver size >> >> It seems to me we still must [simply] be missing something in our >> parameter validation. > > If *userspace* doesn't request either IOC_IN | IOC_OUT in their ioctl > command (which are seperate from the ioctl number), then kdata is set to > NULL. Doesn't that mean that we need these checks everywhere? Or at least a fixup in drm core proper? And I think we need to add trinity to our test setup eventually ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Revert "drm/i915: try to train DP even harder"
On Mon, Mar 11, 2013 at 06:40:16PM +0100, Takashi Iwai wrote: > This reverts commit 0d71068835e2610576d369d6d4cbf90e0f802a71. > > Not only that the commit introduces a bogus check (voltage_tries == 5 > will never meet at the inserted code path), it brings the i915 driver > into an endless dp-train loop on HP Z1 desktop machine with IVY+eDP. > > At least reverting this commit recovers the framebuffer (but X is > still broken by other reasons...) > > Cc: > Signed-off-by: Takashi Iwai Picked up for -fixes, thanks for the patch. Adding Paulo since it's his patch. To assign proper blame please cc: relevant people when sending out reverts. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] drm/i915: Use the fixed pixel clock for eDP in intel_dp_set_m_n()
On Tue, Mar 12, 2013 at 04:32:28PM +0100, Takashi Iwai wrote: > The eDP output on HP Z1 is still broken when X is started even after > fixing the infinite link-train loop. The regression was introduced in > 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c. > > In the past, the clock of the reference mode was modified in > intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was > used for calculating in intel_dp_set_m_n(). This override was removed, > thus the wrong mode clock is used for the calculation, resulting in a > psychedelic smoking output in the end. > > This patch corrects the clock to be used in the place. > > Cc: > Signed-off-by: Takashi Iwai I truly hate this mess of dotclock vs portclock vs. whatever. Can you pls apply a little bikeshed and use the existing intel_edp_target_clock like in ironlake_set_m_n? And if you have the regressing commit a little citation to assign the blame (it's probably me) would be good. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt responses)
On Fri, Mar 15, 2013 at 08:47:39AM -0700, Greg KH wrote: > On Fri, Mar 15, 2013 at 04:37:56PM +0100, Jiri Kosina wrote: > > On Fri, 15 Mar 2013, Greg KH wrote: > > > > > > > I have the same problem on my Lenovo T500. I think the graphics card > > > > > is > > > > > involved. > > > > > > > > > > This laptop has "hybrid graphics" - one Intel GMA 4500MHD and one ATI > > > > > Mobility Radeon HD 3650. When I boot with the Intel card, I get "irq > > > > > 16: > > > > > nobody cared" during boot, not when I boot with the ATI card. > > > > > > > > Confirming this. After a lot of hassle, I have bisected this reliably to > > > > > > > > commit 28c70f162a315bdcfbe0bf940a740ef8bfb918d6 > > > > Author: Daniel Vetter > > > > Date: Sat Dec 1 13:53:45 2012 +0100 > > > > > > > > drm/i915: use the gmbus irq for waits > > > > > > > > Adding Daniel, Imre and Daniel to CC while I will try to figure out > > > > what's > > > > happening in parallel. > > > > > > Wasn't this fixed by the merge from David > > > (2cc79544bd0aabb4b3cf467ead5df526d9134c64)? > > > > Why do you think it should, please? > > The line: > - Fix PCH irq handling race which resulted in missed gmbus/dp > aux irqs and subsequent fallout (Paulo) > > > (I am seeing this with a2362d247 still). > > Ok, I guess it isn't still fixed properly, just was guessing :) Yeah, the above fix is for pch split platforms, whereas these reports here are for gm45 (which doesn't have the pch display split). Acking of gmbus interrupts works differently on those, I'm testing right now whether I can reproduce this fail. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3
On Sat, Mar 16, 2013 at 01:28:50PM +0100, Richard Cochran wrote: > > I have an Acer Aspire One netbook, and on it I get the following > warning when closing and opening the lid. I think this warning first > appeared in 3.7. > > Does this need fixing? If so, who can do it? Another pesky BIOS which changes the display state behind our back on lid closing! Should be duct-tapped over with commit 45e2b5f640b3766da3eda48f6c35f088155c06f3 Author: Daniel Vetter Date: Fri Nov 23 18:16:34 2012 +0100 drm/i915: force restore on lid open which is in 3.8. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Revert a bunch of patches in stable kernels
Hi Greg&all, So a recent stable backport to fix rc6 on ilk (which is disabled by default and with dubious power savings at best, unlike rc6 on snb and later) totally blew up all over the place: https://bugzilla.kernel.org/show_bug.cgi?id=55291 https://lkml.org/lkml/2013/3/14/540 There might be more, I'm still recovering from mail floods due to traveling last week. I think the right course of action is to revert the offending patch (plus anything depending upon it) and give up on rc6 on ilk in 3.8 - too messy. All bug reports confirmed that 3.9-rc kernels work as expected. Upstream commits to revert: 15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9 drm/i915: enable irqs earlier when resuming 52d7ecedac3f96fb562cb482c139015372728638 drm/i915: reorder setup sequence to have irqs for output setup Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] drm/i915: Use the fixed pixel clock for eDP in intel_dp_set_m_n()
On Mon, Mar 18, 2013 at 10:29:16AM +0100, Takashi Iwai wrote: > At Sun, 17 Mar 2013 23:12:03 +0100, > Daniel Vetter wrote: > > > > On Tue, Mar 12, 2013 at 04:32:28PM +0100, Takashi Iwai wrote: > > > The eDP output on HP Z1 is still broken when X is started even after > > > fixing the infinite link-train loop. The regression was introduced in > > > 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c. > > > > > > In the past, the clock of the reference mode was modified in > > > intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was > > > used for calculating in intel_dp_set_m_n(). This override was removed, > > > thus the wrong mode clock is used for the calculation, resulting in a > > > psychedelic smoking output in the end. > > > > > > This patch corrects the clock to be used in the place. > > > > > > Cc: > > > Signed-off-by: Takashi Iwai > > > > I truly hate this mess of dotclock vs portclock vs. whatever. Can you pls > > apply a little bikeshed and use the existing intel_edp_target_clock like > > in ironlake_set_m_n? And if you have the regressing commit a little > > citation to assign the blame (it's probably me) would be good. > > OK, the revised patch is below. Picked up for -fixes, thanks for the patch. -Daniel > > > thanks, > > Takashi > > --- > From: Takashi Iwai > Subject: [PATCH v2] drm/i915: Use the fixed pixel clock for eDP in > intel_dp_set_m_n() > > The eDP output on HP Z1 is still broken when X is started even after > fixing the infinite link-train loop. The regression was introduced in > 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c > by the commit [71244653: drm/i915: adjusted_mode->clock in the dp > mode_fix]. > > In the past, the clock of the reference mode was modified in > intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was > used for calculating in intel_dp_set_m_n(). This override was removed, > thus the wrong mode clock is used for the calculation, resulting in a > psychedelic smoking output in the end. > > This patch corrects the clock to be used in the place. > > v1->v2: Use intel_edp_target_clock() for checking eDP fixed clock > instead of open code as in ironlake_set_m_n(). > > Cc: > Signed-off-by: Takashi Iwai > --- > drivers/gpu/drm/i915/intel_dp.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 6f728e5..2606811 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -820,6 +820,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct > drm_display_mode *mode, > struct intel_link_m_n m_n; > int pipe = intel_crtc->pipe; > enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder; > + int target_clock; > > /* >* Find the lane count in the intel_encoder private > @@ -835,13 +836,22 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct > drm_display_mode *mode, > } > } > > + target_clock = mode->clock; > + for_each_encoder_on_crtc(dev, crtc, intel_encoder) { > + if (intel_encoder->type == INTEL_OUTPUT_EDP) { > + target_clock = intel_edp_target_clock(intel_encoder, > + mode); > + break; > + } > + } > + > /* >* Compute the GMCH and Link ratios. The '3' here is >* the number of bytes_per_pixel post-LUT, which we always >* set up for 8-bits of R/G/B, or 3 bytes total. >*/ > intel_link_compute_m_n(intel_crtc->bpp, lane_count, > -mode->clock, adjusted_mode->clock, &m_n); > +target_clock, adjusted_mode->clock, &m_n); > > if (IS_HASWELL(dev)) { > I915_WRITE(PIPE_DATA_M1(cpu_transcoder), > -- > 1.8.2 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Revert a bunch of patches in stable kernels
On Mon, Mar 18, 2013 at 11:05 AM, Daniel Vetter wrote: > Hi Greg&all, > > So a recent stable backport to fix rc6 on ilk (which is disabled by > default and with dubious power savings at best, unlike rc6 on snb and > later) totally blew up all over the place: > https://bugzilla.kernel.org/show_bug.cgi?id=55291 > https://lkml.org/lkml/2013/3/14/540 This likely also is the culprit behind the ghost vga issues reported on gm45, e.g: https://bugzilla.redhat.com/show_bug.cgi?id=922304 Again, fixed in 3.9-rc kernels by commit 20afbda209d708be66944907966486d0c1331cb8 Author: Daniel Vetter Date: Tue Dec 11 14:05:07 2012 +0100 drm/i915: Fixup hpd irq register setup ordering Cheers, Daniel > There might be more, I'm still recovering from mail floods due to > traveling last week. I think the right course of action is to revert > the offending patch (plus anything depending upon it) and give up on > rc6 on ilk in 3.8 - too messy. All bug reports confirmed that 3.9-rc > kernels work as expected. Upstream commits to revert: > > 15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9 drm/i915: enable irqs earlier > when resuming > 52d7ecedac3f96fb562cb482c139015372728638 drm/i915: reorder setup > sequence to have irqs for output setup > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt responses)
On Mon, Mar 18, 2013 at 10:12:49AM +0100, Jiri Kosina wrote: > On Fri, 15 Mar 2013, Yinghai Lu wrote: > > > > Just a datapoint -- I have put a trivial debugging patch in place, and it > > > reveals that "nobody cared" for irq 16 happens long after last > > > > > > I915_WRITE(GMBUS4 + reg_offset, 0); > > > > > > has been performed in gmbus_wait_hw_status(). On the other hand, if I > > > comment out both GMBUS4 register offset writes in gmbus_wait_hw_status(), > > > then it of course falls back to GPIO bit-banging, but the "nobody cared" > > > for irq 16 is gone. > > > > > > So it seems like something gets severely confused by the I915_WRITE to > > > GMBUS4 + reg_offset. So far this seems to have been reported solely on > > > Lenovos as far as I can see (although a completely different types), so it > > > might be some platform-specific quirk? > > > > > > Honestly, I still don't understand how all the GMBUS stuff relates to IRQ > > > 16 at all. > > > > that device is using > > i915 :00:02.0: irq 44 for MSI/MSI-X > > > > so can you try to boot with pci=nomsi? > > Yes, switching from MSI to IO-APIC-fasteoi makes the report about lost > interrupts go away. > > My understanding from the other mail is that DAniel Vetter already has an > idea what might be going wrong with IRQ acking on GM45 chipsets; hopefully > this datapoint regarding MSI will fit into it. Yep, there's a big comment in the irq handler for that chipset that we have a gaping race with when using MSI interrupts. Although the comment bodly claims that the race is small enough to avoid the dreaded "nobody cared" message. Looks like gmbus is good at hitting that race - on newer chips it already brought up a similar race in handling pch interrupts. Can you please give the below patch a whirl? It removes the probably race msi race avoidance code and replaces it with the same trick Paulo used to fix pch irq handling races. Thanks, Daniel --- diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 3c7bb04..13de12e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2684,7 +2684,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) { struct drm_device *dev = (struct drm_device *) arg; drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 iir, new_iir; + u32 iir, new_iir, ier; u32 pipe_stats[I915_MAX_PIPES]; unsigned long irqflags; int irq_received; @@ -2692,9 +2692,14 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) atomic_inc(&dev_priv->irq_received); + /* irq race avoidance, copy&pasta from Paulo's PCH irq fix */ + ier = I915_READ(IER); + I915_WRITE(IER, 0); + POSTING_READ(IER); + iir = I915_READ(IIR); - for (;;) { + do { bool blc_event = false; irq_received = iir != 0; @@ -2792,7 +2797,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) * stray interrupts. */ iir = new_iir; - } + } while (0); + + I915_WRITE(IER, ier); + POSTING_READ(IER); i915_update_dri1_breadcrumb(dev); -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips (was Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt responses))
On Mon, Mar 18, 2013 at 04:56:02PM +0100, Jiri Kosina wrote: > Okay, so I think that for 3.9 we want the patch below, and if eventually > hardware root cause / workaround is found for GM45, we can have it merged > later. I'd prefer if we dig into this for a bit more. I've been traveling last week and only just now recovered from the mail backlog, hence my delays in handling this. Also the "nobody cared" showing up only very late after the last gmbus transaction isn't too surprising: It only fires after 100k interrupts, and apparently the irq handler is already a bit racy, so any interrupt might push it over the edge. gmbus interrupts are apparently just help to expose the race (presuming it's indeed the msi race already docuemented in the code). -Daniel > From: Jiri Kosina > Subject: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips > > Commit 28c70f162 ("drm/i915: use the gmbus irq for waits") switched to > using GMBUS irqs instead of GPIO bit-banging for chipset generations 4 > and above. > > It turns out though that on many systems this leads to spurious interrupts > being generated, long after the register write to disable the IRQs has been > issued. > > Flushing of the register writes by POSTING_READ() directly after the register > write doesn't work either. > > Disable using of GMBUS IRQs on Gen4 systems before the root cause is found and > revert back to old behavior. > > Also be more careful about not issuing GMBUS4 register reads in > gmbus_wait_hw_status() if we are not using GMBUS IRQs. > > Signed-off-by: Jiri Kosina > --- > drivers/gpu/drm/i915/intel_i2c.c | 12 ++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c > b/drivers/gpu/drm/i915/intel_i2c.c > index acf8aec..8638036 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -58,12 +58,14 @@ to_intel_gmbus(struct i2c_adapter *i2c) > return container_of(i2c, struct intel_gmbus, adapter); > } > > +#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 5) > void > intel_i2c_reset(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0); > - I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0); > + if (HAS_GMBUS_IRQ(dev)) > + I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0); > } > > static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool > enable) > @@ -203,7 +205,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) > algo->data = bus; > } > > -#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4) > static int > gmbus_wait_hw_status(struct drm_i915_private *dev_priv, >u32 gmbus2_status, > @@ -214,6 +215,13 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv, > u32 gmbus2 = 0; > DEFINE_WAIT(wait); > > + if (!HAS_GMBUS_IRQ(dev_priv->dev)) { > + int ret; > + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & > + (GMBUS_SATOER | gmbus2_status), > + 50); > + return ret; > + } > /* Important: The hw handles only the first bit, so set only one! Since >* we also need to check for NAKs besides the hw ready/idle signal, we >* need to wake up periodically and check that ourselves. */ > > -- > Jiri Kosina > SUSE Labs -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Stable kernel 3.8.3 appears to break displayport on intel gen4
On Mon, Mar 18, 2013 at 04:59:35PM +0100, Bjørn Mork wrote: > Sergio Callegari writes: > > > This is just a short note to let you know that after installing 3.8.3, > > display port stopped working on my laptop. Going back to 3.8.2 brought > > it back to life. > > This has been tested with the ubuntu mainline kernels that should be > > vanilla stable kernels. Hope this is not an incorrect report due to > > something wrong on their side. Laptop is a DELL E6500 with intel gen4 > > integrated graphics (Intel Corporation Mobile 4 Series Chipset > > Integrated Graphics Controller (rev 07)). > > With 3.8.3, xrandr does not report anymore the external monitor when > > it is actually attached via displayport. > > I can confirm seeing this bug on: > > # lspci -nnvvvs 00:02.0 > 00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 4 Series > Chipset Integrated Graphics Controller [8086:2a42] (rev 07) (prog-if 00 [VGA > controller]) > Subsystem: Lenovo Device [17aa:20e4] > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- > SERR- Latency: 0 > Interrupt: pin A routed to IRQ 45 > Region 0: Memory at f000 (64-bit, non-prefetchable) [size=4M] > Region 2: Memory at d000 (64-bit, prefetchable) [size=256M] > Region 4: I/O ports at 1800 [size=8] > Expansion ROM at [disabled] > Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- > Address: fee0300c Data: 4152 > Capabilities: [d0] Power Management version 3 > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA > PME(D0-,D1-,D2-,D3hot-,D3cold-) > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- > Kernel driver in use: i915 > > > > Looking through the changes from v3.8.2 to v3.8.3, there weren't really > that many suspects. This partial (to avoid having to change any > following patches) revert of commit 2a98104 ("drm/i915: reorder setup > sequence to have irqs for output setup") fixes the problem for me. I > have no idea why, so I leave it to Daniel and the other wise men working > on this driver to explain and cleanup :) Already taken care of hopefully: http://lists.freedesktop.org/archives/intel-gfx/2013-March/025767.html My apologies to everyone who's suffering through this breakage, it looks like I've managed to serious burn myself with an innocent looking stable backport :( Yours, Daniel > > > --- > drivers/gpu/drm/i915/i915_dma.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 5206f24..b15b65d 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1297,21 +1297,19 @@ static int i915_load_modeset_init(struct drm_device > *dev) > if (ret) > goto cleanup_vga_switcheroo; > > - ret = drm_irq_install(dev); > - if (ret) > - goto cleanup_gem_stolen; > - > - /* Important: The output setup functions called by modeset_init need > - * working irqs for e.g. gmbus and dp aux transfers. */ > intel_modeset_init(dev); > > ret = i915_gem_init(dev); > if (ret) > - goto cleanup_irq; > + goto cleanup_gem_stolen; > + > + intel_modeset_gem_init(dev); > > INIT_WORK(&dev_priv->console_resume_work, intel_console_resume); > > - intel_modeset_gem_init(dev); > + ret = drm_irq_install(dev); > + if (ret) > + goto cleanup_gem; > > /* Always safe in the mode setting case. */ > /* FIXME: do pre/post-mode set stuff in core KMS code */ > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
gm45 intel gfx can generate non-MSI irq# in MSI mode (was Re: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips (was Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt responses
On Tue, Mar 19, 2013 at 10:03 AM, Chris Wilson wrote: >> > How about just using: >> > if (!HAS_GMBUS_IRQ(dev_priv->dev)) gmbus4_irq_en = 0; >> > and the existing wait loop? >> >> I explicitly wanted to avoid touching GMBUS4 register, as the real cause >> of the failure is not clear. >> >> But, as Yinghai Lu points out, the problem is most likely caused by >> interrupt disabling not working properly (see his very good point >> regarding DisINTx+ and INTx+ discrepancy), so zeroing the register out >> should work and it indeed does in my case, hence the (tested) patch >> below. >> >> I think it's a 3.9-rc material, and I am all open to debug this further >> for 3.10 so that the race is closed and gmbus irqs can be used on Gen4 >> platform properly. > > Agreed. Using the IRQ for GMBUS is just a performance feature that can > be deferred until after we determine the root cause - and hope that the > failure is somehow peculiar to GMBUS. Ok, I've merged this patch. But some further investigation points at a much more severe dragon hiding here: The MSI interrupt for the intel gfx is commonly in the 40+ range, but the interrupt vector with the spurious interrupts is 16. Which is the irq of the intel gfx when MSI is disabled! So it looks like gmbus on the intel gfx is capable of generating non-MSI interrupts in parallel to the MSI interrupts (since apparently gmbus still works, so we get the interrupts we expect). I have no idea how that could happen. Hence adding a bunch of people with more clue than me. For reference below the updated commit message. Cheers, Daniel Author: Jiri Kosina Date: Tue Mar 19 09:56:57 2013 +0100 drm/i915: stop using GMBUS IRQs on Gen4 chips Commit 28c70f162 ("drm/i915: use the gmbus irq for waits") switched to using GMBUS irqs instead of GPIO bit-banging for chipset generations 4 and above. It turns out though that on many systems this leads to spurious interrupts being generated, long after the register write to disable the IRQs has been issued. Typically this results in the spurious interrupt source getting disabled: [9.636345] irq 16: nobody cared (try booting with the "irqpoll" option) [9.637915] Pid: 4157, comm: ifup Tainted: GF 3.9.0-rc2-00341-g0863702 #422 [9.639484] Call Trace: [9.640731][] __report_bad_irq+0x1d/0xc7 [9.640731] [] note_interrupt+0x15b/0x1e8 [9.640731] [] handle_irq_event_percpu+0x1bf/0x214 [9.640731] [] handle_irq_event+0x3c/0x5c [9.640731] [] handle_fasteoi_irq+0x7a/0xb0 [9.640731] [] handle_irq+0x1a/0x24 [9.640731] [] do_IRQ+0x48/0xaf [9.640731] [] common_interrupt+0x6a/0x6a [9.640731][] ? system_call_fastpath+0x16/0x1b [9.640731] handlers: [9.640731] [] usb_hcd_irq [usbcore] [9.640731] [] yenta_interrupt [yenta_socket] [9.640731] Disabling IRQ #16 The really curious thing is now that irq 16 is _not_ the interrupt for the i915 driver when using MSI, but it _is_ the interrupt when not using MSI. So by all indications it seems like gmbus is able to generate a legacy (shared) interrupt in MSI mode on some configurations. I've tried to reproduce this and the differentiating thing seems to be that on unaffected systems no other device uses irq 16 (which seems to be the non-MSI intel gfx interrupt on all gm45). I have no idea how that even can happen. To avoid tempting this elephant into a rage, just disable gmbus interrupt support on gen 4. v2: Improve the commit message with exact details of what's going on. Also add a comment in the code to warn against this particular elephant in the room. Signed-off-by: Jiri Kosina (v1) Acked-by: Chris Wilson (v1) References: https://lkml.org/lkml/2013/3/8/325 Signed-off-by: Daniel Vetter -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: gm45 intel gfx can generate non-MSI irq# in MSI mode (was Re: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips (was Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt respo
On Tue, Mar 19, 2013 at 4:38 PM, Alan Stern wrote: > On Tue, 19 Mar 2013, Daniel Vetter wrote: > >> For reference below the updated commit message. >> >> Cheers, Daniel >> >> Author: Jiri Kosina >> Date: Tue Mar 19 09:56:57 2013 +0100 > >> >> drm/i915: stop using GMBUS IRQs on Gen4 chips >> >> Commit 28c70f162 ("drm/i915: use the gmbus irq for waits") switched to >> using GMBUS irqs instead of GPIO bit-banging for chipset generations 4 >> and above. >> >> It turns out though that on many systems this leads to spurious >> interrupts >> being generated, long after the register write to disable the IRQs has >> been >> issued. >> >> Typically this results in the spurious interrupt source getting >> disabled: >> >> [9.636345] irq 16: nobody cared (try booting with the "irqpoll" >> option) >> [9.637915] Pid: 4157, comm: ifup Tainted: GF >> 3.9.0-rc2-00341-g0863702 #422 >> [9.639484] Call Trace: >> [9.640731][] __report_bad_irq+0x1d/0xc7 >> [9.640731] [] note_interrupt+0x15b/0x1e8 >> [9.640731] [] handle_irq_event_percpu+0x1bf/0x214 >> [9.640731] [] handle_irq_event+0x3c/0x5c >> [9.640731] [] handle_fasteoi_irq+0x7a/0xb0 >> [9.640731] [] handle_irq+0x1a/0x24 >> [9.640731] [] do_IRQ+0x48/0xaf >> [9.640731] [] common_interrupt+0x6a/0x6a >> [9.640731][] ? >> system_call_fastpath+0x16/0x1b >> [9.640731] handlers: >> [9.640731] [] usb_hcd_irq [usbcore] >> [9.640731] [] yenta_interrupt [yenta_socket] >> [9.640731] Disabling IRQ #16 >> >> The really curious thing is now that irq 16 is _not_ the interrupt for >> the i915 driver when using MSI, but it _is_ the interrupt when not >> using MSI. So by all indications it seems like gmbus is able to >> generate a legacy (shared) interrupt in MSI mode on some >> configurations. I've tried to reproduce this and the differentiating >> thing seems to be that on unaffected systems no other device uses irq >> 16 (which seems to be the non-MSI intel gfx interrupt on all gm45). > > That might be misleading. It's possible that the erroneous IRQs _are_ > being issued but you're simply not aware of them. If the kernel thinks > that no device is using IRQ 16 then it will leave that IRQ disabled. I guess I should have phrased it more precisely, but that's exactly what I expect is happening on my machine: I don't have anything on irq16 (i.e. in non-msi mode the gfx interrupt isn't shared) and hence the irq is completely disabled. Which obviously makes it impossible for me to reproduce the issue. To test that theory, is there a quick way to force-enable a given interrupt, short of just hacking up a 2nd dummy irq handler in my driver? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] dummy-irq: introduce a dummy IRQ handler driver (was Re: gm45 intel gfx can generate non-MSI irq# in MSI mode (was Re: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips (was Re: [3.9-r
On Thu, Mar 21, 2013 at 12:21:21AM +0100, Jiri Kosina wrote: > On Wed, 20 Mar 2013, Alan Stern wrote: > > > > Ok, so how about this? > > > Daniel, is it enough to make the problem appear on your system (by > > > building this into the kernel and booting with dummy-irq.irq=16)? > > > > > > Thanks. > > > > > > From: Jiri Kosina > > > Subject: [PATCH] dummy-irq: introduce a dummy IRQ handler driver > > > > > > This module accepts a single 'irq' parameter, which it should register > > > for. > > > > > > Its sole purpose is to help with debugging of IRQ sharing problems, by > > > force-enabling IRQ that would otherwise be disabled. > > > > > > Suggested-by: Alan Stern > > > Signed-off-by: Jiri Kosina > > > > This is just what I was thinking of. Three extremely minor > > suggestions... > > Thanks Alan. Updated version below. > > Greg, willing to merge this simple debugging facility? > > > From: Jiri Kosina > Subject: [PATCH] dummy-irq: introduce a dummy IRQ handler driver > > This module accepts a single 'irq' parameter, which it should register for. > > Its sole purpose is to help with debugging of IRQ sharing problems, by > force-enabling IRQ that would otherwise be disabled. > > Suggested-by: Alan Stern > Signed-off-by: Jiri Kosina Indeed, this is pretty useful and allowed me to quickly reproduce that phantom irq on my gm45. Thanks to module reloading we can even reset the kernel's irq disabling logic and so test different tricks quickly without rebooting. Really useful. Acked-by: Daniel Vetter Thanks, Daniel > --- > drivers/misc/Kconfig |8 ++ > drivers/misc/Makefile|1 + > drivers/misc/dummy-irq.c | 59 > ++ > 3 files changed, 68 insertions(+), 0 deletions(-) > create mode 100644 drivers/misc/dummy-irq.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index e83fdfe..69bb79d 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -93,6 +93,14 @@ config ATMEL_TCB_CLKSRC_BLOCK > TC can be used for other purposes, such as PWM generation and > interval timing. > > +config DUMMY_IRQ > + tristate "Dummy IRQ handler" > + default n > + ---help--- > + This module accepts a single 'irq' parameter, which it should > register for. > + The sole purpose of this module is to help with debugging of systems > on > + which spurious IRQs would happen on disabled IRQ vector. > + > config IBM_ASM > tristate "Device driver for IBM RSA service processor" > depends on X86 && PCI && INPUT > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 35a1463..28ff261 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_ATMEL_TCLIB) += atmel_tclib.o > obj-$(CONFIG_BMP085) += bmp085.o > obj-$(CONFIG_BMP085_I2C) += bmp085-i2c.o > obj-$(CONFIG_BMP085_SPI) += bmp085-spi.o > +obj-$(CONFIG_DUMMY_IRQ) += dummy-irq.o > obj-$(CONFIG_ICS932S401) += ics932s401.o > obj-$(CONFIG_LKDTM) += lkdtm.o > obj-$(CONFIG_TIFM_CORE) += tifm_core.o > diff --git a/drivers/misc/dummy-irq.c b/drivers/misc/dummy-irq.c > new file mode 100644 > index 000..7014167 > --- /dev/null > +++ b/drivers/misc/dummy-irq.c > @@ -0,0 +1,59 @@ > +/* > + * Dummy IRQ handler driver. > + * > + * This module only registers itself as a handler that is specified to it > + * by the 'irq' parameter. > + * > + * The sole purpose of this module is to help with debugging of systems on > + * which spurious IRQs would happen on disabled IRQ vector. > + * > + * Copyright (C) 2013 Jiri Kosina > + */ > + > +/* > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > by > + * the Free Software Foundation. > + */ > +#include > +#include > +#include > + > +static int irq; > + > +static irqreturn_t dummy_interrupt(int irq, void *dev_id) > +{ > + static int count = 0; > + > + if (count == 0) { > + printk(KERN_INFO "dummy-irq: interrupt occured on IRQ %d\n", > + irq); > + count++; > + } > + > + return IRQ_NONE; > +} > + > +static int __init dummy_irq_init(void) > +{ > + if (request_irq(irq, &dummy_interrupt, IRQF_SHARED, "dummy_irq", &
Re: [PATCH 17/62] drm/i915: convert to idr_alloc()
On Sat, Feb 02, 2013 at 05:20:18PM -0800, Tejun Heo wrote: > Convert to the much saner new idr interface. > > Only compile tested. > > Signed-off-by: Tejun Heo > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-de...@lists.freedesktop.org > --- > This patch depends on an earlier idr changes and I think it would be > best to route these together through -mm. Please holler if there's > any objection. Thanks. Indeed, this looks nice. Acked-by: Daniel Vetter -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-mm-sig] [PATCH 6/7] reservation: cross-device reservation support
On Mon, Feb 04, 2013 at 10:57:22AM +0100, Maarten Lankhorst wrote: > Op 04-02-13 08:06, Inki Dae schreef: > >> +/** > >> + * ticket_commit - commit a reservation with a new fence > >> + * @ticket:[in]the reservation_ticket returned by > >> + * ticket_reserve > >> + * @entries: [in]a linked list of struct reservation_entry > >> + * @fence: [in]the fence that indicates completion > >> + * > >> + * This function will call reservation_ticket_fini, no need > >> + * to do it manually. > >> + * > >> + * This function should be called after a hardware command submission is > >> + * completed succesfully. The fence is used to indicate completion of > >> + * those commands. > >> + */ > >> +void > >> +ticket_commit(struct reservation_ticket *ticket, > >> + struct list_head *entries, struct fence *fence) > >> +{ > >> + struct list_head *cur; > >> + > >> + if (list_empty(entries)) > >> + return; > >> + > >> + if (WARN_ON(!fence)) { > >> + ticket_backoff(ticket, entries); > >> + return; > >> + } > >> + > >> + list_for_each(cur, entries) { > >> + struct reservation_object *bo; > >> + bool shared; > >> + > >> + reservation_entry_get(cur, &bo, &shared); > >> + > >> + if (!shared) { > >> + int i; > >> + for (i = 0; i < bo->fence_shared_count; ++i) { > >> + fence_put(bo->fence_shared[i]); > >> + bo->fence_shared[i] = NULL; > >> + } > >> + bo->fence_shared_count = 0; > >> + if (bo->fence_excl) > >> + fence_put(bo->fence_excl); > >> + > >> + bo->fence_excl = fence; > >> + } else { > >> + if (WARN_ON(bo->fence_shared_count >= > >> + ARRAY_SIZE(bo->fence_shared))) { > >> + mutex_unreserve_unlock(&bo->lock); > >> + continue; > >> + } > >> + > >> + bo->fence_shared[bo->fence_shared_count++] = fence; > >> + } > > Hi, > > > > I got some questions to fence_excl and fence_shared. At the above > > code, if bo->fence_excl is not NULL then it puts bo->fence_excl and > > sets a new fence to it. This seems like that someone that committed a > > new fence, wants to access the given dmabuf exclusively even if > > someone is accessing the given dmabuf. > Yes, if there were shared fences, they had to issue a wait for the previous > exclusive fence, so if you add > (possibly hardware) wait ops on those shared fences to your command stream, > it follows that you also > waited for the previous exclusive fence implicitly. > > If there is only an exclusive fence, you have to issue some explicit wait op > on it before you start the rest > of the commands. > > On the other hand, in case of fence_shared, someone wants to access > > that dmabuf non-exclusively. So this case seems like that the given > > dmabuf could be accessed by two more devices. So I guess that the > > fence_excl could be used for write access(may need buffer sync like > > blocking) and read access for the fence_shared(may not need buffer > > sync). I'm not sure that I understand these two things correctly so > > could you please give me more comments for them? > Correct, if you create a shared fence, you still have to emit a wait op for > the exclusive fence. Just a quick note: We limit the number of non-exclusive fences to avoid reallocating memory, which would be a bit a pain. Otoh if we support some form of fence timeline concept, the fence core code could overwrite redundant fences. Which would reasonable limit the total attached fence count. Still we'd need to thread potential -ENOMEM failures through all callers. Do you see a use-case for more than just a few non-exclusive fences? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
* check and update the status of LVDS connector after receiving >* the LID nofication event. > @@ -520,21 +524,24 @@ static int intel_lid_notify(struct notifier_block *nb, > unsigned long val, > > /* Don't force modeset on machines where it causes a GPU lockup */ > if (dmi_check_system(intel_no_modeset_on_lid)) > - return NOTIFY_OK; > + goto exit; > if (!acpi_lid_open()) { > - dev_priv->modeset_on_lid = 1; > - return NOTIFY_OK; > + /* do modeset on next lid open event */ > + dev_priv->modeset = MODESET_ON_LID_OPEN; > + goto exit; > } > > - if (!dev_priv->modeset_on_lid) > - return NOTIFY_OK; > - > - dev_priv->modeset_on_lid = 0; > + if (dev_priv->modeset == MODESET_DONE) > + goto exit; > > mutex_lock(&dev->mode_config.mutex); > intel_modeset_setup_hw_state(dev, true); > mutex_unlock(&dev->mode_config.mutex); > > + dev_priv->modeset = MODESET_DONE; > + > +exit: > + mutex_unlock(&dev_priv->modeset_lock); > return NOTIFY_OK; > } > > -- > 1.7.9.5 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH V2 3/3] i915: ignore lid open event when resuming
(&dev_priv->modeset_restore_lock); > + if (dev_priv->modeset_restore == MODESET_SUSPENDED) > + goto exit; > /* >* check and update the status of LVDS connector after receiving >* the LID nofication event. > @@ -520,21 +524,24 @@ static int intel_lid_notify(struct notifier_block *nb, > unsigned long val, > > /* Don't force modeset on machines where it causes a GPU lockup */ > if (dmi_check_system(intel_no_modeset_on_lid)) > - return NOTIFY_OK; > + goto exit; > if (!acpi_lid_open()) { > - dev_priv->modeset_on_lid = 1; > - return NOTIFY_OK; > + /* do modeset on next lid open event */ > + dev_priv->modeset_restore = MODESET_ON_LID_OPEN; > + goto exit; > } > > - if (!dev_priv->modeset_on_lid) > - return NOTIFY_OK; > - > - dev_priv->modeset_on_lid = 0; > + if (dev_priv->modeset_restore == MODESET_DONE) > + goto exit; > > mutex_lock(&dev->mode_config.mutex); > intel_modeset_setup_hw_state(dev, true); > mutex_unlock(&dev->mode_config.mutex); > > + dev_priv->modeset_restore = MODESET_DONE; > + > +exit: > + mutex_unlock(&dev_priv->modeset_restore_lock); > return NOTIFY_OK; > } > > -- > 1.7.9.5 > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[pull] drm-intel-next for 3.9
Hi Dave, Probably the last feature pull for 3.9, there's some fixes outstanding thought that I'd like to sneak in. And maybe 3.8 takes a bit longer ... Anyway, highlights of this pull: - Kill the horrible IS_DISPLAYREG hack to handle the mmio offset movements on vlv, big thanks to Ville. - Dynamic power well support for Haswell, shaves away a bit when only using the eDP port on pipe A (Paulo). Plus unclaimed register fixes uncovered by this. - Clarifications of the gpu hang/reset state transitions, hopefully fixing a few spurious -EIO deaths in userspace. - Haswell ELD fixes. - Some more (pp)gtt cleanups from Ben. - A few smaller things all over. Plus all the stuff from the previous rather small pull request: - Broadcast RBG improvements and reduced color range fixes from Ville. - Ben is on a "kill legacy gtt code for good" spree, first pile of patches included. - No-relocs and bo lut improvements for faster execbuf from Chris. - Some refactorings from Imre. QA filed tons of bugs this cycle, all due to us finally enabling all the really anal timestamp and vblank counter checks in kms_flip, now that the locking is fixed and EDID reads don't cause stalls any more. In short, our vblank code seems to be ridiculously racy and broken :( Otoh that's not really news. Cheers, Daniel The following changes since commit b5cc6c0387b2f8d269c1df1e68c97c958dd22fed: Merge tag 'drm-intel-next-2012-12-21' of git://people.freedesktop.org/~danvet/drm-intel into drm-next (2013-01-17 20:34:08 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-next-2013-02-01 for you to fetch changes up to 7d37beaaf3dbc6ff16f4d32a4dd6f8c557c6ab50: GPU/i915: Fix acpi_bus_get_device() check in drivers/gpu/drm/i915/intel_opregion.c (2013-02-01 11:01:50 +0100) Ben Widawsky (16): drm/i915: Kill gtt_end drm/i915: Mappable_end can't ever be > end drm/i915: Remove gtt_mappable_total drm/i915: Create a gtt structure drm/i915: Remove use on gma_bus_addr on gen6+ drm/i915: Remove use of gtt_mappable_entries drm/i915: Cut out the infamous ILK w/a from AGP layer drm/i915: Remove scratch page from shared drm/i915: Needs_dmar, not agp/intel: Add gma_bus_addr drm/i915: Implement WaVSRefCountFullforceMissDisable drm/i915: Error state should print /sys/kernel/debug drm/i915: Add probe and remove to the gtt ops drm/i915: remove intel_gtt structure drm/i915: Reclaim GTT space for failed PPGTT drm/i915: Fix CAGF for HSW Changlong Xie (1): drm/i915: gen6_gmch_remove can be static Chris Wilson (9): drm/i915: Add a debug interface to forcibly evict and shrink our object caches drm/i915: Bail if we attempt to allocate pages for a purged object drm/i915: Mark a temporary allocation for copy-from-user as such drm/i915: Take the handle idr spinlock once for looking up the exec objects drm/i915: Move the execbuffer objects list from the stack into the tracker drm/i915: Use the reloc.handle as an index into the execbuffer array drm/i915: Only insert the mb() before updating the fence parameter drm/i915: Only apply the mb() when flushing the GTT domain during a finish drm/i915: Only run idle processing from i915_gem_retire_requests_worker Daniel Vetter (21): drm/i915: wake up all pageflip waiters drm/i915: Allow userspace to hint that the relocations were known drm/i915: move dev_priv->mm out of line drm/i915: extract hangcheck/reset/error_state state into substruct drm/i915: move wedged to the other gpu error handling stuff drm/i915: fix reset handling in the throttle ioctl drm/i915: clear up wedged transitions drm/i915: create a race-free reset detection drm/i915: clarify concurrent hang detect/gpu reset consistency drm/i915: fixup sbi_read/write locking drm/i915: move modeset checks out of save/restore_modeset_reg drm/i915: extract ums suspend/resume into i915_ums.c drm/i915: dont save/restore VGA state for kms drm/i915: move DP save/restore into i915_ums.c drm/i915: vfuncs for gtt_clear_range/insert_entries drm/i915: vfuncs for ppgtt drm/i915: pte_encode is gen6+ drm/i915: extract hw ppgtt setup/cleanup code drm/i915: kill cargo-culted locking from power well code drm/i915: don't run hsw power well code on !hsw drm/i915: dynamic Haswell display power well support Egbert Eich (1): drm/i915: Remove pch_rq_mask from struct drm_i915_private. Imre Deak (3): drm/i915: merge get_gtt_alignment/get_unfenced_gtt_alignment() drm/i915: merge {i965, sandybridge}_write_fence_reg() drm/i915: use gtt_get_size() instead of open coding it Jani Nikula (3): drm/i915: ad
Re: [Intel-gfx] [PATCH 3/3] drm/i915: support resume without VT switch v2
R("skipping disabled crtc\n"); > + continue; > + } > + > + ret = intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, > + crtc->fb); > + > + if (ret == false) > + DRM_ERROR("failed to set mode on crtc %p\n", crtc); > + } > + > + return 0; > +} intel_modeset_setup_hw_state(dev, true); should do exactly this trick, see the lid notifier in intel_lvds.c. That presumes though that you don't kill the modeset state, which might require some changes in intel_modeset_disable. > + > /* > * Return which encoder is currently attached for connector. > */ > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c > index 7b30b5c..d4955b5 100644 > --- a/drivers/gpu/drm/i915/intel_fb.c > +++ b/drivers/gpu/drm/i915/intel_fb.c > @@ -148,6 +148,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev, > } > info->screen_size = size; > > + /* This driver doesn't need a VT switch to restore the mode on resume */ > + info->skip_vt_switch = true; > + > // memset(info->screen_base, 0, size); Follow-up patch to kill this // commented-line? > > drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth); > -- > 1.7.9.5 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH 1/3] PM: make VT switching to the suspend console optional v3
On Tue, Feb 05, 2013 at 11:02:17PM +0100, Rafael J. Wysocki wrote: > On Tuesday, February 05, 2013 01:55:44 PM Jesse Barnes wrote: > > On Mon, 04 Feb 2013 21:26:26 +0100 > > "Rafael J. Wysocki" wrote: > > > > > On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote: > > > > KMS drivers can potentially restore the display configuration without > > > > userspace help. Such drivers can can call a new funciton, > > > > pm_vt_switch_required(false) if they support this feature. In that > > > > case, the PM layer won't VT switch to the suspend console at suspend > > > > time and then back to the original VT on resume, but rather leave things > > > > alone for a nicer looking suspend and resume sequence. > > > > > > > > v2: make a function so we can handle multiple drivers (Alan) > > > > v3: use a list to track device requests (Rafael) > > > > > > > > Signed-off-by: Jesse Barnes > > > > > > Acked-by: Rafael J. Wysocki > > > > > > for all [1-3/3]. > > > > Any chance for an r-b on the PM one at least? Then Daniel could > > probably push this through drm-intel-next. > > Done. Thanks, I've merged the first two patches to drm-intel-next, the i915 one still needs a bit of care imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: radeon causing sleeping function called from invalid context
On Fri, Feb 08, 2013 at 10:47:42AM +0300, Sergey Senozhatsky wrote: > On (02/07/13 22:53), Andreas Bombe wrote: > > On Sun, Jan 13, 2013 at 02:38:20PM +0300, Sergey Senozhatsky wrote: > > > On (01/12/13 20:27), Dave Jones wrote: > > > > BUG: sleeping function called from invalid context at mm/slub.c:925 > > > > in_atomic(): 1, irqs_disabled(): 0, pid: 566, name: Xorg > > > > INFO: lockdep is turned off. > > > > Pid: 566, comm: Xorg Not tainted 3.8.0-rc3+ #49 > > > > Call Trace: > > > > [] __might_sleep+0x141/0x200 > > > > [] kmem_cache_alloc_trace+0x4b/0x2a0 > > > > [] ttm_bo_move_accel_cleanup+0x1d3/0x330 [ttm] > > > > [] radeon_move_blit.isra.4+0xf8/0x160 [radeon] > > > > [] radeon_bo_move+0xb0/0x1f0 [radeon] > > > > [] ttm_bo_handle_move_mem+0x27d/0x5d0 [ttm] > > > > [] ? get_parent_ip+0x11/0x50 > > > > [] ttm_bo_move_buffer+0x127/0x140 [ttm] > > > > [] ? ttm_eu_list_ref_sub+0x3d/0x60 [ttm] > > > > [] ttm_bo_validate+0xa2/0x120 [ttm] > > > > [] radeon_bo_list_validate+0x75/0x90 [radeon] > > > > [] radeon_cs_ioctl+0x582/0x950 [radeon] > > > > [] drm_ioctl+0x4d3/0x580 [drm] > > > > [] ? radeon_cs_finish_pages+0xf0/0xf0 [radeon] > > > > [] do_vfs_ioctl+0x99/0x5a0 > > > > [] ? file_has_perm+0x97/0xb0 > > > > [] ? rcu_eqs_exit+0x65/0xb0 > > > > [] sys_ioctl+0x91/0xb0 > > > > [] tracesys+0xdd/0xe2 > > > > > > > > > > I see lots of these [mostly from page fault], the following one (quick > > > and dirty) works for me. > > > > Is that patch or any other fix being picked up? It's over three weeks > > now and I'm still seeing those BUGs with the latest 3.8-rc. > > > > None that I'm aware of. Either this one https://patchwork.kernel.org/patch/2094501/ or a bit an older approach here https://patchwork.kernel.org/patch/1972071/ should fix this. /me looks at Dave Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH 1/4] drm: add helper to walk a dma scatter list a page at a time
Hi Imre! On Sat, Feb 09, 2013 at 05:27:33PM +0200, Imre Deak wrote: > Add a helper to walk through a scatter list a page at a time. Needed by > upcoming patches fixing the scatter list walking logic in the i915 driver. Nice patch, but I think this would make a rather nice addition to the common scatterlist api in scatterlist.h, maybe called sg_page_iter. There's already another helper which does cpu mappings, but it has a different use-case (gives you the page mapped, which we don't neeed and can cope with not page-aligned sg tables). With dma-buf using sg tables I expect more users of such a sg page iterator to pop up. Most possible users of this will hang around on linaro-mm-sig, so please also cc that besides the usual suspects. Cheers, Daniel > > Signed-off-by: Imre Deak > --- > include/drm/drmP.h | 44 > 1 file changed, 44 insertions(+) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index fad21c9..0c0c213 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, > void *data, > extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * > request); > extern int drm_sg_free(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +struct drm_sg_iter { > + struct scatterlist *sg; > + int sg_offset; Imo using sg_pfn_offset (i.e. sg_offset >> PAGE_SIZE) would make it clearer that this is all about iterating page-aligned sg tables. > + struct page *page; > +}; > + > +static inline int > +__drm_sg_iter_seek(struct drm_sg_iter *iter) > +{ > + while (iter->sg && iter->sg_offset >= iter->sg->length) { > + iter->sg_offset -= iter->sg->length; > + iter->sg = sg_next(iter->sg); And adding a WARN_ON(sg->legnth & ~PAGE_MASK); here would enforce that. > + } > + > + return iter->sg ? 0 : -1; > +} > + > +static inline struct page * > +drm_sg_iter_next(struct drm_sg_iter *iter) > +{ > + struct page *page; > + > + if (__drm_sg_iter_seek(iter)) > + return NULL; > + > + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); > + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; > + > + return page; > +} > + > +static inline struct page * > +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, > +unsigned long offset) > +{ > + iter->sg = sg; > + iter->sg_offset = offset; > + > + return drm_sg_iter_next(iter); > +} > + > +#define drm_for_each_sg_page(iter, sg, pgoffset) \ > + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ > + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) Again, for the initialization I'd go with page numbers, not an offset in bytes. > > /* ATI PCIGART support (ati_pcigart.h) */ > extern int drm_ati_pcigart_init(struct drm_device *dev, > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PULL] drm-intel-next
Hi Dave, So I've figured we should get drm-next for 3.10 started ;-) Highlights: - Imre's for_each_sg_pages rework (now also with the stolen mem backed case fixed with a hack) plus the drm prime sg list coalescing patch from Rahul Sharma. I have some follow-up cleanups pending, already acked by Andrew Morton. - Some prep-work for the crazy no-pch/display-less platform by Ben. - Some vlv patches, by far not all (Jesse et al). - Clean up the HDMI/SDVO #define confusion (Paulo) - gen2-4 vblank fixes from Ville. - Unclaimed register warning fixes for hsw (Paulo). More still to come ... - Complete pageflips which have been stuck in a gpu hang, should prevent stuck gl compositors (Ville). - pm patches for vt-switchless resume (Jesse). Note that the i915 enabling is not (yet) included, that took a bit longer to settle. PM patches are acked by Rafael Wysocki. - Minor fixlets all over from various people. All together it's been pretty quiet thus far. Cheers, Daniel The following changes since commit a937536b868b8369b98967929045f1df54234323: Linux 3.9-rc3 (2013-03-17 15:59:32 -0700) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-next-2013-03-23 for you to fetch changes up to e3dff585508636c8d2915cc1595e04f16ccd66ba: drm/i915: Implement WaSwitchSolVfFArbitrationPriority (2013-03-23 12:18:06 +0100) Ben Widawsky (9): drm/i915: Created a sized object error dump drm/i915: exclude CCID for platforms without it drm/i915: Capture current context on error drm/i915: Remove unused file arg from execbuf drm/i915: Remove unneeded dev argument drm/i915: Move num_pipes to intel info drm/i915: Introduce GEN7_FEATURES for device info drm/i915: Correct sandybrige overclocking drm/i915: Implement WaSwitchSolVfFArbitrationPriority Chris Wilson (1): drm/i915: Resurrect ring kicking for semaphores, selectively Damien Lespiau (1): drm/i915: Remove platforms in the preliminary_hw_support description Daniel Vetter (4): drm/i915: gen2 has no tv out support Merge tag 'v3.9-rc3' into drm-intel-next-queued style nit: Align function parameter continuation properly. drm/i915: fixup pd vs pt confusion in gen6 ppgtt code Imre Deak (5): drm: handle compact dma scatter lists in drm_clflush_sg() drm/i915: set dummy page for stolen objects drm/i915: handle walking compact dma scatter lists drm/i915: create compact dma scatter lists for gem objects drm/i915: use for_each_sg_page for setting up the gtt ptes Jani Nikula (2): drm/i915: add \n to the end of sysfs attributes drm/i915: reduce power in the ilk rc6 enable error message Jesse Barnes (18): PM: make VT switching to the suspend console optional v3 fb: add support for drivers not needing VT switch at suspend/resume time drm/i915: don't restore LVDS enable state blindly v2 drm/i915: remove disabled memset of framebuffer from intel_fb drm/i915: don't init LVDS on VLV drm/i915: VLV has force wake drm/i915/dp: don't use ILK paths on VLV drm/i915: use gen6 stolen check on VLV drm/i915/dp: add pre-PCH eDP checking to DP detect for VLV drm/i915: allow force wake at init time on VLV v2 drm/i915: don't use plane pipe select on VLV drm/i915: add media well to VLV force wake routines v2 drm/i915: use VLV DIP routines on VLV v2 drm/i915: add more VLV IDs drm/i915: fix WaDisablePSDDualDispatchEnable on VLV v2 drm/i915: set conservative clock gating values on VLV v2 drm/i915: DSPFW and BLC regs are in the display offset range drm/i915: VLV doesn't have HDMI on port C Kees Cook (2): drm/i915: use simple attribute in debugfs routines drm/i915: clarify reasoning for the access_ok call Mihnea Dobrescu-Balaur (1): gpu: don't cast kzalloc() return value Mika Kuoppala (1): drm/i915: remove obsolete obj assignment in page flip Paulo Zanoni (19): drm/i915: use HAS_DDI on intel_hdmi.c and intel_display.c drm/i915: wait_event_timeout's timeout is in jiffies drm/i915: add aux_ch_ctl_reg to struct intel_dp drm/i915: rename sdvox_reg to hdmi_reg on HDMI context drm/i915: create functions for the "unclaimed register" checks drm/i915: use FPGA_DBG for the "unclaimed register" checks drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init drm/i915: clarify confusion between SDVO and HDMI registers drm/i915: unify the definitions of the HDMI/SDVO register drm/i915: remove duplicated SDVO/HDMI bit definitions drm/i915: rename some HDMI bit definitions drm/i915: disable sound first on intel_disable_ddi drm/i915: capture the correct cursor registers on IVB
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Tue, Apr 2, 2013 at 1:00 PM, Peter Zijlstra wrote: > Also, is there anything in CS literature that comes close to this? I'd > think the DBMS people would have something similar with their > transactional systems. What do they call it? I've looked around a bit and in dbms row-locking land this seems to be called the wound-wait deadlock avoidance algorithm. It's the same approach where if you encounter an older ticket (there called transaction timestamp) you drop all locked rows and retry (or abort) the transaction. If you encounter a newer ticket when trying to grab a lock simply do a blocking wait. So ticket/reservation in Maartens patches is the analog of timestamp/transaction. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Tue, Apr 2, 2013 at 6:59 PM, Peter Zijlstra wrote: >> > Head hurts, needs more time to ponder. It would be good if someone else >> > (this would probably be you maarten) would also consider this explore >> > this 'interesting' problem space :-) > >> My head too, evil priority stuff! >> >> Hacky but pragmatical workaround for now: use a real mutex around all >> the reserve_mutex_lock* calls instead of a virtual lock. It can be >> unlocked as soon as all locks have been taken, before any actual work >> is done. >> >> It only slightly kills the point of having a reservation in the first >> place, but at least it won't break completely -rt completely for now. > > Yeah, global lock, yay :-( We've discussed this quite a bit on irc and came up with a bunch of other ideas. The global lock is completely transparent to users, since the lockdep annotations already rely on ticket_init/fini being a virtual lock. So we can always fall back to that option. For more fancy approaches we need to consider the aim first - do we just want to prevent deadlocks through PI or do we aim for bounded per-reservation_mutex wait block-to-acquire times for the thread with highest rt-prio. If it's just the former I think we can get by by piggy-packing on top of the existing PI mutex code. Only downside is that threads can lock arbitrary many reservation locks and so we're looking at boosting an entire tree of processes. Otoh common operations done while holding such a lock are swapping buffer objects in or waiting for gpu rendering. And since we can easily queue up a few ms of rendering rt guarantees are out the window ;-) If that's not good enough and the global lock not scalable enough we could try to limit the fan-out by setting a PI-boost flag in the reservation ticket (in additional to the normal PI boosting for the reservation mutex). Threads which are boosted in that fashion will get a -EAGAIN on the next mutex_reserv_lock call, ensuring that the blocking doesn't spread to further threads. But that requires that we pass around pointers to tickets instead of values, so lifetime fun (atm the ticket is on the stack) and probably tons of races in updating the ticket boost state. I'd like to avoid that until we've demonstrated a need for it ... In any way I think that all three approaches should fit into the proposed interfaces, so we should be able to do something sane here. But since I have pretty much zero clue about rt I have no idea which of the first two approaches would be preferable. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Tue, Apr 2, 2013 at 6:59 PM, Peter Zijlstra wrote: >> > Also, is there anything in CS literature that comes close to this? I'd >> > think the DBMS people would have something similar with their >> > transactional systems. What do they call it? > >> I didn't study cs, but judging from your phrasing I guess you mean you >> want me to call it transaction_mutexes instead? > > Nah, me neither, I just hate reinventing names for something that's > already got a perfectly fine name under which a bunch of people know > it. > > See the email from Daniel, apparently its known as wound-wait deadlock > avoidance -- its actually described in the "deadlock" wikipedia > article. > > So how about we call the thing something like: > > struct ww_mutex; /* wound/wait */ > > int mutex_wound_lock(struct ww_mutex *); /* returns -EDEADLK */ > int mutex_wait_lock(struct ww_mutex *); /* does not fail */ I'm not sold on this prefix, since wound-wait is just the implementation detail of how it detects/handles deadlocks. For users a really dumb strategy of just doing a mutex trylock and always returning -EAGAIN if that fails (together with a msleep(rand) in the slowpath) would have the same interface. Almost at least, we could ditch the ticket - but the ticket is used as a virtual lock for the lockdep annotation, so ditching it would also reduce lockdep usefulness (due to all those trylocks). So in case we ever switch the deadlock/backoff algo ww_ would be a bit misleading. Otoh reservation_ is just what it's used for in graphics-land, so not that much better. I don't really have a good idea for what this is besides mutexes_with_magic_deadlock_detection_and_backoff. Which is a bit long. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: manual merge of the drm-intel tree with Linus' tree
On Wed, Apr 03, 2013 at 01:43:49PM +1100, Stephen Rothwell wrote: > Hi all, > > Today's linux-next merge of the drm-intel tree got a conflict in > drivers/gpu/drm/i915/intel_panel.c between commit b1289371fcd5 ("Revert > "drm/i915: write backlight harder"") from Linus' tree and commit > 31ad8ec6a614 ("drm/i915: group backlight related stuff into a struct") > from the drm-intel tree. > > I fixed it up (see below) and can carry the fix as necessary (no action > is required). Looks good, I carry the same merge resolution locally. -Daniel > > -- > Cheers, > Stephen Rothwells...@canb.auug.org.au > > diff --cc drivers/gpu/drm/i915/intel_panel.c > index bee8cb6,0e7e873..000 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@@ -318,9 -321,16 +321,13 @@@ void intel_panel_enable_backlight(struc > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (dev_priv->backlight_level == 0) > - dev_priv->backlight_level = intel_panel_get_max_backlight(dev); > + if (dev_priv->backlight.level == 0) { > + dev_priv->backlight.level = intel_panel_get_max_backlight(dev); > + if (dev_priv->backlight.device) > + dev_priv->backlight.device->props.brightness = > + dev_priv->backlight.level; > + } > > -dev_priv->backlight.enabled = true; > -intel_panel_actually_set_backlight(dev, dev_priv->backlight.level); > - > if (INTEL_INFO(dev)->gen >= 4) { > uint32_t reg, tmp; > > @@@ -356,12 -366,12 +363,12 @@@ > } > > set_level: > -/* Check the current backlight level and try to set again if it's zero. > - * On some machines, BLC_PWM_CPU_CTL is cleared to zero automatically > - * when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 are written. > +/* Call below after setting BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1. > + * BLC_PWM_CPU_CTL may be cleared to zero automatically when these > + * registers are set. >*/ > - dev_priv->backlight_enabled = true; > - intel_panel_actually_set_backlight(dev, dev_priv->backlight_level); > -if (!intel_panel_get_backlight(dev)) > -intel_panel_actually_set_backlight(dev, > dev_priv->backlight.level); > ++dev_priv->backlight.enabled = true; > ++intel_panel_actually_set_backlight(dev, dev_priv->backlight.level); > } > > static void intel_panel_init_backlight(struct drm_device *dev) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
ct ww_mutex_key { }; > struct ww_mutex; > struct ww_mutex_ctx; > > void __ww_mutex_init(struct ww_mutex *, void *); > > #define ww_mutex_init(ww_mutex) \ > do { \ > static struct ww_mutex_key __key; \ > __ww_mutex_init(ww_mutex, &__key); \ > } while (0) > > void ww_mutex_acquire_start(struct ww_mutex_ctx *); > void ww_mutex_acquire_end (struct ww_mutex_ctx *); > > int ww_mutex_lock(struct ww_mutex_ctx *, struct ww_mutex *); > void ww_mutex_unlock(struct ww_mutex *); > > Which are to be used like: > > int bufs_lock(bufs) > { > struct ww_mutex_ctx ww_ctx; > > ww_mutex_acquire_start(&ww_ctx); > retry: > for-all-buffers(buf, bufs) { > err = ww_mutex_lock(&ww_ctx, &buf->lock); > if (err) > goto error; > } > > ww_mutex_acquire_end(&ww_ctx); > > return 0; /* success */ > > error: > for-all-buffers(buf2, bufs) { > if (buf2 == buf) > break; > ww_mutex_unlock(&buf->lock); > } > if (err == -EDEADLK) > goto again; > > return err; /* other error */ > } > > void bufs_unlock(bufs) > { > for-all-buffers(buf, bufs) > ww_mutex_unlock(&buf->lock); > } > > This API has a few problems as per Rusty's API guidelines, it is fairly > trivial to use it wrong; mostly the placement of the start and end > markers are easy to get wrong, but of course one can get all kinds of > badness from doing the retry wrong. At least having the local context > forces one to use the start/end markers. I share your concerns about the api, it's way too easy to get wrong. But with the lockdep nesting checks we should at least be covered for the ordering in the fastpath. For the error paths themselves I've just thought about ranodm -EAGAIN injection as a debug option. With that we should at least be able to cover these paths sufficiently in regression test suites. Would be a new patch for Maarten to write though ;-) > The only remaining 'problem' left is RT, however the above suggests a > very clean solution; change the lock queueing order to first sort on > priority (be it the SCHED_FIFO/RR static priority or SCHED_DEADLINE > dynamic priority) and secondly on the age. > > Note that (re)setting the age at every acquisition set is really only a > means to provide FIFO fairness between tasks, RT tasks don't strictly > require this, they have their own ordering. > > With all that this locking scheme should be deterministic; and in cases > where the older task would block (the young task has passed the end > marker) we can apply PI and push Y's priority. We've discussed this approach of using (rt-prio, age) instead of just age to determine the the "oldness" of a task for deadlock-breaking with -EAGAIN. The problem is that through PI boosting or normal rt-prio changes while tasks are trying to acquire ww_mutexes you can create acyclic loops in the blocked-on graph of ww_mutexes after the fact and so cause deadlocks. So we've convinced ourselves that this approche doesn't work. The only somewhat similar approache we couldn't poke holes into is to set a PI-boost flag on the reservation ticket (your ww_ctx) of the task a higher-prio RT thread is blocking on. Any task who has set that flag on its ww_ctx would then unconditionally fail with -EAGAIN on the next ww_mutex_lock. See my other mail from yesterday for some of the ideas we've discussed about RT-compliant ww_mutexes on irc. > Did I miss something? > > Anyway, I haven't actually looked at the details of your implementation > yet, I'll get to that next, but I think something like the above should > be what we want to aim for. > > *phew* thanks for reading this far ! :-) Well, it was a good read and I'm rather happy that we agree on the ww_ctx thing (whatever it's called in the end), even though we have slightly different reasons for it. I don't really have a useful idea to make the retry handling for users more rusty-compliant though, and I'm still unhappy with all current naming proposals ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Thu, Apr 4, 2013 at 3:31 PM, Daniel Vetter wrote: >> In this case when O blocks Y isn't actually blocked, so our >> TASK_DEADLOCK wakeup doesn't actually achieve anything. >> >> This means we also have to track (task) state so that once Y tries to >> acquire A (creating the actual deadlock) we'll not wait so our >> TASK_DEADLOCK wakeup doesn't actually achieve anything. >> >> Note that Y doesn't need to acquire A in order to return -EDEADLK, any >> acquisition from the same set (see below) would return -EDEADLK even if >> there isn't an actual deadlock. This is the cost of heuristic; we could >> walk the actual block graph but that would be prohibitively expensive >> since we'd have to do this on every acquire. > > Hm, I guess your aim with the TASK_DEADLOCK wakeup is to bound the wait > times of older task. This could be interesting for RT, but I'm unsure of > the implications. The trick with the current code is that the oldest task > will never see an -EAGAIN ever and hence is guaranteed to make forward > progress. If the task is really unlucky though it might be forced to wait > for a younger task for every ww_mutex it tries to acquire. [Aside: I'm writing this while your replies trickle in, but I think it's not yet answered already.] Ok, I've discussed this a lot with Maarten on irc and I think I see a bit clearer now what's the aim with the new sleep state. Or at least I have an illusion about it ;-) So let me try to recap my understanding to check whether we're talking roughly about the same idea. I think for starters we need to have a slightly more interesting example: 3 threads O, M, Y: O has the oldest ww_age/ticket, Y the youngest, M is in between. 2 ww_mutexes: A, B Y has already acquired ww_mutex A, M has already acquired ww_mutex B. Now O wants to acquire B and M wants to acquire A (let's ignore detailed ordering for now), resulting in O blocking on M (M holds B already, but O is older) and M blocking on Y (same for lock B). Now first question to check my understanding: Your aim with that special wakeup is to kick M so that it backs off and drops B? That way O does not need to wait for Y to complete whatever it's currently doing, unlock A and then in turn M to complete whatever it's doing so that it can unlock A&B and finally allows O to grab the lock. Presuming I'm still following we should be able to fix this with the new sleep state TASK_DEADLOCK and a flag somewhere in the thread info (let's call it PF_GTFO for simplicity). Then every time a task does a blocking wait on a ww_mutex it would set this special sleep state and also check the PF_GTFO bit. If the later is set, it bails out with -EAGAIN (so that all locks are dropped). Now if a task wants to take a lock and notices that it's held by a younger locker it can set that flag and wake the thread up (need to think about all the races a bit, but we should be able to make this work). Then it can do the normal blocking mutex slowpath and wait for the unlock. Now if O and M race a bit against each another M should either get woken (if it's already blocked on Y) and back off, or notice that the thread flag is set before it even tries to grab another mutex (and so before the block tree can extend further to Y). And the special sleep state is to make sure we don't cause any other spurious interrupts. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Thu, Apr 4, 2013 at 6:38 PM, Peter Zijlstra wrote: > On Thu, 2013-04-04 at 15:31 +0200, Daniel Vetter wrote: >> Hm, I guess your aim with the TASK_DEADLOCK wakeup is to bound the >> wait >> times of older task. > > No, imagine the following: > > struct ww_mutex A, B; > struct mutex C; > > task-O task-Y task-X > A > B > C > C > B > > At this point O finds that Y owns B and thus we want to make Y 'yield' > B to make allow B progress. Since Y is blocked, we'll send a wakeup. > However Y is blocked on a different locking primitive; one that doesn't > collaborate in the -EDEADLK scheme therefore we don't want the wakeup to > succeed. Yeah, I've thought about this some more and the special sleep state seems to be only required to ensure we don't cause spurious wakeups for other any other reasons a task blocks. But I think we can use that kick-current-holder approach to ensure that older tasks get the lock in a more timely fashion than the current code. I've tried to detail it a bit with another 3 task example - that seems to be the point where the fun starts ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Thu, Apr 4, 2013 at 6:49 PM, Peter Zijlstra wrote: > On Thu, 2013-04-04 at 15:31 +0200, Daniel Vetter wrote: >> We've discussed this approach of using (rt-prio, age) instead of just >> age >> to determine the the "oldness" of a task for deadlock-breaking with >> -EAGAIN. The problem is that through PI boosting or normal rt-prio >> changes >> while tasks are trying to acquire ww_mutexes you can create acyclic >> loops >> in the blocked-on graph of ww_mutexes after the fact and so cause >> deadlocks. So we've convinced ourselves that this approche doesn't >> work. > > Could you pretty please draw me a picture, I'm having trouble seeing > what you mean. > > AFAICT as long as you boost Y while its the lock owner things should > work out, no? So this one here took a bit of pondering. But I think you'll like the conclusion. Now the deadlock detection works because it impose a dag onto all lockers which clearly denotes who'll wait and who will get wounded. The problem with using (rt_prio, ww_age/ticket) instead of just the ticket si that it's ambigous in the face of PI boosting. E.g. - rt_prio of A > rt_prio of B, but - task A is younger than task B Before boosting task A wins, but after boosting (when only the age matters) task B wins, since it's now older. So now when B comes around and tries to grab a lock A currently holds, it'll also block. Now the cool thing with TASK_KILLABLE (hey, I start to appreciate it, bear with me!) is that this is no problem - it limits the length of any chain of blocked tasks to just one link of blocked tasks: - If the length is currently one it cannot be extended - the blocked task will set the PF_GTFO flag on the current holder, and since that would be checked before blocking on another ww_mutex the chain can't grow past one blocked task. - If the current holder itself is blocked on another ww_mutex it'll be in TASK_DEADLOCK state and get woken up. In both case the current holder of the lock will either continue with what it's been doing (PI-boosted ofc) until it unlocks everything or hits the PF_GTFO check and backs off from all currently held locks. RT mission accomplished I think since the wait time for the highest-prio task for grabbing a lock is bounded by the lock hold time for the completely uncontended case. And within a given rt-prio the normal wait/wound algorithm will resolve conflicts (and ensure forward progress). So I'm now rather convinced that with the TASK_DEADLOCK implementation we can make (rt_prio, age) lock ordering work. But it's not an artifact of consistent PI boosting (I've raced down that alley first trying to prove it to no avail), but the fact that lock holder kicking through PF_GTFO naturally limits blocked task chains on ww_mutexes to just one link. That in turn makes any post-PI-boosted considerations moot and so can't result in havoc due to a PI-boost having flipped an edge in the lock_er_ ordering DAG (we sort tasks with wait/wound, not locks, hence it's not a lock ordering DAG). Please poke holes into this argument, I've probably missed a sublety somewhere ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/cirrus: Add bpp option
On Tue, Jan 29, 2013 at 09:29:17AM +0100, Takashi Iwai wrote: > Add a new option, bpp, to specify the default bpp value. > > Signed-off-by: Takashi Iwai > --- > > This patch is applied on the top of previous two patches. > I couldn't find an easy way to specify the default bpp, so I cooked > the driver quickly. If there is any other convenient way to achieve > this, let me know... Well, you can specify the desired bpp with a full mode on the kernel cmdline - the '-bpp' extension. Reading through the parser I think it should work even with just the '-bpp' and not a full mode, but I haven't tested. Look for cmdline_mode->bpp_specified in drm_fb_helper.c and the relevant parsing code in drm_mode_parse_command_line_for_connector in drm_modes.c If that doesn't work for you, I think it's better to extend/fix it than add driver module options. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/cirrus: Add bpp option
On Tue, Jan 29, 2013 at 10:57 AM, Takashi Iwai wrote: > At Tue, 29 Jan 2013 10:53:50 +0100, > Daniel Vetter wrote: >> >> On Tue, Jan 29, 2013 at 09:29:17AM +0100, Takashi Iwai wrote: >> > Add a new option, bpp, to specify the default bpp value. >> > >> > Signed-off-by: Takashi Iwai >> > --- >> > >> > This patch is applied on the top of previous two patches. >> > I couldn't find an easy way to specify the default bpp, so I cooked >> > the driver quickly. If there is any other convenient way to achieve >> > this, let me know... >> >> Well, you can specify the desired bpp with a full mode on the kernel >> cmdline - the '-bpp' extension. Reading through the parser I think it >> should work even with just the '-bpp' and not a full mode, but I haven't >> tested. Look for cmdline_mode->bpp_specified in drm_fb_helper.c and the >> relevant parsing code in drm_mode_parse_command_line_for_connector in >> drm_modes.c >> >> If that doesn't work for you, I think it's better to extend/fix it than >> add driver module options. > > Well, the fb can be set by that option, but the default modeset > doesn't seem to honor it. So if you start X modeset driver, it still > takes what the driver sets as default. That was the problem I hit. Oh, I've missed the preferred_bpp hint for the generic modesetting driver. Can we fill that in by using the bpp parsed in the fb helper somehow? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Mon, Jan 28, 2013 at 06:06:38PM +0800, Zhang Rui wrote: > On Mon, 2013-01-28 at 09:31 +0100, Daniel Vetter wrote: > > On Mon, Jan 28, 2013 at 3:36 AM, Zhang Rui wrote: > > >> Given that this essentially requires users to manually set this module > > >> option to make stuff work I don't like this. > > >> > > > sorry, I do not understand. > > > this patch just disables modeset_on_lid during suspend. > > > > Pardon me, the misunderstanding is on my side - I've mixed up > > dev_priv->modeset_on_lid with the corresponding module option. > > > > Is my understanding correct that only with the new SCI pm state this > > can happen, since that still allows acpi events to be processed (and > > so our lid_notifier being called? > > > yep. > > > >> I see a few possible options: > > >> - plug the races between all the different parts - I've never really > > >> understood why acpi sends us events before the resume code has > > >> completed ... If that's indeed intentional, we could delay the > > >> handling a bit until at least the i915 resume code completed. > > > > > > IMO, the real question is that, during a suspend/resume cycle, if we > > > need to take care of the lid open event or not. > > > To me, the answer is no, because when resuming from STR, i915 is not > > > aware of such an event, and the i915 resume code works well, right? > > > so I do not think it is a problem to ignore the lid event for another > > > lightweight suspend state, which is introduced in Patch 1/3 in this > > > series. > > > > > >> - Judging from the diff context you're not on the latest 3.8-rc > > >> codebase - we've applied a few fixes to this lid hack recently. Please > > >> retest on a kernel with > > >> > > > the patch is based on 3.8.0-rc3, which already includes the commit > > > below. > > > And yes, the problem still exists. > > > > Ok, I think I see the issue now. But I suspect that we need some > > additional locking to make this solid, since currently > > dev_priv->modeset_on_lid updates can race with our lid notifier > > handler. Let me think about this a bit more. > > hmm, > with this patch, intel_lid_notify() will return immediately if > modeset_on_lid is set to -1. So the only problem in my mind is that we > got a lid open event during i915_suspend(). > > Say, > lid is opened -> i915_lid_notify() is invoked (modeset_on_lid is 1 at > this time) -> i915_suspend() set modeset_on_lid to -1, just before > intel_modeset_setup_hw_state() being invoked in i915_lid_notify() -> > intel_modeset_setup_hw_state() breaks the system. > > but my first question would be is this (lid open on suspend) possible in > real world? > If the answer is yes, then my second question is that, this problem > exists for STR as well because SCI is still valid at this time when > suspending to memory, have we seen such issues for STR before? > > Anyway, if the current code does not break STR, this patch should be > enough for the new suspend state. > what do you think? I think I have two wishlist changes for your patch ;-) - Convert dev_priv->modeset_on_lid to an enum, so that we have descriptive names instead of magic numbers. - I think we can close all races by adding a new lid_notifier mutex (I prefer a new lock instead of overloading an existing one with). If we hold that in the suspend/resume paths just for changing modeset_on_lid and also for the entire lid notifier callback (i.e. grab the lock before first looking at modest_on_lid, only drop it once the modeset fixup has been completed at the end of the function) we should be race-free in all cases. Also, I think we should move the modeset_on_lid updates in the suspend/resume paths to the very beginning/end. Can you pls update your patch with these changes? Or do you see an additional race we need to plug somewhere? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7] mutex: add support for reservation style locks
On Wed, Jan 30, 2013 at 2:07 AM, Rob Clark wrote: > == > Basic problem statement: > - --- - > GPU's do operations that commonly involve many buffers. Those buffers > can be shared across contexts/processes, exist in different memory > domains (for example VRAM vs system memory), and so on. And with > PRIME / dmabuf, they can even be shared across devices. So there are > a handful of situations where the driver needs to wait for buffers to > become ready. If you think about this in terms of waiting on a buffer > mutex for it to become available, this presents a problem because > there is no way to guarantee that buffers appear in a execbuf/batch in > the same order in all contexts. That is directly under control of > userspace, and a result of the sequence of GL calls that an > application makes. Which results in the potential for deadlock. The > problem gets more complex when you consider that the kernel may need > to migrate the buffer(s) into VRAM before the GPU operates on the > buffer(s), which main in turn require evicting some other buffers (and > you don't want to evict other buffers which are already queued up to > the GPU), but for a simplified understanding of the problem you can > ignore this. > > The algorithm that TTM came up with for dealing with this problem is > quite simple. For each group of buffers (execbuf) that need to be > locked, the caller would be assigned a unique reservation_id, from a > global counter. In case of deadlock in the process of locking all the > buffers associated with a execbuf, the one with the lowest > reservation_id wins, and the one with the higher reservation_id > unlocks all of the buffers that it has already locked, and then tries > again. > > Originally TTM implemented this algorithm on top of an event-queue and > atomic-ops, but Maarten Lankhorst realized that by merging this with > the mutex code we could take advantage of the existing mutex fast-path > code and result in a simpler solution, and so ticket_mutex was born. > (Well, there where also some additional complexities with the original > implementation when you start adding in cross-device buffer sharing > for PRIME.. Maarten could probably better explain.) I think the motivational writeup above is really nice, but the example code below is a bit wrong > How it is used: > --- -- -- - > > A very simplified version: > > int submit_execbuf(execbuf) > { > /* acquiring locks, before queuing up to GPU: */ > seqno = assign_global_seqno(); > retry: > for (buf in execbuf->buffers) { > ret = mutex_reserve_lock(&buf->lock, seqno); > switch (ret) { > case 0: > /* we got the lock */ > break; > case -EAGAIN: > /* someone with a lower seqno, so unreserve and try again: */ > for (buf2 in reverse order starting before buf in > execbuf->buffers) > mutex_unreserve_unlock(&buf2->lock); > goto retry; > default: > goto err; > } > } > > /* now everything is good to go, submit job to GPU: */ > ... > } > > int finish_execbuf(execbuf) > { > /* when GPU is finished: */ > for (buf in execbuf->buffers) > mutex_unreserve_unlock(&buf->lock); > } > == Since gpu command submission is all asnyc (hopefully at least) we don't unlock once it completes, but right away after the commands are submitted. Otherwise you wouldn't be able to submit new execbufs using the same buffer objects (and besides, holding locks while going back out to userspace is evil). The trick is to add a fence object for async operation (essentially a waitqueue on steriods to support gpu->gpu direct signalling). And updating fences for a given execbuf needs to happen atomically for all buffers, for otherwise userspace could trick the kernel into creating a circular fence chain. This wouldn't deadlock the kernel, since everything is async, but it'll nicely deadlock the gpus involved. Hence why we need ticketing locks to get dma_buf fences off the ground. Maybe wait for Maarten's feedback, then update your motivational blurb a bit? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] fbcon locking fixes.
On Mon, Jan 28, 2013 at 1:45 PM, Maarten Lankhorst wrote: >> There was a path going into set_con2fb_path if an fb driver was >> already registered, I just pushed the locking out further on anyone >> going in there. >> >> it boots on my EFI macbook here. >> > I cherry picked those patches to my tree, and the full series no longer > triggers a lockdep warning. > It also no longer locks up during modprobing or vga-switcheroo either. > > Tested-by: Maarten Lankhorst QA reported spurious failures in some kms_flip tests and lockdep splats, somehow both are fixed with the locking. I'm a bit confused how these issues could cause failures in the flip tests, but alas: Tested-by: Lu Hua (for all three patches). -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: circular locking dependency detected
On Wed, Jan 30, 2013 at 10:52 PM, Russell King wrote: > Also adding Greg and Daniel to this as Daniel introduced the lockdep > checking. > > This looks extremely horrid to be to solve - the paths are rather deep > where the dependency occurs. The two paths between the locks are: > > console_lock+0x5c/0x70 > register_con_driver+0x44/0x150 > take_over_console+0x24/0x3b4 > fbcon_takeover+0x70/0xd4 > fbcon_event_notify+0x7c8/0x818 > notifier_call_chain+0x4c/0x8c > __blocking_notifier_call_chain+0x50/0x68 > blocking_notifier_call_chain+0x20/0x28 > > and > > __blocking_notifier_call_chain+0x34/0x68 > blocking_notifier_call_chain+0x20/0x28 > fb_notifier_call_chain+0x20/0x28 > fb_blank+0x40/0xac > fbcon_blank+0x1f4/0x29c > do_blank_screen+0x1b8/0x270 > console_callback+0x74/0x138 You want Dave Airlie's pile of locking reworks, which fixes all currently known offenders around console_lock and fb_notifier. Patches won't go into 3.9 since it took a few rounds until they did not cause regression by making these deadlocks easier to hit. http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes Long term solution would be to abolish the fb_notifier, at least for the purpose of linking fbdevs up with the fbcon and just replace those with direct function calls. But that requires that we no longer allow fbdev drivers and the fbcon to be loaded in any arbitrary order. Or just force fbcon to be built-in if enabled, imo the sane choice (no one's bothering with config_vt=m either, after all). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: circular locking dependency detected
On Wed, Jan 30, 2013 at 11:19 PM, Russell King wrote: > So... what you seem to be telling me is that 3.9 is going to be a > release which issues lockdep complaints when the console blanks, and > you think that's acceptable? > > Adding Linus and Andrew so they're aware of this issue... Linus was the guy who shot down the patches for 3.9 since one of the earlier iterations caused instant deadlocks - I've been pushing them to merge them ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: circular locking dependency detected
On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman wrote: > On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote: >> On Thu, Jan 31, 2013 at 11:13 AM, Russell King wrote: >> > >> > Which may or may not be a good thing depending how you look at it; it >> > means that once your kernel blanks, you get a lockdep dump. At that >> > point you lose lockdep checking for everything else because lockdep >> > disables itself after the first dump. >> >> Fair enough, we may want to revert the lockdep checking for >> console_lock, and make re-enabling it part of the patch-series that >> fixes the locking. >> >> Daniel/Dave? Does that sound reasonable? Yeah, sounds good. > Reverting the patch is fine with me. Just let me know so I can queue it > up again for 3.9. Can you please also pick up the (currently) three locking fixups around fbcon? Just so that we don't repeat the same fun where people complain about lockdep splats, but the fixes are stuck somewhere. And I guess Dave would be happy to not end up as fbcon maintainer ;-) He has a git branch with them at http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes though I have a small bikeshed on his last patch pending. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] drm_crtc: check if fb_create return NULL
On Thu, Jan 31, 2013 at 1:55 AM, Su, Xuemin wrote: > -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Thursday, January 24, 2013 5:05 PM > To: Su, Xuemin > Cc: airl...@linux.ie; dri-de...@lists.freedesktop.org; > linux-kernel@vger.kernel.org; yanmin_zh...@linux.intel.com; He, Bo > Subject: Re: [PATCH V3] drm_crtc: check if fb_create return NULL > >>Ah, sorry, never mind, I missed Daniel's comment. The benefit of the >>BUG_ON() is making it clear what's expected of the drivers. > >>Reviewed-by: Jani Nikula > > Do you think this patch is still needed? > Currently I fix a buggy function radeon_user_framebuffer_create() which > returns NULL and patch is added to 3.7-stable tree. > Do you think it's also needed to do something in drm_mode_addfb()? Imo it's still useful, just to document the assumptions of the interface. Dave? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH drm-intel] drm/i915: gen6_gmch_remove can be static
On Thu, Jan 31, 2013 at 11:32:50AM +0800, Changlong Xie wrote: > Signed_off_by: Changlong Xie > Reported-by: Fengguang Wu Queued for -next, thanks for the patch. Note that checkpatch complained about the _ instead of - in your sob line, fixed up while applying. -Daniel > --- > i915_gem_gtt.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 87249e8..748015b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -773,7 +773,7 @@ static int gen6_gmch_probe(struct drm_device *dev, > return ret; > } > > -void gen6_gmch_remove(struct drm_device *dev) > +static void gen6_gmch_remove(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > iounmap(dev_priv->gtt.gsm); -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: circular locking dependency detected
On Thu, Jan 31, 2013 at 10:21 AM, Greg Kroah-Hartman wrote: >> Can you please also pick up the (currently) three locking fixups >> around fbcon? Just so that we don't repeat the same fun where people >> complain about lockdep splats, but the fixes are stuck somewhere. And >> I guess Dave would be happy to not end up as fbcon maintainer ;-) He >> has a git branch with them at >> http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes >> though I have a small bikeshed on his last patch pending. > > Care to just send me the patches through email when you all get done > bikesheding? And for some reason I thought Andrew was going to handle > these fbcon patches, but if not, I'll be glad to take them. Yeah, I'll annoy people again with patches until they're merged ;-) Iirc Andrew picked them due to lack of an fbcon maintainer, and everyone else likes to pass the bucket. Having looked through the code a bit, imo understandable ... btw, I've started to look into how we could fix the locking madness around fbcon for good instead of with duct-tape [1]. I'll try to discuss this with a few fbdev guys at fosdem (some at least should be there). Certainly a long-term thing, but comments from whomever gets volunteered to shepherd fbcon would be great. Cheers, Daniel 1: http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg33535.html -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-mm-sig] [PATCH 4/7] fence: dma-buf cross-device synchronization (v11)
On Thu, Jan 31, 2013 at 06:32:15PM +0900, Inki Dae wrote: > Hi, > > below is my opinion. > > > +struct fence; > > +struct fence_ops; > > +struct fence_cb; > > + > > +/** > > + * struct fence - software synchronization primitive > > + * @refcount: refcount for this fence > > + * @ops: fence_ops associated with this fence > > + * @cb_list: list of all callbacks to call > > + * @lock: spin_lock_irqsave used for locking > > + * @priv: fence specific private data > > + * @flags: A mask of FENCE_FLAG_* defined below > > + * > > + * the flags member must be manipulated and read using the appropriate > > + * atomic ops (bit_*), so taking the spinlock will not be needed most > > + * of the time. > > + * > > + * FENCE_FLAG_SIGNALED_BIT - fence is already signaled > > + * FENCE_FLAG_ENABLE_SIGNAL_BIT - enable_signaling might have been called* > > + * FENCE_FLAG_USER_BITS - start of the unused bits, can be used by the > > + * implementer of the fence for its own purposes. Can be used in different > > + * ways by different fence implementers, so do not rely on this. > > + * > > + * *) Since atomic bitops are used, this is not guaranteed to be the case. > > + * Particularly, if the bit was set, but fence_signal was called right > > + * before this bit was set, it would have been able to set the > > + * FENCE_FLAG_SIGNALED_BIT, before enable_signaling was called. > > + * Adding a check for FENCE_FLAG_SIGNALED_BIT after setting > > + * FENCE_FLAG_ENABLE_SIGNAL_BIT closes this race, and makes sure that > > + * after fence_signal was called, any enable_signaling call will have > > either > > + * been completed, or never called at all. > > + */ > > +struct fence { > > + struct kref refcount; > > + const struct fence_ops *ops; > > + struct list_head cb_list; > > + spinlock_t *lock; > > + unsigned context, seqno; > > + unsigned long flags; > > +}; > > + > > +enum fence_flag_bits { > > + FENCE_FLAG_SIGNALED_BIT, > > + FENCE_FLAG_ENABLE_SIGNAL_BIT, > > + FENCE_FLAG_USER_BITS, /* must always be last member */ > > +}; > > + > > It seems like that this fence framework need to add read/write flags. > In case of two read operations, one might wait for another one. But > the another is just read operation so we doesn't need to wait for it. > Shouldn't fence-wait-request be ignored? In this case, I think it's > enough to consider just only write operation. > > For this, you could add the following, > > enum fence_flag_bits { > ... > FENCE_FLAG_ACCESS_READ_BIT, > FENCE_FLAG_ACCESS_WRITE_BIT, > ... > }; > > And the producer could call fence_init() like below, > __fence_init(..., FENCE_FLAG_ACCESS_WRITE_BIT,...); > > With this, fence->flags has FENCE_FLAG_ACCESS_WRITE_BIT as write > operation and then other sides(read or write operation) would wait for > the write operation completion. > And also consumer calls that function with FENCE_FLAG_ACCESS_READ_BIT > so that other consumers could ignore the fence-wait to any read > operations. Fences here match more to the sync-points concept from the android stuff. The idea is that they only signal when a hw operation completes. Synchronization integration happens at the dma_buf level, where you can specify whether the new operation you're doing is exclusive (which means that you need to wait for all previous operations to complete), i.e. a write. Or whether the operation is non-excluses (i.e. just reading) in which case you only need to wait for any still outstanding exclusive fences attached to the dma_buf. But you _can_ attach more than one non-exclusive fence to a dma_buf at the same time, and so e.g. read a buffer objects from different engines concurrently. There's been some talk whether we also need a non-exclusive write attachment (i.e. allow multiple concurrent writers), but I don't yet fully understand the use-case. In short the proposed patches can do what you want to do, it's just that read/write access isn't part of the fences, but how you attach fences to dma_bufs. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RESEND PATCH 4/4] GPU/i915: Fix acpi_bus_get_device() check in drivers/gpu/drm/i915/intel_opregion.c
On Thu, Jan 31, 2013 at 12:27:26PM +0900, Yasuaki Ishimatsu wrote: > I forgot to change subject. So I resend a patch. > > --- > acpi_bus_get_device() returns int not acpi_status. > > The patch change not to apply ACPI_FAILURE() to the return value of > acpi_bus_get_device(). > > Signed-off-by: Yasuaki Ishimatsu I've tried to apply this to drm-intel-next-queued, but git am didn't really like your patch - it failed to apply. Can you please rebase to the drm-intel-next-queued branch from git://people.freedesktop.org/~danvet/drm-intel and please resubmit the patch, preferrably formatted with git format-patch? Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_opregion.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-pm/drivers/gpu/drm/i915/intel_opregion.c > === > --- linux-pm.orig/drivers/gpu/drm/i915/intel_opregion.c 2013-01-31 > 11:39:37.075849905 +0900 > +++ linux-pm/drivers/gpu/drm/i915/intel_opregion.c2013-01-31 > 11:52:18.796850274 +0900 > @@ -347,7 +347,7 @@ static void intel_didl_outputs(struct dr > int i = 0; > handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev); > - if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) > + if (!handle || acpi_bus_get_device(handle, &acpi_dev)) > return; > if (acpi_is_video_device(acpi_dev)) > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-mm-sig] [PATCH 4/7] fence: dma-buf cross-device synchronization (v11)
On Thu, Jan 31, 2013 at 3:38 PM, Inki Dae wrote: > I think I understand as your comment but I don't think that I > understand fully the dma-fence mechanism. So I wish you to give me > some advices for it. In our case, I'm applying the dma-fence to > mali(3d gpu) driver as producer and exynos drm(display controller) > driver as consumer. > > And the sequence is as the following: > In case of producer, > 1. call fence_wait to wait for the dma access completion of others. > 2. And then the producer creates a fence and a new reservation entry. > 3. And then it sets the given dmabuf's resv(reservation_object) to the > new reservation entry. > 4. And then it adds the reservation entry to entries list. > 5. And then it sets the fence to all dmabufs of the entries list. > Actually, this work is to set the fence to the reservaion_object of > each dmabuf. > 6. And then the producer's dma start. > 7. Finally, when the dma start is completed, we get the entries list > from a 3d job command(in case of mali core, pp job) and call > fence_signal() with each fence of each reservation entry. > > From here, is there my missing point? Yeah, more or less. Although you need to wrap everything into ticket reservation locking so that you can atomically update fences if you have support for some form of device2device singalling (i.e. without blocking on the cpu until all the old users completed). At least that's the main point of Maarten's patches (and this does work with prime between a few drivers by now), but ofc you can use cpu blocking as a fallback. > And I thought the fence from reservation entry at step 7 means that > the producer wouldn't access the dmabuf attaching this fence anymore > so this step wakes up all processes blocked. So I understood that the > fence means a owner accessing the given dmabuf and we could aware of > whether the owner commited its own fence to the given dmabuf to read > or write through the fence's flags. The fence doesn't give ownership of the dma_buf object, but only indicates when the dma access will have completed. The relationship between dma_buf/reservation and the attached fences specify whether other hw engines can access the dma_buf, too (if the fence is non-exclusive). > If you give me some advices, I'd be happy. Rob and Maarten are working on some howtos and documentation with example code, I guess it'd be best to wait a bit until we have that. Or just review the existing stuff Rob just posted and reply with questions there. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Revert "console: implement lockdep support for console_lock"
On Fri, Feb 1, 2013 at 9:21 AM, Sedat Dilek wrote: > people having the fbcon-locking-fixes [1] in their local GIT tree can > revert this change? Yeah, if you have all the fixes reverting this is fine and appreciated to increase testing. Dave will probably push the revert himself to drm-next soon. -Daniel > > commit ff0d05bf73620eb7dc8aee7423e992ef87870bdf > Refs: v3.8-rc5-194-gff0d05b > Author: Dave Airlie > AuthorDate: Thu Jan 31 14:27:03 2013 +1100 > Commit: Dave Airlie > CommitDate: Thu Jan 31 15:46:56 2013 +1100 > > Revert "console: implement lockdep support for console_lock" > > This reverts commit daee779718a319ff9f83e1ba3339334ac650bb22. > > I'll requeue this after the console locking fixes, so lockdep > is useful again for people until fbcon is fixed. > > Signed-off-by: Dave Airlie > > Thanks! > > Regards, > - Sedat > > [1] http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [UPDATE][PATCH 4/4] GPU/i915: Fix acpi_bus_get_device() check in drivers/gpu/drm/i915/intel_opregion.c
On Fri, Feb 01, 2013 at 10:14:20AM +0900, Yasuaki Ishimatsu wrote: > 2013/02/01 9:53, Yasuaki Ishimatsu wrote: > >2013/02/01 9:50, Yasuaki Ishimatsu wrote: > >>2013/01/31 19:03, Daniel Vetter wrote: > >>>On Thu, Jan 31, 2013 at 12:27:26PM +0900, Yasuaki Ishimatsu wrote: > >>>>I forgot to change subject. So I resend a patch. > >>>> > >>>>--- > >>>>acpi_bus_get_device() returns int not acpi_status. > >>>> > >>>>The patch change not to apply ACPI_FAILURE() to the return value of > >>>>acpi_bus_get_device(). > >>>> > >>>>Signed-off-by: Yasuaki Ishimatsu > >>> > >>>I've tried to apply this to drm-intel-next-queued, but git am didn't > >>>really like your patch - it failed to apply. Can you please rebase to the > >>>drm-intel-next-queued branch from > >>> > >>>git://people.freedesktop.org/~danvet/drm-intel > >>> > >>>and please resubmit the patch, preferrably formatted with git > >>>format-patch? > >> > >>My mailer added a space in the patch. I updated the patch. > >>How abot it? > >> > >>--- > >>acpi_bus_get_device() returns int not acpi_status. > >> > >>The patch change not to apply ACPI_FAILURE() to the return value of > >>acpi_bus_get_device(). > >> > >>Signed-off-by: Yasuaki Ishimatsu > >>--- > >> drivers/gpu/drm/i915/intel_opregion.c |2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_opregion.c > >>b/drivers/gpu/drm/i915/intel_opregion.c > >>index 7741c22..4d33874 100644 > >>--- a/drivers/gpu/drm/i915/intel_opregion.c > >>+++ b/drivers/gpu/drm/i915/intel_opregion.c > >>@@ -347,7 +347,7 @@ static void intel_didl_outputs(struct drm_device *dev) > >> int i = 0; > >> > >> handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev); > >>-if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) > >>+if (!handle || acpi_bus_get_device(handle, &acpi_dev)) > >> return; > >> > >> if (acpi_is_video_device(acpi_dev)) > >> > > > >Oops. My mailer added a space again... > >I'll check my mailer again. > > I do not understand why space was added to the patch. > So I attached the patch. Please check it. Yeah, worked this time. Queued for -next, thanks for the patch. -Daniel > > Thanks, > Yasuaki Ishimatsu > > > > >Thanks, > >Yasuaki Ishimatsu > > > >>-- > >>To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > >>the body of a message to majord...@vger.kernel.org > >>More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > >-- > >To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html > > acpi_bus_get_device() returns int not acpi_status. > > The patch change not to apply ACPI_FAILURE() to the return value of > acpi_bus_get_device(). > > Signed-off-by: Yasuaki Ishimatsu > --- > drivers/gpu/drm/i915/intel_opregion.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_opregion.c > b/drivers/gpu/drm/i915/intel_opregion.c > index 7741c22..4d33874 100644 > --- a/drivers/gpu/drm/i915/intel_opregion.c > +++ b/drivers/gpu/drm/i915/intel_opregion.c > @@ -347,7 +347,7 @@ static void intel_didl_outputs(struct drm_device *dev) > int i = 0; > > handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev); > - if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) > + if (!handle || acpi_bus_get_device(handle, &acpi_dev)) > return; > > if (acpi_is_video_device(acpi_dev)) > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] drm/i915: fixup per-crtc locking in intel_release_load_detect_pipe
On Wed, Jan 23, 2013 at 05:25:09PM +0100, Daniel Vetter wrote: > One of the early return cases missed the mutex unlocking. Hilarity > ensued. > > This regression has been introduced in > > commit 7b24056be6db7ce907baffdd4cf142ab774ea60c > Author: Daniel Vetter > Date: Wed Dec 12 00:35:33 2012 +0100 > > drm: don't hold crtc mutexes for connector ->detect callbacks > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59750 > Signed-off-by: Daniel Vetter Tested-by: Cancan Feng Dave, please pick this one up for your drm-next tree since the issue only happens there due to the modeset locking rework. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: circular locking warning during suspend
console_lock); > [ 73.320927]lock((fb_notifier_list).rwsem); > [ 73.320931]lock(console_lock); > [ 73.320934] lock((fb_notifier_list).rwsem); > [ 73.320935] > [ 73.320935] *** DEADLOCK *** > [ 73.320935] > [ 73.320937] 4 locks held by kworker/u:9/2574: > [ 73.320947] #0: (events_unbound){.+.+.+}, at: [] > process_one_work+0x197/0x750 > [ 73.320955] #1: ((&entry->work)){+.+.+.}, at: [] > process_one_work+0x197/0x750 > [ 73.320964] #2: (&__lockdep_no_validate__){..}, at: > [] __device_suspend+0xb8/0x200 > [ 73.320974] #3: (console_lock){+.+.+.}, at: [] > i915_drm_freeze+0x90/0xe0 > [ 73.320975] > [ 73.320975] stack backtrace: > [ 73.320979] Pid: 2574, comm: kworker/u:9 Tainted: GW > 3.8.0-rc4-wl-67484-g04fa847 #106 > [ 73.320981] Call Trace: > [ 73.320990] [] print_circular_bug+0x1fc/0x20e > [ 73.320996] [] __lock_acquire+0x1aee/0x1be0 > [ 73.321016] [] lock_acquire+0x96/0x1e0 > [ 73.321029] [] down_read+0x4e/0x98 > [ 73.321041] [] __blocking_notifier_call_chain+0x5a/0xd0 > [ 73.321047] [] blocking_notifier_call_chain+0x16/0x20 > [ 73.321053] [] fb_notifier_call_chain+0x1b/0x20 > [ 73.321058] [] fb_set_suspend+0x4e/0x60 > [ 73.321063] [] intel_fbdev_set_suspend+0x25/0x30 > [ 73.321069] [] i915_drm_freeze+0x9d/0xe0 > [ 73.321075] [] i915_pm_suspend+0x4a/0xa0 > [ 73.321080] [] pci_pm_suspend+0x74/0x140 > [ 73.321091] [] dpm_run_callback.isra.3+0x3c/0x80 > [ 73.321096] [] __device_suspend+0xe5/0x200 > [ 73.321102] [] async_suspend+0x1f/0xa0 > [ 73.321107] [] async_run_entry_fn+0x92/0x1c0 > [ 73.321113] [] process_one_work+0x208/0x750 > [ 73.321142] [] worker_thread+0x160/0x450 > [ 73.321153] [] kthread+0xf2/0x100 > [ 73.321173] [] ret_from_fork+0x7c/0xb0 > > johannes > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fbcon: fix locking harder
On Fri, Jan 25, 2013 at 2:43 AM, Dave Airlie wrote: > Okay so Alan's patch handled the case where there was no registered fbcon, > however the other path entered in set_con2fb_map pit. > > In there we called fbcon_takeover, but we also took the console lock in a > couple > of places. So push the console lock out to the callers of set_con2fb_map, > > this means fbmem and switcheroo needed to take the lock around the fb notifier > entry points that lead to this. > > This should fix the efifb regression seen by Maarten. > > Signed-off-by: Dave Airlie > --- > drivers/gpu/vga/vga_switcheroo.c | 3 +++ > drivers/video/console/fbcon.c| 11 --- > drivers/video/fbmem.c| 2 ++ > 3 files changed, 13 insertions(+), 3 deletions(-) > [cut] > ret = vgasr_priv.handler->switchto(new_client->id); > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c > index 2aef9ca..2e2d959 100644 > --- a/drivers/video/console/fbcon.c > +++ b/drivers/video/console/fbcon.c > @@ -842,6 +842,8 @@ static void con2fb_init_display(struct vc_data *vc, > struct fb_info *info, > * > * Maps a virtual console @unit to a frame buffer device > * @newidx. > + * > + * This should be called with the console lock held. > */ > static int set_con2fb_map(int unit, int newidx, int user) > { What about throwing a WARN_CONSOLE_UNLOCKED(); in here to make sure this new rule is obeyed? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Sun, Jan 27, 2013 at 4:21 PM, Zhang Rui wrote: > i915 driver needs to do modeset when > 1. system resumes from sleep > 2. lid is opened > > In PM_SUSPEND_MEM state, all the GPEs are cleared when system resumes, > thus it is the i915_resume code does the modeset rather than > intel_lid_notify(). > > In PM_SUSPEND_FREEZE state, system is still resposive to the lid events. > 1. When we close the lid in Freeze state, intel_lid_notify() sets > modeset_on_lid. > 2. When we reopen the lid, intel_lid_notify() will do a modeset, >before the system is resumed. > > here is the error log, > > [92146.548074] WARNING: at drivers/gpu/drm/i915/intel_display.c:1028 > intel_wait_for_pipe_off+0x184/0x190 [i915]() > [92146.548076] Hardware name: VGN-Z540N > [92146.548078] pipe_off wait timed out > [92146.548167] Modules linked in: hid_generic usbhid hid > snd_hda_codec_realtek snd_hda_intel snd_hda_codec parport_pc snd_hwdep ppdev > snd_pcm_oss i915 snd_mixer_oss snd_pcm arc4 iwldvm snd_seq_dummy mac80211 > snd_seq_oss snd_seq_midi fbcon tileblit font bitblit softcursor > drm_kms_helper snd_rawmidi snd_seq_midi_event coretemp drm snd_seq kvm btusb > bluetooth snd_timer iwlwifi pcmcia tpm_infineon i2c_algo_bit joydev > snd_seq_device intel_agp cfg80211 snd intel_gtt yenta_socket pcmcia_rsrc > sony_laptop agpgart microcode psmouse tpm_tis serio_raw mxm_wmi soundcore > snd_page_alloc tpm acpi_cpufreq lpc_ich pcmcia_core tpm_bios mperf processor > lp parport firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci thermal > e1000e > [92146.548173] Pid: 4304, comm: kworker/0:0 Tainted: GW > 3.8.0-rc3-s0i3-v3-test+ #9 > [92146.548175] Call Trace: > [92146.548189] [] warn_slowpath_common+0x72/0xa0 > [92146.548227] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > [92146.548263] [] ? intel_wait_for_pipe_off+0x184/0x190 [i915] > [92146.548270] [] warn_slowpath_fmt+0x33/0x40 > [92146.548307] [] intel_wait_for_pipe_off+0x184/0x190 [i915] > [92146.548344] [] intel_disable_pipe+0x102/0x190 [i915] > [92146.548380] [] ? intel_disable_plane+0x64/0x80 [i915] > [92146.548417] [] i9xx_crtc_disable+0xbc/0x150 [i915] > [92146.548456] [] intel_crtc_update_dpms+0x5e/0x90 [i915] > [92146.548493] [] intel_modeset_setup_hw_state+0x42f/0x8f0 [i915] > [92146.548535] [] intel_lid_notify+0x9b/0xc0 [i915] > [92146.548543] [] notifier_call_chain+0x43/0x60 > [92146.548550] [] __blocking_notifier_call_chain+0x41/0x80 > [92146.548556] [] blocking_notifier_call_chain+0x1f/0x30 > [92146.548563] [] acpi_lid_send_state+0x78/0xa4 > [92146.548569] [] acpi_button_notify+0x3b/0xf1 > [92146.548577] [] ? acpi_os_execute+0x17/0x19 > [92146.548582] [] ? acpi_ec_sync_query+0xa5/0xbc > [92146.548589] [] acpi_device_notify+0x16/0x18 > [92146.548595] [] acpi_ev_notify_dispatch+0x38/0x4f > [92146.548600] [] acpi_os_execute_deferred+0x20/0x2b > [92146.548607] [] process_one_work+0x128/0x3f0 > [92146.548613] [] ? common_interrupt+0x33/0x38 > [92146.548618] [] ? wake_up_worker+0x30/0x30 > [92146.548624] [] ? acpi_os_wait_events_complete+0x1e/0x1e > [92146.548629] [] worker_thread+0x119/0x3b0 > [92146.548634] [] ? manage_workers+0x240/0x240 > [92146.548640] [] kthread+0x94/0xa0 > [92146.548647] [] ? ftrace_raw_output_sched_stat_runtime+0x70/0xf0 > [92146.548652] [] ret_from_kernel_thread+0x1b/0x28 > [92146.548658] [] ? kthread_create_on_node+0xc0/0xc0 > > so I'd like to use tristate for modeset_on_lid instead of bool. > -1: ingore all the lid events. > 0: do not do modeset when there is a lid-open event. > 1: do modeset when there is a lid-open event. > In this way, only device resume code will do modeset in a suspend/resume > cycle. > > Signed-off-by: Zhang Rui Given that this essentially requires users to manually set this module option to make stuff work I don't like this. I see a few possible options: - plug the races between all the different parts - I've never really understood why acpi sends us events before the resume code has completed ... If that's indeed intentional, we could delay the handling a bit until at least the i915 resume code completed. - Judging from the diff context you're not on the latest 3.8-rc codebase - we've applied a few fixes to this lid hack recently. Please retest on a kernel with commit 45e2b5f640b3766da3eda48f6c35f088155c06f3 Author: Daniel Vetter Date: Fri Nov 23 18:16:34 2012 +0100 drm/i915: force restore on lid open - This lid hack is only required since a few moronic BIOS implementations touch the display hw state behind our backs. On platforms with OpRegion support this shouldn't be the case any longer (which means pretty much everything shipped in the last few years), so we could conditionalize this hack on t
Re: [git pull] fbcon locking fixes.
On Fri, Jan 25, 2013 at 1:50 AM, Andrew Morton wrote: > On Fri, 25 Jan 2013 00:42:37 + (GMT) > Dave Airlie wrote: > >> These patches have been sailing around long enough, waiting for a maintainer >> to reappear, so I've decided enough is enough, lockdep is kinda useful to >> have. >> >> Thanks to Daniel for annoying me enough :-) > > Me too, but the patches broke Maarten's EFI driver setup: > https://lkml.org/lkml/2013/1/15/203. Oops, didn't know that there are open issues, otherwise I'd have worked on fixes instead of annoying you guys. To make due I've crawled through fbcon code a bit to hunt for ways we could solve this mess better than just flinging duct-tape. All the patches extend the console_lock'ed area quite a bit, and I don't really like big kernel looks with creeping coverage. More so if they come with funny semantics like console_lock attached. Looking at the fb notifier there are three use-cases for it: - Special blank/unblank handling, used by the backlight and lcd drivers in the fbdev subsystem. Could easily be extracted to another notifier chain, and seems to have no need for the console lock. Also, for kms drivers I don't care about this one bit. - suspend/resume handling for the same fbdev lcd drivers. Same applies (besides that it probably should get converted to proper device model suspend/resume stuff). - Runtime dependency injection between fbdev (and vga_switcheroor) for fbcon. This allows you to load fbcon.ko and fbdev drives in any order, and end up with an fbcon on each fbdev. Or not load fbcon.ko at all, and only use the underlying fbdev. On a quick look it also racy as hell. The last one is the tricky one. If we could just add a hard (compile-time) dependency between fbdevs and fbcon, we could rip out all the fb notifier madness which also involves the console_lock in some way and should be able to straighten out the locking. Users could still opt out of using the fbcon at runtime, but ofc this might break a really obscure setup somewhere. Quick survey of distros shows though that all use the sane option of built-in fbcon support. My proposal is to change the fbcon tristate to a bool and see what happens. If it works out for a few kernel releases, garbage-collect the fb_notifier for fbcon and use direct function calls instead (stubbed it out properly if fbcon is disabeld ofc). Or is that too much bending of the "thou shalt not break stuff" rule? Plan B would be to simply mash-up a console on top of kms drivers directly (since that's what I care about). Which has the downside that we'd still need to keep the drm fbdev emulation helper code around, since quite a few people use that to run older userspace (and even report some bugs, e.g. mplayer on the linux console played some tricks we needed to catch). And so end up with two pieces of code who's main job is to paint console output onto the screen. Ideas? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Sun, Jan 27, 2013 at 11:21 PM, Rafael J. Wysocki wrote: >> Given that this essentially requires users to manually set this module >> option to make stuff work I don't like this. >> >> I see a few possible options: >> - plug the races between all the different parts - I've never really >> understood why acpi sends us events before the resume code has >> completed ... > > This particular one may be a result of patch [2/3] in the series, > actually, because that patch makes SCI work over the whole cycle. > >> If that's indeed intentional, we could delay the >> handling a bit until at least the i915 resume code completed. > > I would do that for now at least if possible. It shouldn't hurt anyway. Since I lack a bit clue about how the pm stuff works exactly: Is there a ready-made helper for that kind of synchronization, where a notifier callback can be called from a different device's resume callbacks, which is someplace completely unrelated in the device-tree? Or any anti-patterns to avoid? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [RFC PATCH 3/3] i915: ignore lid open event when resuming
On Mon, Jan 28, 2013 at 3:36 AM, Zhang Rui wrote: >> Given that this essentially requires users to manually set this module >> option to make stuff work I don't like this. >> > sorry, I do not understand. > this patch just disables modeset_on_lid during suspend. Pardon me, the misunderstanding is on my side - I've mixed up dev_priv->modeset_on_lid with the corresponding module option. Is my understanding correct that only with the new SCI pm state this can happen, since that still allows acpi events to be processed (and so our lid_notifier being called? >> I see a few possible options: >> - plug the races between all the different parts - I've never really >> understood why acpi sends us events before the resume code has >> completed ... If that's indeed intentional, we could delay the >> handling a bit until at least the i915 resume code completed. > > IMO, the real question is that, during a suspend/resume cycle, if we > need to take care of the lid open event or not. > To me, the answer is no, because when resuming from STR, i915 is not > aware of such an event, and the i915 resume code works well, right? > so I do not think it is a problem to ignore the lid event for another > lightweight suspend state, which is introduced in Patch 1/3 in this > series. > >> - Judging from the diff context you're not on the latest 3.8-rc >> codebase - we've applied a few fixes to this lid hack recently. Please >> retest on a kernel with >> > the patch is based on 3.8.0-rc3, which already includes the commit > below. > And yes, the problem still exists. Ok, I think I see the issue now. But I suspect that we need some additional locking to make this solid, since currently dev_priv->modeset_on_lid updates can race with our lid notifier handler. Let me think about this a bit more. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linus GIT 3.8.0-rc5: INFO: possible circular locking dependency detected -- ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x49/0x80
d by 99video/4306: > [ 489.832304] #0: (&buffer->mutex){+.+.+.}, at: > [] sysfs_write_file+0x37/0x121 > [ 489.832310] #1: (s_active#204){.+.+.+}, at: [] > sysfs_write_file+0xd1/0x121 > [ 489.832315] #2: (&fb_info->lock){+.+.+.}, at: > [] lock_fb_info+0x18/0x37 > [ 489.832321] #3: (console_lock){+.+.+.}, at: [] > store_fbstate+0x43/0x71 > [ 489.832321] > [ 489.832321] stack backtrace: > [ 489.832324] Pid: 4306, comm: 99video Not tainted 3.8.0-rc5 #99 > [ 489.832325] Call Trace: > [ 489.832330] [] print_circular_bug+0x1f6/0x204 > [ 489.832333] [] __lock_acquire+0xacc/0xe0c > [ 489.832337] [] lock_acquire+0xfe/0x14d > [ 489.832341] [] ? > __blocking_notifier_call_chain+0x49/0x80 > [ 489.832345] [] down_read+0x3f/0x4b > [ 489.832348] [] ? > __blocking_notifier_call_chain+0x49/0x80 > [ 489.832352] [] __blocking_notifier_call_chain+0x49/0x80 > [ 489.832356] [] blocking_notifier_call_chain+0xf/0x11 > [ 489.832359] [] fb_notifier_call_chain+0x16/0x18 > [ 489.832362] [] fb_set_suspend+0x22/0x4b > [ 489.832365] [] ? console_lock+0x64/0x66 > [ 489.832368] [] store_fbstate+0x4e/0x71 > [ 489.832372] [] dev_attr_store+0x13/0x1f > [ 489.832375] [] sysfs_write_file+0xe9/0x121 > [ 489.832378] [] vfs_write+0x91/0xd0 > [ 489.832381] [] sys_write+0x5a/0x8b > [ 489.832385] [] system_call_fastpath+0x16/0x1b > [ 490.115770] PM: Syncing filesystems ... done. -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the drm-intel tree
On Tue, Feb 19, 2013 at 3:01 AM, Stephen Rothwell wrote: > On Fri, 15 Feb 2013 08:16:26 -0800 Jesse Barnes > wrote: >> >> On Fri, 15 Feb 2013 10:30:16 +0100 >> Daniel Vetter wrote: >> >> > On Fri, Feb 15, 2013 at 3:37 AM, Stephen Rothwell >> > wrote: >> > > >> > > After merging the drm-intel tree, today's linux-next build (x86_64 >> > > allmodconfig) failed like this: >> > > >> > > ERROR: "pm_vt_switch_unregister" [drivers/video/fb.ko] undefined! >> > > >> > > I have dropped the tree for today. >> > >> > Meh, that fail was already reported from Wu's kernel builder a few >> > days ago, but no patch yet showed up to fix things. Since the i915 >> > side of that work isn't ready yet either I've dropped the offending >> > patches. >> >> I sent a patch yesterday for this. I'll bounce it over again. > > I am still getting that build failure. Ok, should be really fixed now, my apologies for the mess. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 black screen introduced by ACPI changes
On Wed, Feb 20, 2013 at 8:17 AM, Chris Li wrote: > On Tue, Feb 19, 2013 at 12:52 PM, Jesse Barnes > wrote: >> Well it definitely sounds i915 related. I was just thinking that if >> certain bits were routed to the nvidia chip instead of the i915 one, >> the i915 driver may get confused and panic. > > That is certainly possible. > >> For debugging, you could modify the modeset_init function in i915_dma.c >> and make it return early with an error. You could use that to narrow >> down which part of init was failing. > > I did try that. It seems there is not fail at modeset_init function. The > function complete successfully. Here is the related section. > [i915 Hack] was the line add by me. > > There is one line said: "vgaarb: transferring owner from > PCI::00:02.0 to PCI::01:00.0". > Does it mean the display switch to the Nvidia video card? vgaarb is just about legacy VGA access routing, gpu switching is done with vgaswitcheroo (if you have such a platform). Still, it could be that the bios exposes different such interfaces with the differen acpi os name. Can you please check the dmesg of a working kernel (or attach it)? > [4.917591] i915 :00:02.0: setting latency timer to 64 > [4.962649] i915 :00:02.0: irq 43 for MSI/MSI-X > [4.962656] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010). > [4.962660] [drm] Driver supports precise vblank timestamp query. > [4.962663] [i915 Hack] enter modeset > [4.962670] [i915 Hack] after intel_parse_bios > [4.962672] [i915 Hack] after vga_client_register > [4.962727] [i915 Hack] after intel_register_dsm_handler > [4.962730] [i915 Hack] after intel_switchroo_register_client > [4.962733] [i915 Hack] after i915_gen_init_stolen > [4.962752] vgaarb: device changed decodes: > PCI::00:02.0,olddecodes=io+mem,decodes=none:owns=io+mem > [4.962757] vgaarb: transferring owner from PCI::00:02.0 to > PCI::01:00.0 > [5.018486] [i915 Hack] after intel_modeset_init > [5.020823] [i915 Hack] after i915_gem_init > [5.020869] [i915 Hack] after intel_modeset_gem_init > [5.020892] [i915 Hack] after drm_irq_install > [5.051980] fbcon: inteldrmfb (fb0) is primary device > [5.089303] psmouse serio1: synaptics: Touchpad model: 1, fw: 8.1, > id: 0x1e2b1, caps: 0xd00123/0x840300/0x123c00, board id: 1800, fw id: > 1087391 > [5.202472] input: SynPS/2 Synaptics TouchPad as > /devices/platform/i8042/serio1/input/input5 > [5.711448] Console: switching to colour frame buffer device 240x67 > [5.722628] i915 :00:02.0: fb0: inteldrmfb frame buffer device > [5.722630] i915 :00:02.0: registered panic notifier > [5.722633] [i915 Hack] after intel_fbdev_init > [5.722634] [i915 Hack] after drm_kms_helper_poll_init > [5.722635] [i915 Hack] exit mode switch > [5.722682] nouveau :01:00.0: enabling device (0006 -> 0007) > [5.723017] [Firmware Bug]: ACPI(PEGP) defines _DOD but not _DOS > [5.733222] acpi device:3c: registered as cooling_device8 > [5.733587] ACPI: Video Device [PEGP] (multi-head: yes rom: yes post: no) > [5.733746] input: Video Bus as > /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:3a/LNXVIDEO:00/input/input6 > [5.745904] acpi device:48: registered as cooling_device9 > > I attach the dmesg and config of the kernel. It is with the tip of git tree. > > BTW, my previous claim was wrong about the kernel is dead. It turn out the > wifi is usable after the black screen, I just need to wait long > enough. The kernel > has no panic at all. That explains why I didnot get a kernel crash > dump previously. > The caps locks LED actually works. Starts to smell like a bog-standard i915 black screen bug (albeit with a strange cause for the regression). Can you please boot the broken kernels again with drm.debug=0xe and grab the complete dmesg? Depending upon how long it takes for your wifi to get up, you might need to extend the log buffer, it dumps a lot. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 black screen introduced by ACPI changes
On Wed, Feb 20, 2013 at 8:45 PM, Chris Li wrote: > On Wed, Feb 20, 2013 at 2:57 AM, Daniel Vetter wrote: > >> Starts to smell like a bog-standard i915 black screen bug (albeit with >> a strange cause for the regression). Can you please boot the broken >> kernels again with drm.debug=0xe and grab the complete dmesg? >> Depending upon how long it takes for your wifi to get up, you might >> need to extend the log buffer, it dumps a lot. > > Here is the dmesg with "drm.debug=0xe". It is from the tip of git > kernel. The computer has the black screen. > Let me know what else do you need. The debug option did not stick for some reason. Also you're hitting a bunch of WARNs in our driver self-checks. Iirc those should be fixed in the latest code. Can you please test with the latest drm-intel-nightly branch from http://cgit.freedesktop.org/~danvet/drm-intel Also can you please attach a dmesg from a working kernel without the i915.modeset=0 option? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/i915: use do_div() as needed in debugfs code
On Mon, Mar 11, 2013 at 10:46:31PM -0700, Kees Cook wrote: > This replaces the open-coded divisions in the debugfs code by calls > to do_div(). > > Signed-off-by: Kees Cook > Cc: Daniel Vetter Squashed into the debugfs patch which introduced this regression, thanks for the quick fixup. -Daniel > --- > drivers/gpu/drm/i915/i915_debugfs.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index d86c304..6f3cbf8 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1814,9 +1814,9 @@ i915_max_freq_set(void *data, u64 val) > /* >* Turbo will still be enabled, but won't go above the set value. >*/ > - dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER; > - > - gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER); > + do_div(val, GT_FREQUENCY_MULTIPLIER); > + dev_priv->rps.max_delay = val; > + gen6_set_rps(dev, val); > mutex_unlock(&dev_priv->rps.hw_lock); > > return 0; > @@ -1865,9 +1865,9 @@ i915_min_freq_set(void *data, u64 val) > /* >* Turbo will still be enabled, but won't go below the set value. >*/ > - dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER; > - > - gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER); > + do_div(val, GT_FREQUENCY_MULTIPLIER); > + dev_priv->rps.min_delay = val; > + gen6_set_rps(dev, val); > mutex_unlock(&dev_priv->rps.hw_lock); > > return 0; > -- > 1.7.9.5 > > > -- > Kees Cook > Chrome OS Security -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree (drm-intel tree related)
On Tue, Mar 12, 2013 at 03:22:26PM +1100, Stephen Rothwell wrote: > Hi all, > > After merging the final tree, today's linux-next build (i386 defconfig) > failed like this: > > drivers/built-in.o: In function `i915_min_freq_set': > i915_debugfs.c:(.text+0xb1adc): undefined reference to `__udivdi3' > drivers/built-in.o: In function `i915_max_freq_set': > i915_debugfs.c:(.text+0xb1bac): undefined reference to `__udivdi3' > > Caused by commit 2389cc500686 ("drm/i915: use simple attribute in debugfs > routines") from the drm-intel tree. > > I have reverted that commit for today. Should be fixed now, thanks for the report (and my apologies for the little screw-up). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] drm/i915: bounds check execbuffer relocation count
On Tue, Mar 12, 2013 at 09:07:46AM +, Chris Wilson wrote: > On Mon, Mar 11, 2013 at 05:31:45PM -0700, Kees Cook wrote: > > It is possible to wrap the counter used to allocate the buffer for > > relocation copies. This could lead to heap writing overflows. > > > > CVE-2013-0913 > > > > v3: collapse test, improve comment > > v2: move check into validate_exec_list > > > > Signed-off-by: Kees Cook > > Reported-by: Pinkie Pie > > Cc: sta...@vger.kernel.org > > Looks good to me. The only bikeshed that remains is whether we should > just collapse the two variables into one, but the current 'max - count' > is more idiomatic and so preferrable. > Reviewed-by: Chris Wilson Picked up for -fixes, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] drm/i915: bounds check execbuffer relocation count
On Wed, Mar 13, 2013 at 9:28 PM, Daniel Vetter wrote: > On Tue, Mar 12, 2013 at 09:07:46AM +, Chris Wilson wrote: >> On Mon, Mar 11, 2013 at 05:31:45PM -0700, Kees Cook wrote: >> > It is possible to wrap the counter used to allocate the buffer for >> > relocation copies. This could lead to heap writing overflows. >> > >> > CVE-2013-0913 >> > >> > v3: collapse test, improve comment >> > v2: move check into validate_exec_list >> > >> > Signed-off-by: Kees Cook >> > Reported-by: Pinkie Pie >> > Cc: sta...@vger.kernel.org >> >> Looks good to me. The only bikeshed that remains is whether we should >> just collapse the two variables into one, but the current 'max - count' >> is more idiomatic and so preferrable. >> Reviewed-by: Chris Wilson > > Picked up for -fixes, thanks for the patch. I've forgotten to dump my wishlist: Can I have an i-g-t for this? For this bug here specifically an execbuf with just one buffer with too many relocs plus another execbuf with two buffers with relocation so that the 2nd relocation list will overflow should be sufficient. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PULL] drm-intel-next
Hi Dave, New stuff for -next. Highlights: - prep patches for the modeset rework. Note that one of those patches touches the fb helper in the common drm code. - hasw hdmi audio support (Wang Xingchao) - improved instdone dumping for gen7 (Ben) - unbound tracking and a few follow-up patches from Chris - dma_buf->begin/end_cpu_access plus fix for drm/udl (Dave) - improve mmio error reporting for hsw - prep patch for WQ_NON_REENTRANT removal (Tejun Heo) I've expected a conflict with the raw_edid removal, but on second look the conflicting patch went through -fixes and you've already resolved it. I've also included a fixup on top of what QA tested to avoid a conflict with linux-next (the NO_KSWAP removal). Cheers, Daniel The following changes since commit a22ddff8bedfe33eeb1330bbb7ef1fbe007a42c4: Merge tag 'v3.6-rc2' into drm-intel-next (2012-08-17 09:01:08 +0200) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel for-airlied for you to fetch changes up to d7c3b937bdf45f0b844400b7bf6fd3ed50bac604: drm/i915: Remove __GFP_NO_KSWAPD (2012-08-27 17:11:38 +0200) Ben Widawsky (5): drm/i915: Add ERR_INT to gen7 error state drm/i915: Find unclaimed MMIO writes. drm/i915: Extract reading INSTDONE drm/i915: Add new INSTDONE registers drm/i915: Use new INSTDONE registers (Gen7+) Chris Wilson (13): drm/i915: Track unbound pages drm/i915: Add some sanity checks to unbound tracking drm/i915: Show (count, size) of purgeable objects in i915_gem_objects drm/i915: Show pin count in debugfs drm/i915: Try harder to allocate an mmap_offset drm/i915: Cantiga+ cannot handle a hsync front porch of 0 drm/i915: Only pwrite through the GTT if there is space in the aperture drm/i915: Protect private gem objects from truncate (such as imported dmabuf) drm/i915: Extract general object init routine drm/i915: Use cpu relocations if the object is in the GTT but not mappable drm/i915: Juggle code order to ease flow of the next patch drm/i915: Use a non-blocking wait for set-to-domain ioctl drm/i915: Avoid unbinding due to an interrupted pin_and_fence during execbuffer Damien Lespiau (1): drm/i915: Don't hardcode the number of pipes in the error state dump Daniel Vetter (11): drm/i915: add missing gen2 pipe A quirk entries drm/i915/ns2501: kill pll A enabling hack drm/i915: rip out the overlay pipe A workaround drm/i915: prepare load-detect pipe code for dpms changes drm/i915: drop intel_encoder argument to load_detect_pipe functions drm/i915: simplify dvo dpms interface drm/i915: kill a few unused things in dev_priv drm/i915: extract ironlake_fdi_pll_disable drm/fb-helper: don't clobber output routing in setup_crtcs drm/i915: move functions around drm/i915: disable rc6 on ilk when vt-d is enabled Dave Airlie (2): drm/i915: implement dma buf begin_cpu_access (v2) drm/udl: call begin/end cpu access at more appropriate time Keith Packard (1): drm/i915: Allow VGA on CRTC 2 Sedat Dilek (1): drm/i915: Remove __GFP_NO_KSWAPD Tejun Heo (1): i915: use alloc_ordered_workqueue() instead of explicit UNBOUND w/ max_active = 1 Wang Xingchao (4): drm/i915: HSW audio registers definition drm/i915: write eld info for HDMI audio drm/i915: ironlake_write_eld code cleanup drm/i915: Haswell HDMI audio initialization Xu, Anhua (2): drm/i915: fix wrong order of parameters in port checking functions drm/i915: fix reassignment of variable "intel_dp->DP" drivers/gpu/drm/drm_fb_helper.c|6 - drivers/gpu/drm/i915/dvo.h |9 +- drivers/gpu/drm/i915/dvo_ch7017.c |8 +- drivers/gpu/drm/i915/dvo_ch7xxx.c |4 +- drivers/gpu/drm/i915/dvo_ivch.c|8 +- drivers/gpu/drm/i915/dvo_ns2501.c | 21 +- drivers/gpu/drm/i915/dvo_sil164.c |4 +- drivers/gpu/drm/i915/dvo_tfp410.c |4 +- drivers/gpu/drm/i915/i915_debugfs.c| 40 +- drivers/gpu/drm/i915/i915_dma.c| 31 +- drivers/gpu/drm/i915/i915_drv.c|4 + drivers/gpu/drm/i915/i915_drv.h| 39 +- drivers/gpu/drm/i915/i915_gem.c| 1100 +++- drivers/gpu/drm/i915/i915_gem_context.c|4 +- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 37 +- drivers/gpu/drm/i915/i915_gem_evict.c | 19 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 126 ++-- drivers/gpu/drm/i915/i915_gem_gtt.c|2 +- drivers/gpu/drm/i915/i915_irq.c| 76 +- drivers/gpu/drm/i915/i915_reg.h| 79 ++ drivers/gpu/drm/i915/i915_trace.h | 10 +- drivers/gpu/drm/i915/intel_crt.c |8 +-
Re: Linux 3.5.3 - i915 error
On Fri, Aug 31, 2012 at 09:34:17AM +0200, Paul Rolland wrote: > Hello, > > I've just found this error in my logs. It occured once so far, but I never > got it before with 3.5.0, so I'm reporting it. > > [drm] capturing error event; look for more information > in /debug/dri/0/i915_error_state > i915: render error detected, EIR: 0x0010 > i915: IPEIR: 0x > i915: IPEHR: 0x0100 > i915: INSTDONE: 0xfffe > i915: INSTPS: 0x0001e000 > i915: INSTDONE1: 0x > i915: ACTHD: 0xad01f678 > i915: page table error > i915: PGTBL_ER: 0x0001 > [drm:i915_report_and_clear_eir] *ERROR* EIR stuck: 0x0010, masking > > Content of /debug/dri/0/i915_error_state attached as error.gz On a quick look this is a hange in the gen4 mesa code, nothing gone wrong that could be blamed on the kernel part of the driver. Please upgrade to the latest mesa (maybe even try git). If you can still reproduce this (and you've figured out a way to reliably reproduce this), please file a bug on bugs.freedesktop.org agains mesa -> intel/i965. Thanks, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[pull] drm-intel-next
Hi Dave, The big ticket item here is the new i915 modeset infrastructure. Shockingly it didn't not blow up all over the place (i.e. I've managed to fix the ugly issues before merging). 1-2 smaller corner cases broke, but we have patches. Also, there's tons of patches on top of this that clean out cruft and fix a few bugs that couldn't be fixed with the crtc helper based stuff. So more stuff to come ;-) Also a few other things: - Tiny fix in the fb helper to go through the official dpms interface instead of calling the crtc helper code. - forcewake code frobbery from Ben, code should be more in-line with what Windows does now. - fixes for the render ring flush on hsw (Paulo) - gpu frequency tracepoint - vlv forcewake changes to better align it with our understanding of the forcewake magic. - a few smaller cleanups Cheers, Daniel The following changes since commit d7c3b937bdf45f0b844400b7bf6fd3ed50bac604: drm/i915: Remove __GFP_NO_KSWAPD (2012-08-27 17:11:38 +0200) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-next-2012-09-09 for you to fetch changes up to e04190e0ecb236c51af181c18c545ea076fb9cca: drm/fb helper: don't call drm_helper_connector_dpms directly (2012-09-08 00:51:15 +0200) Ben Widawsky (5): drm/i915: Extract forcewake ack timeout drm/i915: use cpu_relax() in wait_for_atomic drm/i915: Change forcewake timeout to 2ms drm/i915: Never read FORCEWAKE drm/i915: Enable some sysfs stuff without CONFIG_PM Chris Wilson (1): drm/i915: Convert remaining debugfs iterators over rings to for_each_ring() Daniel Vetter (66): drm/ips: move drps/ips/ilk related variables into dev_priv->ips drm/i915: add a tracepoint for gpu frequency changes drm/i915: align vlv forcewake with common lore drm/i915: differ error message between forcwake timeouts drm/i915: add crtc->enable/disable vfuncs insted of dpms drm/i915: rip out crtc prepare/commit indirection drm/i915: add direct encoder disable/enable infrastructure drm/i915/hdmi: convert to encoder->disable/enable drm/i915/tv: convert to encoder enable/disable drm/i915/lvds: convert to encoder disable/enable drm/i915/dp: convert to encoder disable/enable drm/i915/crt: convert to encoder disable/enable drm/i915/sdvo: convert to encoder disable/enable drm/i915/dvo: convert to encoder disable/enable drm/i915: convert dpms functions of dvo/sdvo/crt drm/i915: rip out encoder->disable/enable checks drm/i915: clean up encoder_prepare/commit drm/i915: copy&paste drm_crtc_helper_set_config drm/i915: call set_base directly drm/i915: inline intel_best_encoder drm/i915: copy&paste drm_crtc_helper_set_mode drm/i915: simplify intel_crtc_prepare_encoders drm/i915: rip out encoder->prepare/commit drm/i915: call crtc functions directly drm/i915: WARN when trying to enabled an unused crtc drm/i915: Add interfaces to read out encoder/connector hw state drm/i915/dp: implement get_hw_state drm/i915/hdmi: implement get_hw_state drm/i915/tv: implement get_hw_state drm/i915/lvds: implement get_hw_state drm/i915/crt: implement get_hw_state drm/i915/sdvo: implement get_hw_state drm/i915/dvo: implement get_hw_state drm/i915: read out the modeset hw state at load and resume time drm/i915: check connector hw/sw state drm/i915: rip out intel_crtc->dpms_mode drm/i915: rip out intel_dp->dpms_mode drm/i915: ensure the force pipe A quirk is actually followed drm/i915: introduce struct intel_set_config drm/i915: extract modeset config save/restore code drm/i915: extract intel_set_config_compute_mode_changes drm/i915: extract intel_set_config_update_output_state drm/i915: implement crtc helper semantics relied upon by the fb helper drm/i915: don't update the fb base if there is no fb drm/i915: convert pointless error checks in set_config to BUGs drm/i915: don't save all the encoder/crtc state in set_config drm/i915: stage modeset output changes drm/i915: push crtc->fb update into pipe_set_base drm/i915: remove crtc disabling special case drm/i915: move output commit and crtc disabling into set_mode drm/i915: extract adjusted mode computation drm/i915: use staged outuput config in tv->mode_fixup drm/i915: use staged outuput config in lvds->mode_fixup drm/i915: compute masks of crtcs affected in set_mode drm/i915: implement new set_mode code flow drm/i915: push commit_output_state past crtc disabling drm/i915: s/intel_encoder_disable/intel_encoder_noop drm/i915: WARN if the pipe won't turn off drm/i915:
Re: [Intel-gfx] [pull] drm-intel-next
On Fri, Sep 14, 2012 at 09:55:58AM -0400, Bobby Powers wrote: > This tree gives me recursive dependency problems, which ends up > removing a big (& important) part of my .config: > > [bpowers@fina linux]$ git reset --hard drm-intel-next-2012-09-09 > HEAD is now at e04190e drm/fb helper: don't call > drm_helper_connector_dpms directly > [bpowers@fina linux]$ git status > # On branch master > # Your branch and 'origin/master' have diverged, > # and have 207 and 323 different commits each, respectively. > # > nothing to commit (working directory clean) > [bpowers@fina linux]$ make oldconfig > scripts/kconfig/conf --oldconfig Kconfig > drivers/gpu/drm/udl/Kconfig:1:error: recursive dependency detected! > drivers/gpu/drm/udl/Kconfig:1:symbol DRM_UDL depends on > USB_ARCH_HAS_HCD > drivers/usb/Kconfig:76: symbol USB_ARCH_HAS_HCD depends on USB_SUPPORT > drivers/usb/Kconfig:58: symbol USB_SUPPORT is selected by DRM_USB > drivers/gpu/drm/Kconfig:22: symbol DRM_USB is selected by DRM_UDL > # > # configuration written to .config > # That's an issue with Dave Airlie's udl code I'd say - the drm-intel-next tree here simply includes a few drm-next patches already. Dave? -Daniel > > > I've attached my config & the diff between what is attached and the > result of make oldconfig. Let me know if there is any other info that > would help, or if I'm just doing something boneheaded. Thanks! > > yours, > Bobby > > On Thu, Sep 13, 2012 at 10:18 AM, Daniel Vetter wrote: > > Hi Dave, > > > > The big ticket item here is the new i915 modeset infrastructure. > > Shockingly it didn't not blow up all over the place (i.e. I've managed to > > fix the ugly issues before merging). 1-2 smaller corner cases broke, but > > we have patches. Also, there's tons of patches on top of this that clean > > out cruft and fix a few bugs that couldn't be fixed with the crtc helper > > based stuff. So more stuff to come ;-) > > > > Also a few other things: > > - Tiny fix in the fb helper to go through the official dpms interface > > instead of calling the crtc helper code. > > - forcewake code frobbery from Ben, code should be more in-line with > > what Windows does now. > > - fixes for the render ring flush on hsw (Paulo) > > - gpu frequency tracepoint > > - vlv forcewake changes to better align it with our understanding of the > > forcewake magic. > > - a few smaller cleanups > > > > Cheers, Daniel > > > > > > The following changes since commit d7c3b937bdf45f0b844400b7bf6fd3ed50bac604: > > > > drm/i915: Remove __GFP_NO_KSWAPD (2012-08-27 17:11:38 +0200) > > > > are available in the git repository at: > > > > git://people.freedesktop.org/~danvet/drm-intel > > tags/drm-intel-next-2012-09-09 > > > > for you to fetch changes up to e04190e0ecb236c51af181c18c545ea076fb9cca: > > > > drm/fb helper: don't call drm_helper_connector_dpms directly (2012-09-08 > > 00:51:15 +0200) > > > > > > > > Ben Widawsky (5): > > drm/i915: Extract forcewake ack timeout > > drm/i915: use cpu_relax() in wait_for_atomic > > drm/i915: Change forcewake timeout to 2ms > > drm/i915: Never read FORCEWAKE > > drm/i915: Enable some sysfs stuff without CONFIG_PM > > > > Chris Wilson (1): > > drm/i915: Convert remaining debugfs iterators over rings to > > for_each_ring() > > > > Daniel Vetter (66): > > drm/ips: move drps/ips/ilk related variables into dev_priv->ips > > drm/i915: add a tracepoint for gpu frequency changes > > drm/i915: align vlv forcewake with common lore > > drm/i915: differ error message between forcwake timeouts > > drm/i915: add crtc->enable/disable vfuncs insted of dpms > > drm/i915: rip out crtc prepare/commit indirection > > drm/i915: add direct encoder disable/enable infrastructure > > drm/i915/hdmi: convert to encoder->disable/enable > > drm/i915/tv: convert to encoder enable/disable > > drm/i915/lvds: convert to encoder disable/enable > > drm/i915/dp: convert to encoder disable/enable > > drm/i915/crt: convert to encoder disable/enable > > drm/i915/sdvo: convert to encoder disable/enable > > drm/i915/dvo: convert to encoder disable/enable > > drm/i915: convert dpms functions of dvo/sdvo/crt > > drm/i915: rip out encoder->disable/enable che
Re: [PATCH] i915: Don't register backlight when max PWM value is unknown
On Fri, Sep 14, 2012 at 04:34:28PM +0100, Grant Likely wrote: > When a backlight isn't connected to the i915 it doesn't make any sense > to register the backlight device, but the driver currently tries to limp > along using a max brightness value of 1. Instead, this patch makes it so > that if the maximum PWM value cannot be determined, then the backlight > will not be registered. > > Tested on MacbookPro8,3. > > Signed-off-by: Grant Likely > Cc: Daniel Vetter > Cc: David Airlie > Cc: Matthew Garrett > Cc: David Woodhouse I've already merged a rather similar patch from Jani Nikula commit 28dcc2d60cb570d9f549c329b2f51400553412a1 Author: Jani Nikula Date: Mon Sep 3 16:25:12 2012 +0300 drm/i915: do not expose a dysfunctional backlight interface to userspace Should land in 3.6 rsn. -Daniel > --- > drivers/gpu/drm/i915/intel_panel.c | 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index 3df4f5f..f410c6e 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -168,13 +168,8 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev) > u32 max; > > max = i915_read_blc_pwm_ctl(dev_priv); > - if (max == 0) { > - /* XXX add code here to query mode clock or hardware clock > - * and program max PWM appropriately. > - */ > - pr_warn_once("fixme: max PWM is zero\n"); > - return 1; > - } > + if (max == 0) > + return 0; /* Cannot read max PWM. Assume no backlight */ > > if (HAS_PCH_SPLIT(dev)) { > max >>= 16; > @@ -413,6 +408,12 @@ int intel_panel_setup_backlight(struct drm_device *dev) > struct backlight_properties props; > struct drm_connector *connector; > > + /* Is there a backlight present? max will be zero if not */ > + if (intel_panel_get_max_backlight(dev) == 0) { > + DRM_INFO("i915 doesn't seem to be connected to backlight\n"); > + return 0; > + } > + > intel_panel_init_backlight(dev); > > if (dev_priv->int_lvds_connector) > -- > 1.7.9.5 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: 3.4.10 i915 [GM45] regression
On Mon, Oct 1, 2012 at 11:56 PM, Andreas Sturmlechner wrote: >> Andreas, can you please check whether 3.5.x works for you, just to >> make sure that this particular elephant is only wreaking havoc in 3.4 >> and earlier? > > I've been using 3.5 all the time since .0, I never had any trouble there - a > few reboots just now with a vanilla 3.5.4 image were > successful as expected. Meanwhile I have upgraded to 3.6, zero issues as well. So 3.5 and later really don't seem to have any issue with this patch. I'm as puzzled as ever. Thanks for testing this again. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
t; >I was swamped with other things and meant to do a full reply on monday. I > >didn't mean to make it sound > >like I only cared blindly about merging my code, just wanted to find a good > >solution. > >>We *need* to carefully weigh it against any benefits you have in your work, > >>and you need to test these codepaths > >>in parallell cases subject to heavy aperture / vram thrashing and frequent > >>signals causing interrupted waits. > >Agreed, is there already a tester for this or should I write my own? > Although I think it would be nice to have a highly parallel execbuf > implementation on an extremely simple software GPU, > what I typically do is to take an existing driver (none of them > implements parallel reserve yet, but vmware is about to soon) > > a) Use an application that frequently recycles buffers, so that the > delayed-delete code gets busy (Perhaps google-earth, panning over a > landscape not too high above the earth) > b) Hack the drivers aperture / vram sizes to something small, so > that you can see that the eviction code gets hit. > c) Adjust the memory limits in TTM sysfs memory accounting (You can > write and change on the fly), so that you can see that the swapping > code gets hit. > d) Code a signal delivery so that every 20th time or so the eviction > code is about to wait, it receives an -ERESTARTSYS with a harmless > signal. > e) start another instance of google-earth. tbh, this should be a simple testsuite that you can just run. Like we're (slowly) building up for drm/i915 in intel-gpu-tools. At least that'll be one of the merge requirements for i915.ko. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] console: implement lockdep support for console_lock
On Sat, Sep 22, 2012 at 10:06 PM, Greg KH wrote: > On Sat, Sep 22, 2012 at 07:52:11PM +0200, Daniel Vetter wrote: >> Dave Airlie recently discovered a locking bug in the fbcon layer, >> where a timer_del_sync (for the blinking cursor) deadlocks with the >> timer itself, since both (want to) hold the console_lock: >> >> https://lkml.org/lkml/2012/8/21/36 >> >> Unfortunately the console_lock isn't a plain mutex and hence has no >> lockdep support. Which resulted in a few days wasted of tracking down >> this bug (complicated by the fact that printk doesn't show anything >> when the console is locked) instead of noticing the bug much earlier >> with the lockdep splat. >> >> Hence I've figured I need to fix that for the next deadlock involving >> console_lock - and with kms/drm growing ever more complex locking >> that'll eventually happen. >> >> Now the console_lock has rather funky semantics, so after a quick irc >> discussion with Thomas Gleixner and Dave Airlie I've quickly ditched >> the original idead of switching to a real mutex (since it won't work) >> and instead opted to annotate the console_lock with lockdep >> information manually. >> >> There are a few special cases: >> - The console_lock state is protected by the console_sem, and usually >> grabbed/dropped at _lock/_unlock time. But the suspend/resume code >> drops the semaphore without dropping the console_lock (see >> suspend_console/resume_console). But since the same thread that did >> the suspend will do the resume, we don't need to fix up anything. >> >> - In the printk code there's a special trylock, only used to kick off >> the logbuffer printk'ing in console_unlock. But all that happens >> while lockdep is disable (since printk does a few other evil >> tricks). So no issue there, either. >> >> - The console_lock can also be acquired form irq context (but only >> with a trylock). lockdep already handles that. >> >> This all leaves us with annotating the normal console_lock, _unlock >> and _trylock functions. >> >> And yes, it works - simply unloading a drm kms driver resulted in >> lockdep complaining about the deadlock in fbcon_deinit: >> >> == >> [ INFO: possible circular locking dependency detected ] >> 3.6.0-rc2+ #552 Not tainted >> --- >> kms-reload/3577 is trying to acquire lock: >> ((&info->queue)){+.+...}, at: [] wait_on_work+0x0/0xa7 >> >> but task is already holding lock: >> (console_lock){+.+.+.}, at: [] bind_con_driver+0x38/0x263 >> >> which lock already depends on the new lock. >> >> the existing dependency chain (in reverse order) is: >> >> -> #1 (console_lock){+.+.+.}: >>[] lock_acquire+0x95/0x105 >>[] console_lock+0x59/0x5b >>[] fb_flashcursor+0x2e/0x12c >>[] process_one_work+0x1d9/0x3b4 >>[] worker_thread+0x1a7/0x24b >>[] kthread+0x7f/0x87 >>[] kernel_thread_helper+0x4/0x10 >> >> -> #0 ((&info->queue)){+.+...}: >>[] __lock_acquire+0x999/0xcf6 >>[] lock_acquire+0x95/0x105 >>[] wait_on_work+0x3b/0xa7 >>[] __cancel_work_timer+0xbf/0x102 >>[] cancel_work_sync+0xb/0xd >>[] fbcon_deinit+0x11c/0x1dc >>[] bind_con_driver+0x145/0x263 >>[] unbind_con_driver+0x14f/0x195 >>[] store_bind+0x1ad/0x1c1 >>[] dev_attr_store+0x13/0x1f >>[] sysfs_write_file+0xe9/0x121 >>[] vfs_write+0x9b/0xfd >>[] sys_write+0x3e/0x6b >>[] system_call_fastpath+0x16/0x1b >> >> other info that might help us debug this: >> >> Possible unsafe locking scenario: >> >>CPU0CPU1 >> >> lock(console_lock); >>lock((&info->queue)); >>lock(console_lock); >> lock((&info->queue)); >> >> *** DEADLOCK *** >> >> v2: Mark the lockdep_map static, noticed by Jani Nikula. >> >> Cc: Dave Airlie >> Cc: Thomas Gleixner >> Cc: Alan Cox >> Cc: Peter Zijlstra >> Signed-off-by: Daniel Vetter >> --- >> kernel/printk.c |9 + >> 1 file changed, 9 insertions(+) > > So I'm guessing I should take this through the tty tree, right? Any > objections to that for 3.7? I've noticed that the tty tree went in already :( Any chance you could still slip this in for 3.7? I'd _really_ like to have this stuff in for debugging console_lock madness in drm drivers - we've already had our fair share of those ... Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] console: implement lockdep support for console_lock
On Tue, Oct 2, 2012 at 3:28 PM, Greg KH wrote: > No, as it hasn't been in linux-next already, I can't send it in for 3.7, > sorry, you know that. I'll be glad to queue it up for 3.8 if you want me to. Hey, was worth a shot ;-) Yeah, if you can pick it up for 3.8, that would be nice, since the patches have been floating for a while by now ... Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
On Wed, Oct 3, 2012 at 9:45 AM, Thomas Hellstrom wrote: > On 10/02/2012 10:03 AM, Daniel Vetter wrote: >> >> On Tue, Oct 02, 2012 at 08:46:32AM +0200, Thomas Hellstrom wrote: >>> >>> On 10/01/2012 11:47 AM, Maarten Lankhorst wrote: >>>> >>>> I was doing a evil hack where I 'released' lru_lock to lockdep before >>>> doing the annotation >>>> for a blocking acquire, and left trylock annotations as they were. This >>>> made lockdep do the >>>> right thing. >>> >>> I've never looked into how lockdep works. Is this something that can >>> be done permanently or just for testing >>> purposes? Although not related to this, is it possible to do >>> something similar to the trylock reversal in the >>> fault() code where mmap_sem() and reserve() change order using a >>> reserve trylock? >> >> lockdep just requires a bunch of annotations, is a compile-time configure >> option CONFIG_PROVE_LOCKING and if disabled, has zero overhead. And it's >> rather awesome in detected deadlocks and handling crazy locking schemes >> correctly: >> - correctly handles trylocks >> - correctly handles nested locking (i.e. grabbing a global lock, then >>grabbing subordinate locks in an unordered sequence since the global >>lock ensures that no deadlocks can happen). >> - any kinds of inversions with special contexts like hardirq, softirq >> - same for page-reclaim, i.e. it will yell if you could (potentially) >>deadlock because your shrinker grabs a lock that you hold while calling >>kmalloc. >> - there are special annotates for various subsystems, e.g. to check for >>del_timer_sync vs. locks held by that timer. Or the console_lock >>annotations I've just recently submitted. >> - all that with a really flexible set of annotation primitives that afaics >>should work for almost any insane locking scheme. The fact that Maarten >>could come up with proper reservation annotations without any changes >> to >>lockdep testifies this (he only had to fix a tiny thing to make it a >> bit >>more strict in a corner case). >> >> In short I think it's made of awesome. The only downside is that it lacks >> documentation, you have to read the code to understand it :( >> >> The reason I've suggested to Maarten to abolish the trylock_reservation >> within the lru_lock is that in that way lockdep only ever sees the >> trylock, and hence is less strict about complainig about deadlocks. But >> semantically it's an unconditional reserve. Maarten had some horrible >> hacks that leaked the lockdep annotations out of the new reservation code, >> which allowed ttm to be properly annotated. But those also reduced the >> usefulness for any other users of the reservation code, and so Maarten >> looked into whether he could remove that trylock dance in ttm. >> >> Imo having excellent lockdep support for cross-device reservations is a >> requirment, and ending up with less strict annotations for either ttm >> based drivers or other drivers is not good. And imo the ugly layering that >> Maarten had in his first proof-of-concept also indicates that something is >> amiss in the design. >> >> > So if I understand you correctly, the reservation changes in TTM are > motivated by the > fact that otherwise, in the generic reservation code, lockdep can only be > annotated for a trylock and not a waiting lock, when it *is* in fact a > waiting lock. > > I'm completely unfamiliar with setting up lockdep annotations, but the only > place a > deadlock might occur is if the trylock fails and we do a > wait_for_unreserve(). > Isn't it possible to annotate the call to wait_for_unreserve() just like an > interruptible waiting lock > (that is always interrupted, but at least any deadlock will be catched?). Hm, I have to admit that idea hasn't crossed my mind, but it's indeed a hole in our current reservation lockdep annotations - since we're blocking for the unreserve, other threads could potential block waiting on us to release a lock we're holding already, resulting in a deadlock. Since no other locking primitive that I know of has this wait_for_unlocked interface, I don't know how we could map this in lockdep. One idea is to grab the lock and release it again immediately (only in the annotations, not the real lock ofc). But I need to check the lockdep code to see whether that doesn't trip it up. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
On Wed, Oct 3, 2012 at 10:37 AM, Thomas Hellstrom wrote: >>> So if I understand you correctly, the reservation changes in TTM are >>> motivated by the >>> fact that otherwise, in the generic reservation code, lockdep can only be >>> annotated for a trylock and not a waiting lock, when it *is* in fact a >>> waiting lock. >>> >>> I'm completely unfamiliar with setting up lockdep annotations, but the >>> only >>> place a >>> deadlock might occur is if the trylock fails and we do a >>> wait_for_unreserve(). >>> Isn't it possible to annotate the call to wait_for_unreserve() just like >>> an >>> interruptible waiting lock >>> (that is always interrupted, but at least any deadlock will be catched?). >> >> Hm, I have to admit that idea hasn't crossed my mind, but it's indeed >> a hole in our current reservation lockdep annotations - since we're >> blocking for the unreserve, other threads could potential block >> waiting on us to release a lock we're holding already, resulting in a >> deadlock. >> >> Since no other locking primitive that I know of has this >> wait_for_unlocked interface, I don't know how we could map this in >> lockdep. One idea is to grab the lock and release it again immediately >> (only in the annotations, not the real lock ofc). But I need to check >> the lockdep code to see whether that doesn't trip it up. > > > I imagine doing the same as mutex_lock_interruptible() does in the > interrupted path should work... It simply calls the unlock lockdep annotation function if it breaks out. So doing a lock/unlock cycle in wait_unreserve should do what we want. And to properly annotate the ttm reserve paths we could just add an unconditional wait_unreserve call at the beginning like you suggested (maybe with #ifdef CONFIG_PROVE_LOCKING in case ppl freak out about the added atomic read in the uncontended case). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 3.6.0+ (GIT) -- WARNING: at drivers/gpu/drm/i915/intel_display.c:1271 intel_disable_pipe+0xab/0x12e() -- plane B assertion failure, should be off on pipe B but is still active
This should help: https://patchwork.kernel.org/patch/1436231/ It's not merged yet since it cause a black screen on resume on one of my really old machines. Issue isn't new at all, but the new modeset code is _much_ more anal about cross-checking the hw state with what we expect to be there. Thanks, Daniel On Thu, Oct 4, 2012 at 4:18 PM, Miles Lane wrote: > [4.266874] WARNING: at drivers/gpu/drm/i915/intel_display.c:1271 > intel_disable_pipe+0xab/0x12e() > [4.266877] Hardware name: UL50VT > [4.266880] plane B assertion failure, should be off on pipe B but > is still active > [4.266882] Modules linked in: > [4.266888] Pid: 1, comm: swapper/0 Not tainted 3.6.0+ #27 > [4.266891] Call Trace: > [4.266899] [] warn_slowpath_common+0x7e/0x97 > [4.266905] [] warn_slowpath_fmt+0x41/0x43 > [4.266911] [] intel_disable_pipe+0xab/0x12e > [4.266917] [] i9xx_crtc_disable+0xd1/0x144 > [4.266923] [] intel_modeset_setup_hw_state+0x438/0x69f > [4.266929] [] intel_modeset_gem_init+0x1f/0x24 > [4.266936] [] i915_driver_load+0xada/0xcde > [4.266945] [] drm_get_pci_dev+0x16d/0x266 > [4.266951] [] ? trace_hardirqs_on+0xd/0xf > [4.266957] [] i915_pci_probe+0x4d/0x57 > [4.266964] [] local_pci_probe+0x5b/0xa2 > [4.266969] [] pci_device_probe+0xc0/0xed > [4.266976] [] ? driver_probe_device+0x1b5/0x1b5 > [4.266981] [] driver_probe_device+0x9a/0x1b5 > [4.266987] [] __driver_attach+0x4e/0x6f > [4.266992] [] bus_for_each_dev+0x52/0x84 > [4.266998] [] driver_attach+0x19/0x1b > [4.267003] [] bus_add_driver+0xf4/0x219 > [4.267009] [] driver_register+0x8e/0x114 > [4.267014] [] __pci_register_driver+0x5a/0x5f > [4.267020] [] drm_pci_init+0x81/0xe7 > [4.267027] [] ? ttm_init+0x62/0x62 > [4.267033] [] i915_init+0x66/0x68 > [4.267039] [] do_one_initcall+0x7a/0x130 > [4.267045] [] kernel_init+0x10b/0x18f > [4.267051] [] ? do_early_param+0x8c/0x8c > [4.267058] [] kernel_thread_helper+0x4/0x10 > [4.267065] [] ? finish_task_switch+0x38/0xb0 > [4.267070] [] ? __switch_to+0x19c/0x426 > [4.267076] [] ? retint_restore_args+0xe/0xe > [4.267082] [] ? start_kernel+0x3b3/0x3b3 > [4.267087] [] ? gs_change+0xb/0xb -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 problems with suspend to disk
On Thu, Oct 04, 2012 at 08:20:17PM +0100, Hugo Mills wrote: >Hi, > >On 3.6, I've got a problem with my video driver after resuming from > suspend-to-disk: the lower part of the display flickers, rapidly but > irregularly (think of a neon sign in a bad film noir), flicking > between the correct display and... something else, it's hard to see > what. > >Shutting the lid of the machine (to trigger a suspend-to-ram) and > waking it up again fixes the flicker. > >In syslogs, I have nothing obvious on suspend, and this warning on > resume: > > Oct 4 19:53:04 ruth kernel: [ 2892.184086] [ cut here > ] > Oct 4 19:53:04 ruth kernel: [ 2892.184114] WARNING: at > drivers/gpu/drm/i915/intel_display.c:1225 intel_crtc_disable+0x52/0x86 > [i915]() > Oct 4 19:53:04 ruth kernel: [ 2892.184115] Hardware name: 0196CTO > Oct 4 19:53:04 ruth kernel: [ 2892.184117] pipe B assertion failure > (expected off, current on) > Oct 4 19:53:04 ruth kernel: [ 2892.184188] Modules linked in: > cpufreq_powersave cpufreq_userspace cpufreq_stats cpufreq_conservative rfcomm > bnep binfmt_misc uinput nfsd auth_rpcgss nfs_acl nfs lockd fscache sunrpc > ext4 jbd2 mbcache loop arc4 iwldvm mac80211 snd_hda_codec_hdmi > snd_hda_codec_conexant coretemp i915 snd_hda_intel snd_hda_codec > drm_kms_helper iwlwifi snd_hwdep snd_pcm snd_page_alloc joydev snd_seq > snd_seq_device snd_timer kvm_intel drm i2c_algo_bit thinkpad_acpi > acpi_cpufreq nvram btusb bluetooth kvm crc16 cfg80211 microcode psmouse evdev > i2c_i801 video rfkill wmi mperf i2c_core snd battery ac processor soundcore > button btrfs crc32c libcrc32c zlib_deflate sha256_generic ablk_helper cryptd > aes_x86_64 aes_generic cbc dm_crypt dm_mod sd_mod crc_t10dif ums_realtek > usb_storage uas thermal thermal_sys ahci libahci libata uhci_hcd scsi_mod > ehci_hcd r8169 mii usbcore usb_common > Oct 4 19:53:04 ruth kernel: [ 2892.184198] Pid: 4117, comm: kworker/u:3 Not > tainted 3.6.0 #14 > Oct 4 19:53:04 ruth kernel: [ 2892.184199] Call Trace: > Oct 4 19:53:04 ruth kernel: [ 2892.184207] [] ? > warn_slowpath_common+0x78/0x8c > Oct 4 19:53:04 ruth kernel: [ 2892.184213] [] ? > async_schedule+0xc/0xc > Oct 4 19:53:04 ruth kernel: [ 2892.184216] [] ? > warn_slowpath_fmt+0x45/0x4a > Oct 4 19:53:04 ruth kernel: [ 2892.184221] [] ? > async_schedule+0xc/0xc > Oct 4 19:53:04 ruth kernel: [ 2892.184238] [] ? > intel_crtc_disable+0x52/0x86 [i915] > Oct 4 19:53:04 ruth kernel: [ 2892.184245] [] ? > drm_helper_disable_unused_functions+0xd0/0xfb [drm_kms_helper] > Oct 4 19:53:04 ruth kernel: [ 2892.184250] [] ? > drm_helper_resume_force_mode+0xf1/0xff [drm_kms_helper] > Oct 4 19:53:04 ruth kernel: [ 2892.184254] [] ? > async_schedule+0xc/0xc > Oct 4 19:53:04 ruth kernel: [ 2892.184264] [] ? > i915_drm_thaw+0xd4/0x10b [i915] > Oct 4 19:53:04 ruth kernel: [ 2892.184274] [] ? > i915_resume+0x3b/0x50 [i915] > Oct 4 19:53:04 ruth kernel: [ 2892.184279] [] ? > pci_legacy_resume+0x39/0x39 > Oct 4 19:53:04 ruth kernel: [ 2892.184284] [] ? > dpm_run_callback.isra.5+0x26/0x54 > Oct 4 19:53:04 ruth kernel: [ 2892.184289] [] ? > device_resume+0xd0/0x110 > Oct 4 19:53:04 ruth kernel: [ 2892.184292] [] ? > async_resume+0x14/0x38 > Oct 4 19:53:04 ruth kernel: [ 2892.184296] [] ? > async_run_entry_fn+0x9d/0x175 > Oct 4 19:53:04 ruth kernel: [ 2892.184300] [] ? > process_one_work+0x184/0x2a3 > Oct 4 19:53:04 ruth kernel: [ 2892.184304] [] ? > worker_thread+0x1fe/0x29f > Oct 4 19:53:04 ruth kernel: [ 2892.184307] [] ? > manage_workers+0x223/0x223 > Oct 4 19:53:04 ruth kernel: [ 2892.184310] [] ? > manage_workers+0x223/0x223 > Oct 4 19:53:04 ruth kernel: [ 2892.184315] [] ? > kthread+0x67/0x6f > Oct 4 19:53:04 ruth kernel: [ 2892.184319] [] ? > kernel_thread_helper+0x4/0x10 > Oct 4 19:53:04 ruth kernel: [ 2892.184324] [] ? > kthread_freezable_should_stop+0x3c/0x3c > Oct 4 19:53:04 ruth kernel: [ 2892.184327] [] ? > gs_change+0xb/0xb > Oct 4 19:53:04 ruth kernel: [ 2892.184328] ---[ end trace 4e63ed9ed8ebc493 > ]--- > >Hardware is a Lenovo Thinkpad Edge13, lspci says: > > 00:02.0 VGA compatible controller: Intel Corporation Mobile 4 Series Chipset > Integrated Graphics Controller (rev 07) > > aka > > 00:02.0 0300: 8086:2a42 (rev 07) > >Let me know if you need more information. I can probably manage a > bisect if necessary -- it's easily repeatable as a bug. Before you do a bisect, can you try the drm-intel-fixes branch from http://cgit.freedesktop.org/~danvet/drm-intel or a bit more risky, latest upstream git? That contains a completely rewritten modeset code, which migh
Re: [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer elapsed... GPU hung
On Tue, Oct 23, 2012 at 10:06:52AM -0700, Justin P. Mattock wrote: > This is happening both with MAINLINE and NEXT. > > basically system is running fine, then under load system becomes > really sluggish and unresponsive. I was able to get dmesg of the > error..: > > [ 7745.007008] ath9k :05:00.0 wlan0: disabling VHT as WMM/QoS is > not supported by the AP > [ 7745.007736] wlan0: associate with 68:7f:74:b8:05:82 (try 1/3) > [ 7745.011456] wlan0: RX AssocResp from 68:7f:74:b8:05:82 > (capab=0x411 status=0 aid=5) > [ 7745.011529] wlan0: associated > [ 8120.812482] [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer > elapsed... GPU hung > [ 8120.812642] [drm] capturing error event; look for more > information in /debug/dri/0/i915_error_state > [ 8122.328682] [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer > elapsed... GPU hung > [ 8122.328845] [drm:i915_reset] *ERROR* GPU hanging too fast, > declaring wedged! > [ 8122.328850] [drm:i915_reset] *ERROR* Failed to reset chip. > > full log is here: http://fpaste.org/7xH8/ > > as for good kernels from what I remember 3.6.0-rc1. I can try a > bisect on this once I get the time. or if anybody has a patch I can > test. Can you please rehand your machine, and then grab the i915_error_state from debugfs? That contains the gpu hang dump we need to diagnose things. And the bisect would obviously be awesome. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer elapsed... GPU hung
On Thu, Oct 25, 2012 at 7:22 AM, Justin P. Mattock wrote: > > here is a link to the file..: intel_error_decode > http://www.filefactory.com/file/22bypyjhs4mx I haven't figured out how to access this thing. Can you please file a bug report on bugs.freedesktop.org and attach it there? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/i915/dp: allow configuring eDP panel fitting scaling mode
On Thu, Oct 25, 2012 at 01:57:47PM -0400, Yuly Novikov wrote: > LVDS allowed changing panel fitting scaling mode, while eDP didn't. > Copied relevant code from LVDS to eDP. > This also changes default mode on eDP to ascpect ratio preserving scaling. > > Signed-off-by: Yuly Novikov Jani from our team is working on unifying a bunch of things between lvds and eDP, some of them already merged into drm-intel-next-queued branch. Jani, can you please take a look? Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 33 ++--- > drivers/gpu/drm/i915/intel_drv.h |1 + > 2 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 368ed8e..a65546e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -685,7 +685,7 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, > > if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) { > intel_fixed_panel_mode(intel_dp->panel_fixed_mode, > adjusted_mode); > - intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN, > + intel_pch_panel_fitting(dev, intel_dp->fitting_mode, > mode, adjusted_mode); > } > > @@ -2358,6 +2358,22 @@ intel_dp_set_property(struct drm_connector *connector, > goto done; > } > > + if (is_edp(intel_dp) && > + property == connector->dev->mode_config.scaling_mode_property) { > + if (val == DRM_MODE_SCALE_NONE) { > + DRM_DEBUG_KMS("no scaling not supported\n"); > + return -EINVAL; > + } > + > + if (intel_dp->fitting_mode == val) { > + /* the eDP scaling property is not changed */ > + return 0; > + } > + intel_dp->fitting_mode = val; > + > + goto done; > + } > + > return -EINVAL; > > done: > @@ -2469,10 +2485,21 @@ bool intel_dpd_is_edp(struct drm_device *dev) > } > > static void > -intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector > *connector) > +intel_dp_add_properties(struct drm_device *dev, > + struct intel_dp *intel_dp, > + struct drm_connector *connector) > { > intel_attach_force_audio_property(connector); > intel_attach_broadcast_rgb_property(connector); > + > + if (is_edp(intel_dp)) { > + drm_mode_create_scaling_mode_property(dev); > + drm_connector_attach_property( > + connector, > + dev->mode_config.scaling_mode_property, > + DRM_MODE_SCALE_ASPECT); > + intel_dp->fitting_mode = DRM_MODE_SCALE_ASPECT; > + } > } > > void > @@ -2665,7 +2692,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, > enum port port) > intel_panel_setup_backlight(dev); > } > > - intel_dp_add_properties(intel_dp, connector); > + intel_dp_add_properties(dev, intel_dp, connector); > > /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written >* 0xd. Failure to do so will result in spurious interrupts being > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index fe71425..da50cd4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -358,6 +358,7 @@ struct intel_dp { > int backlight_on_delay; > int backlight_off_delay; > struct drm_display_mode *panel_fixed_mode; /* for eDP */ > + int fitting_mode; /* for eDP */ > struct delayed_work panel_vdd_work; > bool want_panel_vdd; > struct edid *edid; /* cached EDID for eDP */ > -- > 1.7.7.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer elapsed... GPU hung
On Fri, Oct 26, 2012 at 6:43 AM, Justin P. Mattock wrote: >> >> No worries, it is another ILK hang similar to the ones reported earlier >> - it just seems the ring stops advancing. Hopefully it is a missing w/a >> from http://cgit.freedesktop.org/~danvet/drm/log/?h=ilk-wa-pile >> -Chris >> > > well if this means building libdrm etc.. then thats not a problem, more time > consuming if anything. perhaps an *.rpm that I can test to see? It's not libdrm, the above is just a kernel git tree with a bunch of ironlake workarounds. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[pull] drm-intel-next for 3.8
Hi Dave, Quite a pile since this is 4 weeks worth of patches: - tons of hsw dp prep patches form Paulo - round scheduled work items and timers to nearest second (Chris) - some hw workarounds (Jesse&Damien) - vlv dp support and related fixups (Vijay et al.) - basic haswell dp support, not yet wired up for external ports (Paulo) - edp support (Paulo) - tons of refactorings to prepare for the above (Paulo) - panel rework, unifiying code between lvds and edp panels (Jani) - panel fitter scaling modes (Jani + Yuly Novikov) - panel power improvements, should now work without the BIOS setting it up - extracting some dp helpers from radeon/i915 and move them to drm_dp_helper.c - randome pile of workarounds (Damien, Ben, ...) - some cleanups for the register restore code for suspend/resume - secure batchbuffer support, should enable tear-free blits on gen6+ (Chris) - random smaller fixlets and cleanups. For Haswell display support, this is not yet everything, big things still missing are: - hdmi/dp encoder unification, otherwise we can't enable non-eDP outputs - vga fixes (which essentially required forking all the fdi/pch code) Both are already in -next-queued, so for the next pull I plan to move Haswell out of experimental support. Note that this also contains a -rc2 backmerge, which I've botched up slightly:( Luckily Jani caught me and fixed things up, his patch is included on top of what QA beat on. For drm core stuff I have two series outstanding: - kerneldoc/DocBook patches demanded by Lauren Pinchart. Note that the last patch in that series depends upon the dp helper refactoring included in here. - relaunched hpd rework, requested&reviewed by Alex Deucher. Can you please look into slurping these into drm-next, too? Yours, Daniel The following changes since commit 6f0c0580b70c89094b3422ba81118c7b959c7556: Linux 3.7-rc2 (2012-10-20 12:11:32 -0700) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel for-airlied for you to fetch changes up to c8241969b44438c9335b59d375b627214bc36483: drm/i915: pass adjusted_mode to intel_choose_pipe_bpp_dither(), again (2012-11-02 09:57:28 +0100) Adam Jackson (6): drm: Export drm_probe_ddc() drm/dp: Update DPCD defines drm/i915/dp: Fetch downstream port info if needed during DPCD fetch drm/i915/dp: Be smarter about connection sense for branch devices drm/dp: Document DP spec versions for various DPCD registers drm/dp: Make sink count DP 1.2 aware Ben Widawsky (3): drm/i915: Extract PCU communication drm/i915: Workaround to bump rc6 voltage to 450 drm/i915: Add rc6vids to debugfs Chris Wilson (5): drm/i915: Align the hangcheck wakeup to the nearest second drm/i915: Align the retire_requests worker to the nearest second drm/i915: Allow DRM_ROOT_ONLY|DRM_MASTER to submit privileged batchbuffers drm/i915: Document the multi-threaded FORCEWAKE bits drm/i915: Clear FORCEWAKE when taking over from BIOS Damien Lespiau (9): drm/i915: Remove the disabling of VHR unit clock gating for HSW drm/i915: Document that we are implementing WaDisableBackToBackFlipFix drm/i915: Remove the WaDisableBackToBackFlipFix w/a for Haswell drm/i915: Fix the SCC/SSC typo in the SPLL bits definition drm/i915: Consolidate ILK_DSPCLK_GATE and PCH_DSPCLK_GATE drm/i915: Program DSPCLK_GATE_D only once on Ironlake drm/i915: Don't program DSPCLK_GATE_D twice on IVB and VLV drm/i915: Don't try to use SPR_SCALE when we don't have a sprite scaler drm/i915: VLV does not have a sprite scaler Daniel Vetter (24): drm/i915: s/DRM_IRQ_ARGS/int irq, void *arg drm/i915: move hpd handling to (ibx|cpt)_irq_handler drm/i915: don't save/restore DP regs for kms drm/i915: don't save/restore irq regs for kms drm/i915: don't save/restore HWS_PGA reg for kms drm/i915/crt: don't set HOTPLUG bits on !PCH drm/i915/crt: explicitly set up HOTPLUG_BITS on resume drm/i915: don't save/restor ADPA for kms drm/i915: unconditionally use mt forcewake on hsw/ivb Merge tag 'v3.7-rc2' into drm-intel-next-queued drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c drm: dp helper: extract drm_dp_channel_eq_ok drm: dp helper: extract drm_dp_clock_recovery_ok drm: extract helpers to compute new training values from sink request drm: extract dp link train delay functions from radeon drm/i915: use the new dp train delay helpers drm: extract dp link bw helpers drm: extract drm_dp_max_lane_count helper drm/i915/dp: actually nack test request drm/i915: make edp panel power sequence setup more robust drm/i915: enable/disable backlight for eDP drm/i915/eDP: compute the panel power clock divisor fro
Re: Linux 3.7-rc3
On Fri, Nov 2, 2012 at 9:25 PM, Linus Torvalds wrote: > On Mon, Oct 29, 2012 at 5:10 PM, Rafael J. Wysocki wrote: >> >> Unfortunately, s2disk is broken with this one and previous -rc. In the >> majority of cases it just hangs the machine during hibernation, although >> sometimes it returns to user space reporting freezing problems, suspicions >> RCU usage and similar stuff, pretty much without any useful debug >> information. > > Ugh. I was hoping that we'd get more reports on this, right now > there's not enough information to even make a guess about what's up. > > I also note that no actual i915 people were cc'd, despite your > graphics driver suspect. Adding Daniel and Dave (one of the millions) > to the cc since they probably do have lkml but probably don't react > unless something gets pointed out. > > Daniel/Dave, do you have any reports of hibernation failing (or VT > switching problems) with i915 after 3.6? Not that I've heard of. We did though rewrite the entire modeset sequence driving code in i915 for 3.7, removing massive amounts of hacks and stupid "let's disable/enable things harder" code simply because the old code couldn't keep track of the hw state well enough. So I wouldn't be surprised at all if that unearthed a hidden bug somewhere, but we've also added ridiculous amounts of self-checks. And thus far they caught all issues due to that rework (and some ancient bugs on top) when either the i915 driver or the bios did something nefarious ... I've rechecked the git log and otherwise there's nothing else which pops up that would fit VT-switching/hibernate going bad but the modeset rework. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux 3.7-rc3
On Fri, Nov 2, 2012 at 10:40 PM, Rafael J. Wysocki wrote: > Can you point me to a range of commits worth checking? 8c3f929b6147e142..57df2ae9df6e335a98969cac is the core rework (plus a bit related follow-up) without any other merges interfering. Might oops if you open/close the lid without a suspend/resume cycle (since that's only fixed later on), should otherwise work. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REGRESSION] i915: failure to interoperate with HP ZR30w using an X230
On Fri, Nov 02, 2012 at 08:58:31PM -0400, Theodore Ts'o wrote: > Ping? > > On Tue, Oct 30, 2012 at 04:32:21PM -0400, Theodore Ts'o wrote: > > On Tue, Oct 30, 2012 at 01:57:27PM +0200, Jani Nikula wrote: > > > > [1] drm-intel-next-queued branch at > > > > git://people.freedesktop.org/~danvet/drm-intel > > > > > > Hmm, actually not. Either drm-intel-fixes branch, or Linus' master. > > > > Confirmed, the drm-intel-fixes branch from Daniel's tree > > (3.7.0-rc2-00031-g1623392) works fine for me. > > > > Do you know which commit(s) are likely to have fixed the problem, so we > > can cherry pick the appropriate fix(es) to the 3.6.x tree? Well, we know for sure that fdi link training is broken - it doesn't match at all what the spec says we should do. I've been working on this lately, since in quite a few circumstances the link train fails without the relevent bits indicating so. While testing I've also noticed that this entire thing is highly timing dependent, e.g. denpending upon which desktop is running and which tool I use to change the configuration it fails or succeeds. So I have no suggestions for what could help your system and what should get backported, since the current code is still broken. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REGRESSION] [KMS] [INTEL] Wrong resolution in console and XWindow
On Wed, Jul 25, 2012 at 01:55:59PM +0200, Daniel Vetter wrote: > On Wed, Jul 25, 2012 at 12:57 PM, Maciej Rutecki > wrote: > > On środa, 25 lipca 2012 o 11:29:28 Daniel Vetter wrote: > >> On Wed, Jul 25, 2012 at 10:54:25AM +0200, Maciej Rutecki wrote: > >> > On środa, 25 lipca 2012 o 10:29:26 Daniel Vetter wrote: > >> > > On Wed, Jul 25, 2012 at 10:20:47AM +0200, Maciej Rutecki wrote: > >> > > > Last known good: 3.4.4 > >> > > > First bad: 3.5.0 > >> > > > > >> > > > When booting 3.5.0 resolution (in console, and after in KDE) is set > >> > > > to 1024x768 (60Hz). In 3.4.4 was correct: 1440x900 (60Hz). > >> > > > >> > > Can you please attach the output of xrandr --verbose for both kernels? > >> > > Also, please boot with drm.debug=0xe added to your kernel cmdline and > >> > > grab the dmesg (again for both kernels). > >> > > >> > Thanks for the ansfer. > >> > > >> > Here xrandr and dmesg outputs for 3.4.4 and 3.5.0 > >> > > >> > http://mrutecki.pl/download/kernel/3.5/swinka/debug/ > >> > >> Can you please test this quick hack: > >> > >> > >> diff --git a/drivers/gpu/drm/i915/intel_i2c.c > >> b/drivers/gpu/drm/i915/intel_i2c.c index 1991a44..abe1611 100644 > >> --- a/drivers/gpu/drm/i915/intel_i2c.c > >> +++ b/drivers/gpu/drm/i915/intel_i2c.c > >> @@ -405,7 +405,7 @@ clear_err: > >>* timing out seems to happen when there _is_ a ddc chip present, but > >>* it's slow responding and only answers on the 2nd retry. > >>*/ > >> - ret = -ENXIO; > >> + ret = 0; > >> if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0, > >>10)) { > >> DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n", > >> > >> > >> Thanks, Daniel > > > > Still the same. > > Hm, can you attach the dmesg again (with drm.debug=0xe)? If I haven't > botched up something, we should now retry at least the ddc transfer > ... Also, another little snippet for you to test. Fyi I'll be on vacation next week, so final patch (this one here should really work) might take a notch longer. Yours, Daniel -- diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index bc5e2c9..85eca1c 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -338,6 +338,7 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector) BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG); i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin); + intel_gmbus_force_bit(i2c, true); edid = drm_get_edid(connector, i2c); if (edid) { @@ -546,12 +547,14 @@ static int intel_crt_get_modes(struct drm_connector *connector) struct i2c_adapter *i2c; i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin); + intel_gmbus_force_bit(i2c, true); ret = intel_ddc_get_modes(connector, i2c); if (ret || !IS_G4X(dev)) return ret; /* Try to probe digital port for output in DVI-I -> VGA mode. */ i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB); + intel_gmbus_force_bit(i2c, true); return intel_ddc_get_modes(connector, i2c); } -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] console: implement lockdep support for console_lock
Dave Airlie recently discovered a locking bug in the fbcon layer, where a timer_del_sync (for the blinking cursor) deadlocks with the timer itself, since both (want to) hold the console_lock: https://lkml.org/lkml/2012/8/21/36 Unfortunately the console_lock isn't a plain mutex and hence has no lockdep support. Which resulted in a few days wasted of tracking down this bug (complicated by the fact that printk doesn't show anything when the console is locked) instead of noticing the bug much earlier with the lockdep splat. Hence I've figured I need to fix that for the next deadlock involving console_lock - and with kms/drm growing ever more complex locking that'll eventually happen. Now the console_lock has rather funky semantics, so after a quick irc discussion with Thomas Gleixner and Dave Airlie I've quickly ditched the original idead of switching to a real mutex (since it won't work) and instead opted to annotate the console_lock with lockdep information manually. There are a few special cases: - The console_lock state is protected by the console_sem, and usually grabbed/dropped at _lock/_unlock time. But the suspend/resume code drops the semaphore without dropping the console_lock (see suspend_console/resume_console). But since the same thread that did the suspend will do the resume, we don't need to fix up anything. - In the printk code there's a special trylock, only used to kick off the logbuffer printk'ing in console_unlock. But all that happens while lockdep is disable (since printk does a few other evil tricks). So no issue there, either. - The console_lock can also be acquired form irq context (but only with a trylock). lockdep already handles that. This all leaves us with annotating the normal console_lock, _unlock and _trylock functions. And yes, it works - simply unloading a drm kms driver resulted in lockdep complaining about the deadlock in fbcon_deinit: == [ INFO: possible circular locking dependency detected ] 3.6.0-rc2+ #552 Not tainted --- kms-reload/3577 is trying to acquire lock: ((&info->queue)){+.+...}, at: [] wait_on_work+0x0/0xa7 but task is already holding lock: (console_lock){+.+.+.}, at: [] bind_con_driver+0x38/0x263 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (console_lock){+.+.+.}: [] lock_acquire+0x95/0x105 [] console_lock+0x59/0x5b [] fb_flashcursor+0x2e/0x12c [] process_one_work+0x1d9/0x3b4 [] worker_thread+0x1a7/0x24b [] kthread+0x7f/0x87 [] kernel_thread_helper+0x4/0x10 -> #0 ((&info->queue)){+.+...}: [] __lock_acquire+0x999/0xcf6 [] lock_acquire+0x95/0x105 [] wait_on_work+0x3b/0xa7 [] __cancel_work_timer+0xbf/0x102 [] cancel_work_sync+0xb/0xd [] fbcon_deinit+0x11c/0x1dc [] bind_con_driver+0x145/0x263 [] unbind_con_driver+0x14f/0x195 [] store_bind+0x1ad/0x1c1 [] dev_attr_store+0x13/0x1f [] sysfs_write_file+0xe9/0x121 [] vfs_write+0x9b/0xfd [] sys_write+0x3e/0x6b [] system_call_fastpath+0x16/0x1b other info that might help us debug this: Possible unsafe locking scenario: CPU0CPU1 lock(console_lock); lock((&info->queue)); lock(console_lock); lock((&info->queue)); *** DEADLOCK *** v2: Mark the lockdep_map static, noticed by Jani Nikula. Cc: Dave Airlie Cc: Thomas Gleixner Cc: Alan Cox Cc: Peter Zijlstra Signed-off-by: Daniel Vetter --- kernel/printk.c |9 + 1 file changed, 9 insertions(+) diff --git a/kernel/printk.c b/kernel/printk.c index ed9af6a..e5c6dba 100644 --- a/kernel/printk.c +++ b/kernel/printk.c @@ -87,6 +87,12 @@ static DEFINE_SEMAPHORE(console_sem); struct console *console_drivers; EXPORT_SYMBOL_GPL(console_drivers); +#ifdef CONFIG_LOCKDEP +static struct lockdep_map console_lock_dep_map = { + .name = "console_lock" +}; +#endif + /* * This is used for debugging the mess that is the VT code by * keeping track if we have the console semaphore held. It's @@ -1916,6 +1922,7 @@ void console_lock(void) return; console_locked = 1; console_may_schedule = 1; + mutex_acquire(&console_lock_dep_map, 0, 0, _RET_IP_); } EXPORT_SYMBOL(console_lock); @@ -1937,6 +1944,7 @@ int console_trylock(void) } console_locked = 1; console_may_schedule = 0; + mutex_acquire(&console_lock_dep_map, 0, 1, _RET_IP_); return 1; } EXPORT_SYMBOL(console_trylock); @@ -2097,6 +2105,7 @@ skip: local_irq_restore(flags); } console_locked = 0; + mutex_release(&console_lock
Re: linux-next: build failure after merge of the drm tree
On Mon, Sep 24, 2012 at 01:18:34PM +1000, Stephen Rothwell wrote: > Hi Dave, > > After merging the drm tree, today's linux-next build (x86_64 allmodconfig) > failed like this: > > drivers/gpu/drm/i915/intel_hdmi.c: In function 'intel_enable_hdmi': > drivers/gpu/drm/i915/intel_hdmi.c:633:31: error: 'mode' undeclared (first use > in this function) > > Caused by commit b98b60167279 ("drm/i915: HDMI - Clear Audio Enable bit > for Hot Plug") from Linus' tree interacting with commit 5ab432ef4997 > ("drm/i915/hdmi: convert to encoder->disable/enable") from the drm tree. > > I added the following merge fix patch (which may be wrong, of course): > > From: Stephen Rothwell > Date: Mon, 24 Sep 2012 13:14:40 +1000 > Subject: [PATCH] drm/i915: HDMI: update for API change > > Signed-off-by: Stephen Rothwell Looks good. I've known about this issue and tried to improve matters by applying the patch to both trees (to at least force a conflict), but not even with that patch applied in my drm-intel-next queue I get a proper conflict. I guess I need to do a proper backmerge. Fixup patch looks good for now. Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_hdmi.c |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 2ec9c76..d39ed58 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -630,7 +630,7 @@ static void intel_enable_hdmi(struct intel_encoder > *encoder) > u32 temp; > u32 enable_bits = SDVO_ENABLE; > > - if (intel_hdmi->has_audio || mode != DRM_MODE_DPMS_ON) > + if (intel_hdmi->has_audio) > enable_bits |= SDVO_AUDIO_ENABLE; > > temp = I915_READ(intel_hdmi->sdvox_reg); > @@ -676,8 +676,7 @@ static void intel_disable_hdmi(struct intel_encoder > *encoder) > u32 temp; > u32 enable_bits = SDVO_ENABLE; > > - if (intel_hdmi->has_audio) > - enable_bits |= SDVO_AUDIO_ENABLE; > + enable_bits |= SDVO_AUDIO_ENABLE; > > temp = I915_READ(intel_hdmi->sdvox_reg); > > -- > 1.7.10.280.gaa39 > > -- > Cheers, > Stephen Rothwells...@canb.auug.org.au -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [PATCH] console: implement lockdep support for console_lock
On Sat, Sep 22, 2012 at 01:06:29PM -0700, Greg KH wrote: > On Sat, Sep 22, 2012 at 07:52:11PM +0200, Daniel Vetter wrote: > > Dave Airlie recently discovered a locking bug in the fbcon layer, > > where a timer_del_sync (for the blinking cursor) deadlocks with the > > timer itself, since both (want to) hold the console_lock: > > > > https://lkml.org/lkml/2012/8/21/36 > > > > Unfortunately the console_lock isn't a plain mutex and hence has no > > lockdep support. Which resulted in a few days wasted of tracking down > > this bug (complicated by the fact that printk doesn't show anything > > when the console is locked) instead of noticing the bug much earlier > > with the lockdep splat. > > > > Hence I've figured I need to fix that for the next deadlock involving > > console_lock - and with kms/drm growing ever more complex locking > > that'll eventually happen. > > > > Now the console_lock has rather funky semantics, so after a quick irc > > discussion with Thomas Gleixner and Dave Airlie I've quickly ditched > > the original idead of switching to a real mutex (since it won't work) > > and instead opted to annotate the console_lock with lockdep > > information manually. > > > > There are a few special cases: > > - The console_lock state is protected by the console_sem, and usually > > grabbed/dropped at _lock/_unlock time. But the suspend/resume code > > drops the semaphore without dropping the console_lock (see > > suspend_console/resume_console). But since the same thread that did > > the suspend will do the resume, we don't need to fix up anything. > > > > - In the printk code there's a special trylock, only used to kick off > > the logbuffer printk'ing in console_unlock. But all that happens > > while lockdep is disable (since printk does a few other evil > > tricks). So no issue there, either. > > > > - The console_lock can also be acquired form irq context (but only > > with a trylock). lockdep already handles that. > > > > This all leaves us with annotating the normal console_lock, _unlock > > and _trylock functions. > > > > And yes, it works - simply unloading a drm kms driver resulted in > > lockdep complaining about the deadlock in fbcon_deinit: > > > > == > > [ INFO: possible circular locking dependency detected ] > > 3.6.0-rc2+ #552 Not tainted > > --- > > kms-reload/3577 is trying to acquire lock: > > ((&info->queue)){+.+...}, at: [] wait_on_work+0x0/0xa7 > > > > but task is already holding lock: > > (console_lock){+.+.+.}, at: [] bind_con_driver+0x38/0x263 > > > > which lock already depends on the new lock. > > > > the existing dependency chain (in reverse order) is: > > > > -> #1 (console_lock){+.+.+.}: > >[] lock_acquire+0x95/0x105 > >[] console_lock+0x59/0x5b > >[] fb_flashcursor+0x2e/0x12c > >[] process_one_work+0x1d9/0x3b4 > >[] worker_thread+0x1a7/0x24b > >[] kthread+0x7f/0x87 > >[] kernel_thread_helper+0x4/0x10 > > > > -> #0 ((&info->queue)){+.+...}: > >[] __lock_acquire+0x999/0xcf6 > >[] lock_acquire+0x95/0x105 > >[] wait_on_work+0x3b/0xa7 > >[] __cancel_work_timer+0xbf/0x102 > >[] cancel_work_sync+0xb/0xd > >[] fbcon_deinit+0x11c/0x1dc > >[] bind_con_driver+0x145/0x263 > >[] unbind_con_driver+0x14f/0x195 > >[] store_bind+0x1ad/0x1c1 > >[] dev_attr_store+0x13/0x1f > >[] sysfs_write_file+0xe9/0x121 > >[] vfs_write+0x9b/0xfd > >[] sys_write+0x3e/0x6b > >[] system_call_fastpath+0x16/0x1b > > > > other info that might help us debug this: > > > > Possible unsafe locking scenario: > > > >CPU0CPU1 > > > > lock(console_lock); > >lock((&info->queue)); > >lock(console_lock); > > lock((&info->queue)); > > > > *** DEADLOCK *** > > > > v2: Mark the lockdep_map static, noticed by Jani Nikula. > > > > Cc: Dave Airlie > > Cc: Thomas Gleixner > > Cc: Alan Cox > > Cc: Peter Zijlstra > > Signed-off-by: Daniel Vetter > > --- > > kernel/printk.c |9 + &g
Re: [PATCH 2/2] [RESEND] console: implement lockdep support for console_lock
On Mon, Sep 24, 2012 at 2:24 PM, Peter Zijlstra wrote: > On Mon, 2012-09-24 at 14:17 +0200, Peter Zijlstra wrote: >> On Tue, 2012-09-18 at 01:03 +0200, Daniel Vetter wrote: >> > - In the printk code there's a special trylock, only used to kick off >> > the logbuffer printk'ing in console_unlock. But all that happens >> > while lockdep is disable (since printk does a few other evil >> > tricks). So no issue there, either. >> >> Not particularly evil, just plain broken. >> >> See this series: >> >> http://marc.info/?l=linux-kernel&m=132446644123326&w=2 >> >> In particular see patches 1-4. The problem with lockdep is that lockdep >> uses printk and printk's recursion stuff is utterly broken. >> >> Console drivers being on crack simply doesn't help :-) Note that I would >> never recommend drm/ksm to anybody who really cares about their console >> output. I've read through the patches and I'm hoping you don't volunteer me to pick these up ... ;-) But there doesn't seem to be anything that would get worse through this lockdep annotation patch, right? > Also, don't cross-post with a subscribe only list. Sorry, forgot about this, I'll try to fix this - we really need an intel-gfx that's open to people not subscribed to it. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[pull] last drm-intel-next pull for 3.7
Hi Dave, Last pile of stuff for 3.7, essentially just a bunch of bigger fixes and a few less intrusive features: - cpu freq interface in sysfs from Ben - cpu edp fixes and some related cleanups - write-combining ptes for pre-gen6 (Chris) - basic CADL support (Peter Wu), this fixes quite a few issues with backlights ... - rework of the gem backing pages handling (preps for stolen mem handling) from Chris - some more cleanup-fallout from the modeset-rework On top of that I've done a backmerge of -rc7(since the conflicts got too messy and I've pushed out broken merged trees too often). I've also included 3 fixes on top of what QA beat on: - Fix for a infoframe handling regression in 3.5 - infoframe blows up too often and 3.6 is pretty much done, so I'd like to merge that through -next and the stable process and give it more exposure before it lands in a stable tree. - ioctl cosmetics^Wspelling fix in the structs (userspace won't be affected, since all existing userspace uses private copies of the ioctl struct definitions, and the struct layout itself is abi compatible). - Bugfix for a regression introduced in this pull's testing cycle. Besides the regression in the ilk debugfs code QA reported new DP issues, but afact all old stuff, just new variations of existing bugs. I hope we can pour quite some manpower into our DP code from Intel's sides for the next cycle. Maybe even extract some shared logic, since it sounds like we'll eventually need quircks like for edid in dp-land :( Cheers, Daniel PS: Note that due to the backmerge I've manually frobbed the shortlog again to exclude anything merged through Linus' tree. The following changes since commit 3b7a89fce3e3dc96b549d6d829387b4439044d0d: drm/i915: fix OOPS in lid_notify (2012-09-18 00:59:37 +0200) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel for-airlied for you to fetch changes up to f531dcb23f9a5c6ad77e451459df965dc9a0c0c8: drm/i915: Wrap external callers to IPS state with appropriate locks (2012-09-26 09:24:54 +0200) Ben Widawsky (10): drm/i915: placeholder getparam drm/i915: variable renames drm/i915: #define gpu freq multipler drm/i915: Add current/max/min GPU freq to sysfs drm/i915: POSTING_READ the new rps value drm/i915: Error checks in gen6_set_rps drm/i915: Add setters for min/max frequency drm/i915: Show render P state thresholds in sysfs drm/i915: Fix !CONFIG_PM sysfs for real this time drm/i915: s/cacheing/caching/ Chris Wilson (11): drm/i915: Introduce drm_i915_gem_object_ops drm/i915: Pin backing pages whilst exporting through a dmabuf vmap drm/i915: Pin backing pages for pwrite drm/i915: Pin backing pages for pread drm/i915: Replace the array of pages with a scatterlist drm/i915: Drop the misleading cast to the wrong user pointer type drm/i915: Limit the ioremap of the PCI bar to the registers agp/intel: Use a write-combining map for updating PTEs drm/i915: Convert the dmabuf object to use the new i915_gem_object_ops drm/i915: Assert that the exec object lookup table is a power-of-two drm/i915: Wrap external callers to IPS state with appropriate locks Daniel Vetter (12): drm/i915: rip out early dp port write for gm45/ilk drm/i915: add encoder->pre_enable/post_disable drm/i915: clean up the cpu edp pll special case drm/i915: robustify edp_pll_on/off drm/i915: rip out dp port enabling cludges^Wchecks drm/i915: disable the cpu edp port after the cpu pipe drm/i915: rip out edp special case from dp_link_down drm/i915: rip out intel_disable_pch_ports drm/i915: don't disable fdi links harder in ilk_crtc_enable drm/i915: don't call dpms funcs after set_mode drm/i915: update dpms property in set_mode Merge tag 'v3.6-rc7' into drm-intel-next-queued Lekensteyn (1): i915: initialize CADL in opregion Paulo Zanoni (5): drm/i915: extract ironlake_set_pipeconf form ironlake_crtc_mode_set drm/i915: simplify setting DSPCNTR inside ironlake_crtc_mode_set drm/i915: extract compute_clocks from ironlake_crtc_mode_set drm/i915: BUG() on unexpected HDMI register drm/i915: make sure we write all the DIP data bytes Wang Xingchao (1): drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [drm:i915_hangcheck_hung] *ERROR* Hangcheck timer elapsed... GPU hung
On Fri, Oct 26, 2012 at 10:57 PM, Justin P. Mattock wrote: > > :~/drm> git clone git://people.freedesktop.org/~danvet/drm > Cloning into 'drm'... > remote: Counting objects: 2728390, done. > remote: Compressing objects: 100% (418606/418606), done. > remote: Total 2728390 (delta 2293727), reused 2717443 (delta 2282880) > Receiving objects: 100% (2728390/2728390), 637.95 MiB | 599 KiB/s, done. > Resolving deltas: 100% (2293727/2293727), done. > warning: remote HEAD refers to nonexistent ref, unable to checkout. > > > so now I have to go on a witch hunt for 600MB's in my system. $ git checkout origin/ilk-wa-pile ... and you have the right branch checked out. No need for pitchforks and witch hunts ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] Updated stolen mem patches
On Fri, Jul 26, 2013 at 01:32:50PM -0700, Jesse Barnes wrote: > These address the comments I've received so far, but omit the new E820 > type for this mem. > > Chris's patches could go on top if desired; they add a new type and > resource reservation function for looking up regions by name. That > allows us to remove some duplicate code in the driver for finding stolen > space. > > But I think these two are ready as-is. How should we merge them? Just > through the i915 tree since the first one touches our headers? that and > there probably won't be conflicts on the early-quirks file; that's not > touch too often... Just noticed that these patches aren't in -next yet - somehow I've thought they'd go in in through the x86 tree. Ingo et al: Should I merge these through drm-intel-next or will you pick them up? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] imx-drm: Fix probe failure
On Thu, Aug 29, 2013 at 10:59:22AM -0300, Fabio Estevam wrote: > Since commit b5dc0d10 (drm/imx: kill firstopen callback) the following probe > failure is seen: > > [drm] Supports vblank timestamp caching Rev 1 (10.10.2010). > [drm] No driver support for vblank timestamp query. > [drm] Initialized imx-drm 1.0.0 20120507 on minor 0 > imx-ldb ldb.10: adding encoder failed with -16 > imx-ldb: probe of ldb.10 failed with error -16 > imx-ipuv3 240.ipu: IPUv3H probed > imx-ipuv3 280.ipu: IPUv3H probed > imx-ipuv3-crtc imx-ipuv3-crtc.0: adding crtc failed with -16. > imx-ipuv3-crtc: probe of imx-ipuv3-crtc.0 failed with error -16 > imx-ipuv3-crtc imx-ipuv3-crtc.1: adding crtc failed with -16. > imx-ipuv3-crtc: probe of imx-ipuv3-crtc.1 failed with error -16 > imx-ipuv3-crtc imx-ipuv3-crtc.2: adding crtc failed with -16. > imx-ipuv3-crtc: probe of imx-ipuv3-crtc.2 failed with error -16 > imx-ipuv3-crtc imx-ipuv3-crtc.3: adding crtc failed with -16. > imx-ipuv3-crtc: probe of imx-ipuv3-crtc.3 failed with error -16 > > The reason for the probe failure is that now 'imxdrm->references' is > incremented > early in imx_drm_driver_load(), so the following checks in imx_drm_add_crtc() > and imx_drm_add_encoder(): > > if (imxdrm->references) { > ret = -EBUSY; > goto err_busy; > } > > ,will always fail. > > Let the drm core handle the references instead of doing this manually in > imx-drm. In imx-drm-core the open and close callbacks map to drm_open and > drm_close, which handle the references via open_count. > > After this patch, lvds panel is functional on a mx6qsabrelite board. > > Signed-off-by: Fabio Estevam Yeah, just ripping out the imxdrm->references stuff will restore imx, but of course all the funny races are still fundamentally there. So we still rely on userspace being really careful to obey the right module loading sequence and not start any drm client before that has all completed. So Reviewed-by: Daniel Vetter Greg, when you slurp this in can you pls also add the imx todo entry from the other thread? http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg43698.html Or do you want me to send in a real patch? Thanks, Daniel > --- > Changes since v1: > - Improved commit log by providing an explanation to the probe failure > - Cc'ed Dave/Daniel > > drivers/staging/imx-drm/imx-drm-core.c | 23 --- > 1 file changed, 23 deletions(-) > > diff --git a/drivers/staging/imx-drm/imx-drm-core.c > b/drivers/staging/imx-drm/imx-drm-core.c > index 47c5888..6f0d24c 100644 > --- a/drivers/staging/imx-drm/imx-drm-core.c > +++ b/drivers/staging/imx-drm/imx-drm-core.c > @@ -41,7 +41,6 @@ struct imx_drm_device { > struct list_headencoder_list; > struct list_headconnector_list; > struct mutexmutex; > - int references; > int pipes; > struct drm_fbdev_cma*fbhelper; > }; > @@ -241,8 +240,6 @@ struct drm_device *imx_drm_device_get(void) > } > } > > - imxdrm->references++; > - > return imxdrm->drm; > > unwind_crtc: > @@ -280,8 +277,6 @@ void imx_drm_device_put(void) > list_for_each_entry(enc, &imxdrm->encoder_list, list) > module_put(enc->owner); > > - imxdrm->references--; > - > mutex_unlock(&imxdrm->mutex); > } > EXPORT_SYMBOL_GPL(imx_drm_device_put); > @@ -485,11 +480,6 @@ int imx_drm_add_crtc(struct drm_crtc *crtc, > > mutex_lock(&imxdrm->mutex); > > - if (imxdrm->references) { > - ret = -EBUSY; > - goto err_busy; > - } > - > imx_drm_crtc = kzalloc(sizeof(*imx_drm_crtc), GFP_KERNEL); > if (!imx_drm_crtc) { > ret = -ENOMEM; > @@ -523,7 +513,6 @@ int imx_drm_add_crtc(struct drm_crtc *crtc, > err_register: > kfree(imx_drm_crtc); > err_alloc: > -err_busy: > mutex_unlock(&imxdrm->mutex); > return ret; > } > @@ -564,11 +553,6 @@ int imx_drm_add_encoder(struct drm_encoder *encoder, > > mutex_lock(&imxdrm->mutex); > > - if (imxdrm->references) { > - ret = -EBUSY; > - goto err_busy; > - } > - > imx_drm_encoder = kzalloc(sizeof(*imx_drm_encoder), GFP_KERNEL); > if (!imx_drm_encoder) { > ret = -ENOMEM; > @@ -595,7 +579,6 @@ int imx_drm_add_encoder(struct drm_encoder *encoder, > err_register: > kfree(imx_dr
Re: [ 005/117] drm/i915: make user mode sync polarity setting explicit
On Wed, Oct 2, 2013 at 7:30 PM, Sven Joachim wrote: > See also https://bugs.freedesktop.org/show_bug.cgi?id=65442#c16. Oh dear, somehow I've thought the -fixes pull request I've sent with the fix for this has a cc: stable, but that must have been lost somewhere. Greg, can you please queue up commit 1062b81598bc00e2f6620e6f3788f8f8df2f01e7 Author: Daniel Vetter Date: Tue Sep 10 11:44:30 2013 +0200 drm/i915/tv: clear adjusted_mode.flags for stable kernels? Right now we seem to get about one dupe per day of this report ;-) /me wondered why they kept on coming up ... Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/