Re: Your kernel commit 2f4f649a69a9eb51f6e98130e19dd90a260a4145

2012-11-27 Thread Daniel Vetter
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

2012-11-29 Thread Daniel Vetter
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

2012-11-29 Thread Daniel Vetter
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

2012-11-29 Thread Daniel Vetter
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

2013-03-17 Thread Daniel Vetter
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

2013-03-17 Thread Daniel Vetter
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

2013-03-17 Thread Daniel Vetter
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"

2013-03-17 Thread Daniel Vetter
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()

2013-03-17 Thread Daniel Vetter
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)

2013-03-18 Thread Daniel Vetter
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

2013-03-18 Thread Daniel Vetter
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

2013-03-18 Thread Daniel Vetter
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()

2013-03-18 Thread Daniel Vetter
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

2013-03-18 Thread Daniel Vetter
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)

2013-03-18 Thread Daniel Vetter
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))

2013-03-18 Thread Daniel Vetter
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

2013-03-18 Thread Daniel Vetter
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

2013-03-19 Thread Daniel Vetter
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

2013-03-19 Thread Daniel Vetter
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

2013-03-21 Thread Daniel Vetter
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()

2013-02-04 Thread Daniel Vetter
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

2013-02-04 Thread Daniel Vetter
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

2013-02-04 Thread Daniel Vetter
  * 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

2013-02-05 Thread Daniel Vetter
(&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

2013-02-05 Thread Daniel Vetter
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

2013-02-06 Thread Daniel Vetter
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

2013-02-06 Thread Daniel Vetter
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

2013-02-08 Thread Daniel Vetter
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

2013-02-09 Thread Daniel Vetter
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

2013-04-02 Thread Daniel Vetter
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

2013-04-02 Thread Daniel Vetter
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

2013-04-02 Thread Daniel Vetter
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

2013-04-02 Thread Daniel Vetter
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

2013-04-03 Thread Daniel Vetter
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

2013-04-04 Thread Daniel Vetter
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

2013-04-04 Thread Daniel Vetter
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

2013-04-04 Thread Daniel Vetter
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

2013-04-04 Thread Daniel Vetter
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

2013-01-29 Thread Daniel Vetter
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

2013-01-29 Thread Daniel Vetter
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

2013-01-29 Thread Daniel Vetter
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

2013-01-30 Thread Daniel Vetter
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.

2013-01-30 Thread Daniel Vetter
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

2013-01-30 Thread Daniel Vetter
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

2013-01-30 Thread Daniel Vetter
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

2013-01-31 Thread Daniel Vetter
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

2013-01-31 Thread Daniel Vetter
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

2013-01-31 Thread Daniel Vetter
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

2013-01-31 Thread Daniel Vetter
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)

2013-01-31 Thread Daniel Vetter
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

2013-01-31 Thread Daniel Vetter
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)

2013-01-31 Thread Daniel Vetter
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"

2013-02-01 Thread Daniel Vetter
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

2013-02-01 Thread Daniel Vetter
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

2013-01-24 Thread Daniel Vetter
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

2013-01-24 Thread Daniel Vetter
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

2013-01-25 Thread Daniel Vetter
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

2013-01-27 Thread Daniel Vetter
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.

2013-01-27 Thread Daniel Vetter
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

2013-01-27 Thread Daniel Vetter
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

2013-01-28 Thread Daniel Vetter
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

2013-01-28 Thread Daniel Vetter
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

2013-02-19 Thread Daniel Vetter
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

2013-02-20 Thread Daniel Vetter
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

2013-02-20 Thread Daniel Vetter
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

2013-03-13 Thread Daniel Vetter
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)

2013-03-13 Thread Daniel Vetter
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

2013-03-13 Thread Daniel Vetter
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

2013-03-14 Thread Daniel Vetter
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

2012-08-31 Thread Daniel Vetter
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

2012-09-01 Thread Daniel Vetter
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

2012-09-13 Thread Daniel Vetter
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

2012-09-14 Thread Daniel Vetter
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

2012-09-14 Thread Daniel Vetter
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

2012-10-02 Thread Daniel Vetter
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

2012-10-02 Thread Daniel Vetter
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

2012-10-02 Thread Daniel Vetter
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

2012-10-02 Thread Daniel Vetter
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

2012-10-03 Thread Daniel Vetter
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

2012-10-03 Thread Daniel Vetter
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

2012-10-04 Thread Daniel Vetter
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

2012-10-04 Thread Daniel Vetter
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

2012-10-23 Thread Daniel Vetter
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

2012-10-25 Thread Daniel Vetter
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

2012-10-25 Thread Daniel Vetter
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

2012-10-26 Thread Daniel Vetter
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

2012-11-02 Thread Daniel Vetter
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

2012-11-02 Thread Daniel Vetter
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

2012-11-02 Thread Daniel Vetter
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

2012-11-03 Thread Daniel Vetter
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

2012-07-26 Thread Daniel Vetter
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

2012-09-22 Thread Daniel Vetter
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

2012-09-24 Thread Daniel Vetter
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

2012-09-24 Thread Daniel Vetter
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

2012-09-24 Thread Daniel Vetter
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

2012-09-26 Thread Daniel Vetter
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

2012-10-27 Thread Daniel Vetter
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

2013-08-27 Thread Daniel Vetter
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

2013-08-29 Thread Daniel Vetter
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

2013-10-02 Thread Daniel Vetter
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/


  1   2   3   4   5   6   7   8   9   10   >