Clear Audio Enable bit to trigger unsolicated event to notify Audio
Driver part the HDMI hot plug change. The patch fixed the bug when
remove HDMI cable the bit was not cleared correctly.
In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will
be set to trigger unsolicated event to notify Alsa driver the change.
intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after
remove the hdmi cable, here's debug log:
[ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2
[ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0
so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this
cause the eld infomation and pin presence doesnot update accordingly in alsa driver side.
This patch will also trigger unsolicated event to alsa driver to notify the hot plug event:
[ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1
[ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0
Signed-off-by: Wang Xingchao <xingchao.wang@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
By providing a callback for when we need to bind the pages, and then
release them again later, we can shorten the amount of time we hold the
foreign pages mapped and pinned, and importantly the dmabuf objects then
behave as any other normal object with respect to the shrinker and
memory management.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Hopefully this makes userspace slightly less confused about us
frobbing the dpms state behind its back. Yeah, it would be better
to be more careful with not changing the dpms state, but that is
quite more invasive.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... because our current set_mode implementation doesn't bother to adjust
for the dpms state, we just forcefully update it. So stop pretending that
we're better than we are and rip out this extranous call.
Note that this totally confuses userspace, because the exposed connector
property isn't actually updated ...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because they should have been disabled when shutting down the display
pipe previously. To ensure that this is the case, add a few assserts
instead of unconditionally disabling the fdi link.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Even with the old crtc helper code we should have disabled all
encoders on that pipe by now, and with the new code this would
definitely paper over a bug. We already have the necessary checks
in place in intel_disable_transcoder, so if we accidentally leave
a pch port on, this will be caught.
Hence just rip this all out.
Note that up to the patch in this giant modeset series that removes
the LVDS special case to avoid disabling LVDS in the encoder->prepare
callback ("drm/i915/lvds: ditch ->prepare special case"), this was not
the case for all outputs.
Also note that in
commit 1b3c7a47f9
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date: Wed Nov 25 13:09:38 2009 +0800
drm/i915: Fix LVDS stability issue on Ironlake
this was already discovered independently and worked around. How I
bloody hate this entire mess of cludges piled on top of other cludges.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the future we may like to experiment with using a WC map of the GTT
portion. However, that will conflict with i915.ko mapping the entire bar
as UC in order to access the GPU registers. Instead we can shrink the
register ioremap to only map the register block.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by (IVB): Ben Widawsky <ben@bwidawsk.net>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Squashed-in follow-up fix for gen2/3 registers file size from
Chris Wilson.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is useful for userspace utilities which wish to use the previous
interface, specifically for micromanaging the increase/decrease steps by
setting min == max.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Provide a standardized sysfs interface for setting min, and max
frequencies. The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:
RP0 - ie. max frequency
RP1 - ie. "preferred min" frequency
RPn - seriously lowest frequency
Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).
The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).
v4: Make exceeding the soft limits return -EINVAL as well (Daniel)
v3: bug fix (Ben) - was passing the MHz value to gen6_set_rps instead of
the step value. To fix, deal only with step values by doing the divide
at the top.
v2: add the dropped mutex_unlock in error cases (Chris)
EINVAL on both too min, or too max (Daniel)
v2 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This has been tons of fun to figure out with git blame. The first
notion of this code block goes back to the original cpu edp enabling
for ilk in
commit 32f9d658ae
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date: Fri Jul 24 01:00:32 2009 +0800
drm/i915: Add eDP support on IGDNG mobile chip
Two things are notable in this commit wrt to the this edp special
case:
- The IS_eDP check _only_ fires for DP A, i.e. cpu edp ports.
- The cpu edp port is disabled at the top of the dp_link_down function.
My theory is that these hacks was added to work around the completely
different modeset sequence for cpu edp ports compared to pch edp
ports. With the cpu edp confusion on ilk (and snb/ivb) now fixed up,
this shouldn't be required any more.
The really interesting question is how this special cases survived
this long in the code. The first step is declaring the pch port D as
eDP if it's used for an internal panel:
commit b329530ca7
Author: Adam Jackson <ajax@redhat.com>
Date: Fri Jul 16 14:46:28 2010 -0400
drm/i915/dp: Correctly report eDP in the core connector type
This commit unfortunately failed to notice that not all edp ports are
created equal. Then follow a flurry of refactorings, culminating in a
patch from Keith Packard which resulted in the current logic (by
making it "correct" for all platforms that have edp):
commit 417e822dee
Author: Keith Packard <keithp@keithp.com>
Date: Tue Nov 1 19:54:11 2011 -0700
drm/i915: Treat PCH eDP like DP in most places
None of these cleanups or refactorings supply any reason why we need
this code, they've simply carried it on as-is.
Hence presume it might be harmful with the current code and rip it
out. We do rewrite the link training bits completely anyway when
re-training the link.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The exec_list is of type drm_i915_gem_exec_object2 and so casting it to
a drm_i915_gem_relocation_entry is very confusing!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is rather a hack to fix brightness hotkeys on a Clevo laptop. CADL is not
used anywhere in the driver code at the moment, but it could be used in BIOS as
is the case with the Clevo laptop.
The Clevo B7130 requires the CADL field to contain at least the ID of
the LCD device. If this field is empty, the ACPI methods that are called
on pressing brightness / display switching hotkeys will not trigger a
notification. As a result, it appears as no hotkey has been pressed.
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=45452
Tested-by: Peter Wu <lekensteyn@gmail.com>
Signed-off-by: Peter Wu <lekensteyn@gmail.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
See bspec, Vol3 Part2, Section 1.1.3 "Display Mode Set Sequence". This
applies to all platforms where we currently support eDP on, i.e. ilk,
snb & ivb.
Without this change we fail to light up the eDP port on previously
unused crtcs (likely because something is stuck on the old pipe), and
we also fail to properly disable the old pipe (i.e. bit 30 in the
PIPECONF register is stuck as set until the next reboot).
v2: Rebased on top of the edp panel off sequence changes in 3.6-rc2.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=44001
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These have been added because dp links are fiddle things and don't
like it when we try to re-train an enabled output (or disable a
disabled output harder). And because the crtc helper code is
ridiculously bad add tracking the modeset state.
But with the new code in place it is simply a bug to disable a disabled
encoder or to enable an enabled encoder again. Hence convert these to
WARNs (and bail out for safety), but flatten all conditionals in the
code itself.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the previous patch to clean up where exactly these two functions
are getting called, this patch can tackle the enable/disable code
itself:
- WARN if the port enable bit is in the wrong state or if the edp pll
bit is in the wrong state, just for paranoia's sake.
- Don't disable the edp pll harder in the modeset functions just for
fun.
- Don't set the edp pll enable flag in intel_dp->DP in modeset, do
that while changing the actual hw state. We do the same with the
actual port enable bit, so this is a bit more consistent.
- Track the current DP register value when setting things up and add
some comments how intel_dp->DP is used in the disable code.
v2: Be more careful with resetting intel_dp->DP - otherwise dpms
off->on will fail spectacularly, becuase we enable the eDP port when
we should only enable the eDP pll.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
By using the new pre_enable/post_disable functions.
To ensure that we only frob the cpu edp pll while the pipe is off add
the relevant asserts. Thanks to the new output state staging, this is
now really easy.
With this fixed we can now finally rip out the special-case handling
in the dp dpms code and replace it by the common intel_connector_dpms.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The cpu eDP encoder has some horrible hacks to set up the DP pll at
the right time. To be able to move them to the right place, add some
more encoder callbacks so that this can happen at the right time.
LVDS has some similar funky hacks, but that would require more work
(we need to move around the pll setup a bit). Hence for now only
wire these new callbacks up for ilk+ - we only have cpu eDP on these
platforms.
v2: Bikeshed the vtable ordering, requested by Chris Wilson.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's bogus.
If I've followed the history of this piece of code correctly, i.e. the
initial register write with the following vblank wait, this goes all
the way back to the original enabling of DP support in
commit a4fc5ed698
Author: Keith Packard <keithp@keithp.com>
Date: Tue Apr 7 16:16:42 2009 -0700
drm/i915: Add Display Port support
Unfortunately it seems to be nothing more than glorified duct-tape and
sometimes actively harmful. Adam Jackson noticed this for CPT
platforms with
commit e85194641b
Author: Adam Jackson <ajax@redhat.com>
Date: Thu Jul 21 17:48:38 2011 -0400
drm/i915/dp: Don't turn CPT DP ports on too early
Unfortunately this kept the code around for ilk and gm45.
The specific failure case I'm seeing here is that after a dpms off/on
cycle we have the bits from the last link training (hopefully
successful link training) set in intel_dp->DP. This is requiered so
that complete_link_train can enable the port with the right tuning
values.
Unfortunately writing these again to the disabled port at dpms on time
kills the port somehow until it's disabled - dp link training fails in
an endless loop without this patch on my mobile ilk and gm45.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51493
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the new "standardized" sysfs interfaces we need to be a bit more
careful about setting the RPS values.
Because the sysfs code and the rps workqueue can run at the same time,
if the sysfs setter wins the race to the mutex, the workqueue can come
in and set a value which is out of range (ie. we're no longer protecting
by RPINTLIM).
I was not able to actually make this error occur in testing.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In order to keep our cached values in sync with the hardware, we need a
posting read here.
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Userspace applications such as PowerTOP are interesting in being able to
read the current GPU frequency. The patch itself sets up a generic array
for gen6 attributes so we can easily add other items in the future (and
it also happens to be just about the cleanest way to do this).
The patch is a nice addition to
commit 1ac02185dff3afac146d745ba220dc6672d1d162
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Aug 30 13:26:48 2012 +0200
drm/i915: add a tracepoint for gpu frequency changes
Reading the GPU frequncy can be done by reading a file like:
/sys/class/drm/card0/render_frequency_mhz
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Magic numbers are bad mmmkay. In this case in particular the value is
especially weird because the docs say multiple things. We'll need this
value for sysfs, so extracting it is useful for that as well.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Name variables a bit better for copy-pasters. This got turned up as part
of review for upcoming sysfs patches.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
[danvet: resolved conflicts due to missing some earlier patches.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because declaring a variable in the beginning of the function, then
initializing it 100 lines later, then using it 100 lines later does
not make our code look good IMHO.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because ironlake_crtc_mode_set is a giant function that used to have
404 lines. Let's try to make it less complex/confusing.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Rather than have multiple data structures for describing our page layout
in conjunction with the array of pages, we can migrate all users over to
a scatterlist.
One major advantage, other than unifying the page tracking structures,
this offers is that we replace the vmalloc'ed array (which can be up to
a megabyte in size) with a chain of individual pages which helps reduce
memory pressure.
The disadvantage is that we then do not have a simple array to iterate,
or to access randomly. The common case for this is in the relocation
processing, which will typically fit within a single scatterlist page
and so be almost the same cost as the simple array. For iterating over
the array, the extra function call could be optimised away, but in
reality is an insignificant cost of either binding the pages, or
performing the pwrite/pread.
v2: Fix drm_clflush_sg() to not invoke wbinvd as well! And fix the
trivial compile error from rebasing.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
By using the recently introduced pinning of pages, we can safely drop
the mutex in the knowledge that the pages are not going to disappear
beneath us, and so we can simplify the code for iterating over the pages.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
By using the recently introduced pinning of pages, we can safely drop
the mutex in the knowledge that the pages are not going to disappear
jeneath us, and so we can simplify the code for iterating over the pages.
Note: The old code had such complicated page refcounting since it used
obj->pages as a micro-optimization if it's there, but that could
(before this patch) disappear when we drop the dev->struct_mutex.
Hence some manual page refcounting was required for the slow path,
complicated by the fact that pages returned by shmem_read_mapping_page
already have a pageref, which needs to be dropped again.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Added note to explain the question Ben raised in review.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We need to refcount our pages in order to prevent reaping them at
inopportune times, such as when they currently vmapped or exported to
another driver. However, we also wish to keep the lazy deallocation of
our pages so we need to take a pin/unpinned approach rather than a
simple refcount.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In order to specialise functions depending upon the type of object, we
can attach vfuncs to each object via a new ->ops pointer.
For instance, this will be used in future patches to only bind pages from
a dma-buf for the duration that the object is used by the GPU - and so
prevent them from pinning those pages for the entire of the object.
v2: Bonus comments.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This reverts commit 991083ba60.
We discovered this causes problem on some Dell eDP laptops, so Apple
lose out for now, I might try and whip up a dmi based workaround for 3.6
but I'm not sure I'll get time.
Signed-off-by: Dave Airlie <airlied@redhat.com>
Daniel writes:
"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"
+ 2 fixes.
* 'for-airlied' of git://people.freedesktop.org/~danvet/drm-intel: (78 commits)
drm/i915: fix OOPS in lid_notify
drm/i915: correctly update crtc->x/y in set_base
drm/fb helper: don't call drm_helper_connector_dpms directly
drm/i915: improve modeset state checking after dpms calls
drm/i915: add tons of modeset state checks
drm/i915: no longer call drm_helper_resume_force_mode
drm/i915: disable all crtcs at suspend time
drm/i915: push commit_output_state past the crtc/encoder preparing
drm/i915: switch the load detect code to the staged modeset config
drm/i915: WARN if the pipe won't turn off
drm/i915: s/intel_encoder_disable/intel_encoder_noop
drm/i915: push commit_output_state past crtc disabling
drm/i915: implement new set_mode code flow
drm/i915: compute masks of crtcs affected in set_mode
drm/i915: use staged outuput config in lvds->mode_fixup
drm/i915: use staged outuput config in tv->mode_fixup
drm/i915: extract adjusted mode computation
drm/i915: move output commit and crtc disabling into set_mode
drm/i915: remove crtc disabling special case
drm/i915: push crtc->fb update into pipe_set_base
...
We hit this a lot with i915 and although we'd like to engineer things to hit
it a lot less, this commit at least makes it consume a few less cycles.
from something containing
movzwl 0x0(%rip),%r10d
to
add %r8,%rdx
I only noticed it while using perf to profile something else.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The SH Mobile LCD controller (LCDC) DRM driver supports the main
graphics plane in RGB and YUV formats, as well as the overlay planes (in
alpha-blending mode only).
Only flat panel outputs using the parallel interface are supported.
Support for SYS panels, HDMI and DSI is currently not implemented.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
This patchset introduces a set of helper function for implementing the KMS
framebuffer layer for drivers which use the DRM GEM CMA helper function.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
[Make DRM_KMS_CMA_HELPER a boolean Kconfig option]
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Many embedded drm devices do not have a IOMMU and no dedicated
memory for graphics. These devices use CMA (Contiguous Memory
Allocator) backed graphics memory. This patch provides helper
functions to be able to share the code. The code technically does
not depend on CMA as the backend allocator, the name has been chosen
because CMA makes for a nice, short but still descriptive function
prefix.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Tested-by: Lars-Peter Clausen <lars@metafoo.de>
[Make DRM_GEM_CMA_HELPER a boolean Kconfig option]
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This goes back to
commit c1c7af6089
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Thu Sep 10 15:28:03 2009 -0700
drm/i915: force mode set at lid open time
It was used to fix an issue on a i915GM based Thinkpad X41, which
somehow clobbered the modeset state at lid close time. Since then
massive amounts of things changed: Tons of fixes to the modeset
sequence, OpRegion support, better integration with the acpi code.
Especially OpRegion /should/ allow us to control the display hw
cooperatively with the firmware, without the firmware clobbering the
hw state behind our backs.
So it's dubious whether we still need this.
The second issue is that it's unclear who's responsibility it actually
is to restore the mode - Chris Wilson suggests to just emit a hotplug
event and let userspace figure things out.
The real reason I've stumbled over this is that the new modeset code
breaks drm_helper_resume_force_mode - it OOPSes derefing a NULL vfunc
pointer. The reason this wasn't caught in testing earlier is that in
commit c9354c85c1
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mon Nov 2 09:29:55 2009 -0800
i915: fix intel graphics suspend breakage due to resume/lid event
confusion
logic was added to _not_ restore the modeset state after a resume. And
since most machines are configured to auto-suspend on lid-close, this
neatly papered over the issue.
Summarizing, this shouldn't be required on any platform supporting
OpRegion. And none of the really old machines I have here seem to
require it either. Hence I'm inclined to just rip it out.
But in case that there are really firmwares out there that clobber the
hw state, replace it with a call to intel_modset_check_state. This
will ensure that we catch any issues as soon as they happen.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
While reworking the modeset sequence, this got lost in
commit 25c5b2665f
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Sun Jul 8 22:08:04 2012 +0200
drm/i915: implement new set_mode code flow
I've noticed this because some Xorg versions seem to set up a new mode
with every crtc at (0,0) and then pan to the right multi-monitor
setup. And since some hacks of mine added more calls to mode_set using
the stored crtc->x/y my multi-screen setup blew up.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This reverts commit 985f61f7ee.
This commit fixed certain cases, but ended up regressing others
due to limitations in the current KMS API. A proper fix is too
invasive for 3.6. Push it back to 3.7.
Reported-by: Andres Freund <andres@anarazel.de>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Clear Audio Enable bit to trigger unsolicated event to notify Audio
Driver part the HDMI hot plug change. The patch fixed the bug when
remove HDMI cable the bit was not cleared correctly.
In intel_hdmi_dpms(), if intel_hdmi->has_audio been true, the "Audio enable bit" will
be set to trigger unsolicated event to notify Alsa driver the change.
intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after
remove the hdmi cable, here's debug log:
[ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2
[ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0
so when comes back to intel_hdmi_dpms(), the "Audio enable bit" will not be cleared. And this
cause the eld infomation and pin presence doesnot update accordingly in alsa driver side.
This patch will also trigger unsolicated event to alsa driver to notify the hot plug event:
[ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1
[ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0
Signed-off-by: Wang Xingchao <xingchao.wang@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Pin-leaks persist and we get the perennial bug reports of machine
lockups to the BUG_ON(pin_count==MAX). If we instead loudly report that
the object cannot be pinned at that time it should prevent the driver from
locking up, and hopefully restore a semblance of working whilst still
leaving us a OOPS to debug.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Otherwise things migt not work too well.
Breakage introduced in
commit eb1cbe4848
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Mar 28 23:12:16 2012 +0200
drm/i915: split PLL update code out of i9xx_crtc_mode_set
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: stable@vger.kernel.org (for 3.5 only)
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Limit printing bad edid information at one time per connector.
Connector that are connected to a bad monitor/kvm will likely
stay connected to the same bad monitor/kvm and it makes no
sense to keep printing the bad edid message.
Signed-off-by: Jerome Glisse <jglisse@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
We noticed a plymouth bug on Fedora 18, and I then
noticed this stupid thinko, fixing it fixed the problem
with plymouth.
Cc: stable@vger.kernel.org
Acked-by: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
For DP we can use the same PPLL for all active DP
encoders. Take advantage of that to prevent cases
where we may end up sharing a PPLL between DP and
non-DP which won't work. Also clean up the code
a bit.
v2: - fix missing pll_id assignment in crtc init
v3: - fix DP PPLL check
- document functions
- break in main encoder search loop after matching.
no need to keep checking additional encoders.
fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=54471
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Inki Dae writes:
- fix build warnings
- minor code cleanup
- remove non-standard format, DRM_FORMAT_NV12M
- add dummy mmap for exynos dmabuf
. dma_buf export needs this patch
* 'exynos-drm-fixes' of git://git.infradead.org/users/kmpark/linux-samsung:
drm: Drop the NV12M and YUV420M formats
drm/exynos: remove DRM_FORMAT_NV12M from plane module
drm/exynos: fix double call of drm_prime_(init/destroy)_file_private
drm/exynos: add dummy support for dmabuf-mmap
drm/exynos: Add missing braces around sizeof in exynos_mixer.c
drm/exynos: Add missing braces around sizeof in exynos_hdmi.c
drm/exynos: Make g2d_pm_ops static
drm/exynos: Add dependency for G2D in Kconfig
drm/exynos: fixed page align bug.
drm/exynos: Use ERR_CAST inlined function instead of ERR_PTR(PTR_ERR(.. [1]
drm/exynos: Use devm_* functions in exynos_drm_g2d.c file
drm/exynos: Use devm_kzalloc in exynos_drm_hdmi.c file
drm/exynos: Use devm_kzalloc in exynos_drm_vidi.c file
drm/exynos: Remove redundant check in exynos_drm_fimd.c file
drm/exynos: Remove redundant check in exynos_hdmi.c file
this patch removes DRM_FORMAT_NV12M from plane module because this format
is same as DRM_FORMAT_NV12. DRM_FORMAT_NV12M will be identified by
mode_cmd->handles and mode_cmd->offsets fields internally.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin.park <kyungmin.park@samsung.com>
This patch adds a stub function for DMABUF mmap.
This allows to export a DMABUF.
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Fixes the following checkpatch warnings:
WARNING: sizeof filter_y_horiz_tap8 should be sizeof(filter_y_horiz_tap8)
WARNING: sizeof filter_y_vert_tap4 should be sizeof(filter_y_vert_tap4)
WARNING: sizeof filter_cr_horiz_tap4 should be sizeof(filter_cr_horiz_tap4)
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Fixes the following checkpatch warnings:
WARNING: sizeof *res should be sizeof(*res)
WARNING: sizeof res->regul_bulk[0] should be sizeof(res->regul_bulk[0])
WARNING: sizeof *res should be sizeof(*res)
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Fixes the following warning:
drivers/gpu/drm/exynos/exynos_drm_g2d.c:897:1: warning:
symbol 'g2d_pm_ops' was not declared. Should it be static?
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Select Exynos DRM based G2D only if non-DRM based Exynos G2D driver
is not selected.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
do not align in page unit at dumb creation. the align is done
by exynos_drm_gem_create() to be called commonly.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
The semantic patch that makes this change is available
in scripts/coccinelle/api/err_cast.cocci.
More information about semantic patching is available at
http://coccinelle.lip6.fr/
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
devm_* functions are device managed functions and make error handling
and cleanup cleaner and simpler.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
devm_kzalloc is a device managed function and makes freeing and error
handling simpler.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
devm_kzalloc is a device managed function and makes freeing and error
handling simpler.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
devm_request_and_ioremap function checks the validity of the
pointer returned by platform_get_resource. Hence an additional check
in the probe function is not necessary.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
devm_request_and_ioremap function checks the validity of the
pointer returned by platform_get_resource. Hence an additional check
in the probe function is not necessary.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Testing and works with the -modesetting driver,
Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
This patch removes a unused struct psb_intel_connector
Sparse gives a warning:
drivers/gpu/drm/gma500/cdv_intel_hdmi.c:142:30: warning:
unused variable ‘psb_intel_connector’ [-Wunused-variable]
Signed-off-by: Emil Goode <emilgoode@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The register map patches didn't set one value for the GMA600 which
means the Fujitsu Q550 dies on boot with the GMA500 driver enabled.
Add the map entry so we don't read from the device MMIO + 0 by mistake.
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Horses <stable@vger.kernel.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The memory return by kzalloc() or kmem_cache_zalloc() has already
be set to zero, so remove useless memset(0).
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The current logic for probing ddc is limited to
2 blocks (256 bytes), this patch adds support
for the 4 block (512) data.
To do this, a single 8-bit segment index is
passed to the display via the I2C address 30h.
Data from the selected segment is then immediately
read via the regular DDC2 address using a repeated
I2C 'START' signal.
Signed-off-by: Shirish S <s.shirish@samsung.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Since arch/arm/include/asm/pgtable.h contains:
#define io_remap_pfn_range(vma,from,pfn,size,prot) \
remap_pfn_range(vma, from, pfn, size, prot)
there is no point treating ARM as a special case in distinguishing
between remap_pfn_range() and io_remap_pfn_range().
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Dave Airlie <airlied@redhat.com>
There are two slightly different pieces of code for HDMI VSDB
detection. Unify the code into a single helper function.
Also fix a bug where drm_detect_hdmi_monitor() would stop looking
for the HDMI VSDB after the first vendor specific block is found,
whether or not that block happened to be the HDMI VSDB. The
standard allows for any number of vendor specific blocks to be
present.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The length of HDMI VSDB must be at least 5 bytes. Other than the minimum,
nothing else about the length is specified. Check the length before
accessing any additional field beyond the minimum length.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Make sure drm_detect_hdmi_monitor() and drm_detect_monitor_audio() don't
access beyond the extension block.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
At the moment, there is an inconsistency in the way modes are named.
Modes with timings parsed from the EDID information will call
drm_mode_set_name(), which will name the mode using this form:
<horizontal-res>x<vertical-res><interlace-char>
eg, 1920x1080i for an interlaced mode, or 1920x1080 for a progressive
mode.
However, timings parsed using the tables in drm_edid_modes.h do not
have the 'i' suffix. You are left to deduce that they're interlaced
from xrandr's output by the lower vertical refresh frequencies.
This patch changes the interlaced mode names in drm_edid_modes.h to
follow the style set by drm_mode_set_name(), which makes it clear
in xrandr which modes are interlaced and which are not (as xrandr
groups the refresh rates on a line according to the name field.)
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
DRM users should be able to create/destroy/manage dumb- and frame-buffers
without DRM_MASTER. These ioctls do not affect modesetting so there is no
reason to protect them by drm-master. Particularly, destroying buffers
should always be possible as a client has only access to buffers that they
created. Hence, there is no reason to prevent a client from destroying the
buffers, considering a simple close() would destroy them, anyway.
Furthermore, a display-server currently cannot shutdown correctly if it
does not have DRM_MASTER. If some other display-server becomes active (or
the kernel console), then the background display-server is unable to
destroy its buffers.
Under special curcumstances (like monitor reconfiguration) this might even
happen during runtime.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Daniel writes:
"Nothing really major at all:
- fixup edp setup sequence (Dave)
- disable sdvo hotplug for real, this is a fixup for a messed-up
regression fixer (Jani)
- don't expose dysfunctional backlight driver (Jani)
- properly init spinlock (only used by hsw/vlv code) from Alexander
Shishkin"
along with a couple of more fixes on top.
* 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel:
drm/i915: fix up the IBX transcoder B check
drm/i915: set the right gen3 flip_done mode also at resume
drm/i915: initialize dpio_lock spin lock
drm/i915: do not expose a dysfunctional backlight interface to userspace
drm/i915: only enable sdvo hotplug irq if needed
drm/i915/edp: get the panel delay before powering up
This has been added in
commit de9a35abb3
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Tue Jun 5 11:03:40 2012 +0200
drm/i915: assert that the IBX port transcoder select w/a is implemented
Unfortunately I've failed to notice that these checks are not just
called for the port that is about to be disabled, but for all (which
makes sense for an assert ...), and the WARN missfired when disabling
another pipe than the one with the dp port.
Hence also check whether the port is actually disabled.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54688
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Currently we've only frobbed this bit at irq_init time, but did
not restore it at resume time. Move it to the gen3 clock gating
function to fix this.
Notice while reading through code.
Cc: stable@vger.kernel.org (for 3.5 only)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This thing is killing lockdep.
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
[Jani: move the init next to the other spin lock inits]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Yet again a case where the fb helper is too intimate with the crtc
helper and calls a crtc helepr function directly instead of going
through the interface vtable.
This fixes console blanking in drm/i915 with the new i915-specific
modeset code.
Reported-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This will cause udev to load vmwgfx instead of waiting for X
to do it.
Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
As a quick reference I'll detail the motivation and design of the new code a
bit here (mostly stitched together from patchbomb announcements and commits
introducing the new concepts).
The crtc helper code has the fundamental assumption that encoders and crtcs can
be enabled/disabled in any order, as long as we take care of depencies (which
means that enabled encoders need an enabled crtc to feed them data,
essentially).
Our hw works differently. We already have tons of ugly cases where crtc code
enables encoder hw (or encoder->mode_set enables stuff that should only be
enabled in enocder->commit) to work around these issues. But on the disable
side we can't pull off similar tricks - there we actually need to rework the
modeset sequence that controls all this. And this is also the real motivation
why I've finally undertaken this rewrite: eDP on my shiny new Ivybridge
Ultrabook is broken, and it's broken due to the wrong disable sequence ...
The new code introduces a few interfaces and concepts:
- Add new encoder->enable/disable functions which are directly called from the
crtc->enable/disable function. This ensures that the encoder's can be
enabled/disabled at a very specific in the modeset sequence, controlled by our
platform specific code (instead of the crtc helper code calling them at a time
it deems convenient).
- Rework the dpms code - our code has mostly 1:1 connector:encoder mappings and
does support cloning on only a few encoders, so we can simplify things quite a
bit.
- Also only ever disable/enable the entire output pipeline. This ensures that
we obey the right sequence of enabling/disabling things, trying to be clever
here mostly just complicates the code and results in bugs. For cloneable
encoders this requires a bit of special handling to ensure that outputs can
still be disabled individually, but it simplifies the common case.
- Add infrastructure to read out the current hw state. No amount of careful
ordering will help us if we brick the hw on the initial modeset setup. Which
could happen if we just randomly disable things, oblivious to the state set up
by the bios. Hence we need to be able to read that out. As a benefit, we grow a
few generic functions useful to cross-check our modeset code with actual hw
state.
With all this in place, we can copy&paste the crtc helper code into the
drm/i915 driver and start to rework it:
- As detailed above, the new code only disables/enables an entire output pipe.
As a preparation for global mode-changes (e.g. reassigning shared resources) it
keeps track of which pipes need to be touched by a set of bitmasks.
- To ensure that we correctly disable the current display pipes, we need to
know the currently active connector/encoder/crtc linking. The old crtc helper
simply overwrote these links with the new setup, the new code stages the new
links in ->new_* pointers. Those get commited to the real linking pointers once
the old output configuration has been torn down, before the ->mode_set
callbacks are called.
- Finally the code adds tons of self-consistency checks by employing the new hw
state readout functions to cross-check the actual hw state with what the
datastructure think it should be. These checks are done both after every
modeset and after the hw state has been read out and sanitized at boot/resume
time. All these checks greatly helped in tracking down regressions and bugs in
the new code.
With this new basis, a lot of cleanups and improvements to the code are now
possible (besides the DP fixes that ultimately made me write this), but not yet
done:
- I think we should create struct intel_mode and use it as the adjusted mode
everywhere to store little pieces like needs_tvclock, pipe dithering values or
dp link parameters. That would still be a layering violation, but at least we
wouldn't need to recompute these kinds of things in intel_display.c. Especially
the port bpc computation needed for selecting the pipe bpc and dithering
settings in intel_display.c is rather gross.
- In a related rework we could implement ->mode_valid in terms of ->mode_fixup
in a generic way - I've hunted down too many bugs where ->mode_valid did the
right thing, but ->mode_fixup didn't. Or vice versa, resulting in funny bugs
for user-supplied modes.
- Ditch the idea to rework the hdp handling in the common crtc helper code and
just move things to i915.ko. Which would rid us of the ->detect crtc helper
dependencies.
- LVDS wire pair and pll enabling is all done in the crtc->mode_set function
currently. We should be able to move this to the crtc_enable callbacks (or in
the case of the LVDS wire pair enabling, into some encoder callback).
Last, but not least, this new code should also help in enabling a few neat
features: The hw state readout code prepares (but there are still big pieces
missing) for fastboot, i.e. avoiding the inital modeset at boot-up and just
taking over the configuration left behind by the bios. We also should be able
to extend the configuration checks in the beginning of the modeset sequence and
make better decisions about shared resources (which is the entire point behind
the atomic/global modeset ioctl).
Tested-by: Jani Nikula <jani.nikula@intel.com>
Tested-by: Ben Widawsky <ben@bwidawsk.net>
Tested-by: Damien Lespiau <damien.lespiau@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
Acked-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Tested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that we have solid modeset state tracking and checking code in
place, we can do the Full Monty also after dpms calls.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... let's see whether this catches anything earlier and I can track
down a few bugs.
v2: Add more checks and also add DRM_DEBUG_KMS output so that it's
clear which connector/encoder/crtc is being checked atm. Which proved
rather useful for debugging ...
v3: Add a WARN in the common encoder dpms function, now that also
modeset changes properly update the dpms state ...
v4: Properly add a short explanation for each WARN, to avoid the need
to correlate dmesg lines with source lines accurately. Suggested by
Chris Wilson.
v5: Also dump (expected, found) for state checks (or wherever it's not
apparent from the test what exactly mismatches with expectations).
Again suggested by Chris Wilson.
v6: Due to an issue reported by Paulo Zanoni I've noticed that the
encoder checking is by far not as strict as it could and should be.
Improve this.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since this only calls crtc helper functions, of which a shocking
amount are NULL.
Now the curious thing is how the new modeset code worked with this
function call still present:
Thanks to the hw state readout and the suspend fixes to properly
quiescent the register state, nothing is actually enabled at resume
(if the bios doesn't set up anything). Which means resume_force_mode
doesn't actually do anything and hence nothing blows up at resume
time.
The other reason things do work is that the fbcon layer has it's own
resume notifier callback, which restores the mode. And thanks to the
force vt switch at suspend/resume, that then forces X to restore it's
own mode.
Hence everything still worked (as long as the bios doesn't enable
anything). And we can just kill the call to resume_force_mode.
The upside of both this patch and the preceeding patch to quiescent
the modeset state is that our resume path is much simpler:
- We now longer restore bogus register values (which most often would
enable the backlight a bit and a few ports), causing flickering.
- We now longer call resume_force_mode to restore a mode that the
fbcon layer would overwrite right away anyway.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We need this to avoid confusing the hw state readout code with the cpt
pch plls at resume time: We'd read the new pipe state (which is
disabled), but still believe that we have a life pll connected to that
pipe (from before the suspend). Hence properly disable pipes to clear
out all the residual state.
This has the neat side-effect that we don't enable ports prematurely
by restoring bogus state from the saved register values.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With this change we can (finally!) rip out a few of the temporary hacks
and clean up a few other things:
- Kill intel_crtc_prepare_encoders, now unused.
- Kill the hacks in the crtc_disable/enable functions to always call the
encoder callbacks, we now always call the crtc functions with the right
encoder -> crtc links.
- Also push down the crtc->enable, encoder and connector dpms state
updates. Unfortunately we can't add a WARN in the crtc_disable
callbacks to ensure that the crtc is always still enabled when
disabling an output pipe - the crtc sanitizer of the hw readout path
can hit this when it needs to disable an active pipe without any
enabled outputs.
- Only call crtc->disable if the pipe is already enabled - again avoids
running afoul of the new WARN.
v2: Copy&paste our own version of crtc_in_use, too.
v3: We need to update the dpms an encoder->connectors_active states,
too.
v4: I've forgotten to kill the unconditional encoder->disable calls in
the crtc_disable functions.
v5: Rip out leftover debug printk.
v6: Properly clear intel_encoder->connectors_active. This wasn't
properly cleared when disabling an encoder because it was no longer on
the new connector list, but the crtc was still enabled (i.e. switching
the encoder of an active crtc). Reported by Jani Nikula.
v7: Don't clobber the encoder->connectors_active state of untouched
encoders. Since X likes to first disable all outputs with dpms off
before setting a new framebuffer, this hit a few warnings. Reported by
Paulo Zanoni.
v8: Kill the now stale comment warning that intel_crtc->active is not
always updated at the right times.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that set_mode also disables crtcs and expects it's new
configuration in the staged output links we need to adjust the load
detect code a bit.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This seems to be the symptom of a few neat bugs, hence be more
obnoxious when this fails.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because that's what it is. Unfortunately we can't rip this out because
the fb helper has an incetious relationship with the crtc helper - it
likes to call disable_unused_functions, among other things.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This requires a few changes
- We still need a noop function for crtc->disable, becuase the fb
helper is a bit too intimate with the crtc helper.
- We need to clear crtc->fb ourselves in intel_crtc_disable now that
we no longer rely on the helper's disable_unused_functions to do
that.
- We need to split out the sare update code, becuase the crtc code
can't call update_dpms any more, it needs to disable the crtc
unconditionally. This is because we now keep onto the encoder ->
crtc mapping of the (still) active output pipe configuration.
- To check that we really disable a crtc that still has encoders,
insert a WARN_ON(!enabled) in the crtc disable function.
- Lastly, we need to walk over all crtcs to update their enabled state
after having called commit_output_state - for all disabled crtcs the
crtc helper code has done that for us previously.
v2: Update connector dpms and encoder->connectors_active after
disabling the crtc, too.
v3: Noop-out intel_encoder_disable. Similarly to the crtc disable
callback used by the crtc helper code we can't simply remove all these
encoder callbacks: The fb helper (which we still use) has a rather
incetious relationship with the crtc helper code ...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... using the pipe masks from the previous patch.
Well, not quite:
- We still need to call the disable_unused_functions helper, until
we've moved the call to commit_output_state further down and
adjusted intel_crtc_disable a bit. The next patch will do that.
- Because we don't support (yet) mode changes on more than one crtc at
a time, some of the modeset_pipes checks are a bit hackish - but
that only needs fixing once we incorporate global modeset support.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is definetely a bit more generic than currently required, but if
we keep track of all crtcs that need to be disabled/enable (because
they loose an encoder or something similar), crtcs that get completely
disabled and those that we need to do an actual mode change nicely
prepares us for global modeset operations on multiple crtcs.
The only big thing missing here would be a global resource allocation
step (for e.g. pch plls), which would equally frob these bitmasks if
e.g. a crtc only needs a new pll. Or if we need to enable dithering on
an another pipe due to bandwidth constrains somewhere.
These masks aren't yet put to use in this patch, this will follow in the
next one.
v2-v5: Fix up the computations for good (hopefully).
v6: Fixup a confusion reported by Damien Lespiau: I've conserved the
(imo braindead) behaviour of the crtc helper to disable _any_
disconnected outputs if we do a modeset, even when that newly disabled
connector isn't connected to the crtc being changed by the modeset.
The effect of that is that we could disable an arbitrary number of
unrelated crtcs, which I haven't taken into account when writing this
code. Fix this up.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- Use the check_cloned helper from the previous patch.
- Use encoder->new_crtc to check crtc properties.
v2: Kill the double negation with s/!non_cloned/is_cloned, suggested
by Jesse Barnes.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The "is this encoder cloned" check will be reused by the lvds encoder,
hence exract it.
v2: Be a bit more careful about that we need to check the new, staged
ouput configuration in the check_non_cloned helper ...
v3: Kill the double negation with s/!non_cloned/is_cloned/, suggested
by Jesse Barnes.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
While at it, adjust a few things:
- Only assigng the new mode to crtc->mode right before calling the
mode_set callbacks - none of the previous callbacks depend upon
this, they all use the mode argument (as they should).
- Check encoder->new_crtc instead of the current crtc to check whether
the encoder will be used. This prepares for moving the staged output
committing further down in the sequence. Follow-on patches will fix
up individual ->mode_fixup callbacks (only tv and lvds are affected
though).
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's rather pointless to compute crtc->enabled twice right away ;-)
The only thing we really have to be careful about is that we frob the
dpms state only after a successful modeset and when we've actually
haven't just disabled the crtc.
Hooray for convoluted interfaces ...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Originally this has been introduced in
commit 6eebd6bb5f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Mon Nov 28 21:10:05 2011 +0000
drm: Fix lack of CRTC disable for drm_crtc_helper_set_config(.fb=NULL)
With the improvements of the output state staging and no longer
overwriting crtc->fb before the hw state is updated we can now handle
crtc disabling as part of the normal modeset sequence.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Passing in the old fb, having overwritten the current fb, leads to
some neatly convoluted code. It's much simpler if we defer the
crtc->fb update to the place that updates the hw, in pipe_set_base.
This way we also don't need to restore anything in case something
fails - we only update crtc->fb once things have succeeded.
The real reason for this change is that now we keep the old fb
assigned to crtc->fb, which allows us to finally move the crtc disable
case into the common low-level set_mode function in the next patch.
Also don't clobber crtc->x and crtc->y, we neatly pass these down the
callchain already. Unfortunately we can't do the same with crtc->mode,
because that one is being used in the mode_set callbacks.
v2: Don't restore the drm_crtc object any more on failed modesets,
since we've lose an fb reference otherwise. Also (and this is the
reason this has been found), this totally confused the modeset state
tracking, since it clobbers crtc->enabled. Issue reported by Paulo
Zanoni.
v3: Rip out the entire crtc saving into struct intel_set_config, not
just the restoring part.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
