- Accurate frontbuffer tracking and frontbuffer rendering invalidate, flush and
flip events. This is prep work for proper PSR support and should also be
useful for DRRS&fbc.
- Runtime suspend hardware on system suspend to support the new SOix sleep
states, from Jesse.
- PSR updates for broadwell (Rodrigo)
- Universal plane support for cursors (Matt Roper), including core drm patches.
- Prefault gtt mappings (Chris)
- baytrail write-enable pte bit support (Akash Goel)
- mmio based flips (Sourab Gupta) instead of blitter ring flips
- interrupt handling race fixes (Oscar Mateo)
And old, not yet merged features from the previous round:
- rps/turbo support for chv (Deepak)
- some other straggling chv patches (Ville)
- proper universal plane conversion for the primary plane (Matt Roper)
- ppgtt on vlv from Jesse
- pile of cleanups, little fixes for insane corner cases and improved debug
support all over
* tag 'drm-intel-next-2014-06-20' of git://anongit.freedesktop.org/drm-intel: (99 commits)
drm/i915: Update DRIVER_DATE to 20140620
drivers/i915: Fix unnoticed failure of init_ring_common()
drm/i915: Track frontbuffer invalidation/flushing
drm/i915: Use new frontbuffer bits to increase pll clock
drm/i915: don't take runtime PM reference around freeze/thaw
drm/i915: use runtime irq suspend/resume in freeze/thaw
drm/i915: Properly track domain of the fbcon fb
drm/i915: Print obj->frontbuffer_bits in debugfs output
drm/i915: Introduce accurate frontbuffer tracking
drm/i915: Drop schedule_back from psr_exit
drm/i915: Ditch intel_edp_psr_update
drm/i915: Drop unecessary complexity from psr_inactivate
drm/i915: Remove ctx->last_ring
drm/i915/chv: Ack interrupts before handling them (CHV)
drm/i915/bdw: Ack interrupts before handling them (GEN8)
drm/i915/vlv: Ack interrupts before handling them (VLV)
drm/i915: Ack interrupts before handling them (GEN5 - GEN7)
drm/i915: Don't BUG_ON in i915_gem_obj_offset
drm/i915: Grab dev->struct_mutex in i915_gem_pageflip_info
drm/i915: Add some L3 registers to the parser whitelist
...
Conflicts:
drivers/gpu/drm/i915/i915_drv.c
This property will be used by the MST code to provide userspace
with a path to parse so it can recognise connectors around hotplugs.
Reviewed-by: Todd Previte <tprevite@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This can be called to update things after dynamic connectors/encoders
are created/deleted.
Reviewed-by: Todd Previte <tprevite@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Add a file to debugfs for each connector to allow the EDID to be
overridden.
v2: Copy ubuf before accessing it and reject invalid length data. (David
Herrmann)
Ensure override_edid is reset when a new EDID value is written.
(David Herrmann)
Fix the debugfs file permissions. (David Herrmann)
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add a file to debugfs for each connector to enable modification of the
"force" connector attribute. This allows connectors to be enabled or
disabled for testing and debugging purposes.
v2: Add stricter value checking and clean up debugfs_entry if file
creation fails in drm_debugfs_connector_add. (David Herrmann)
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Introduce generic functions to register and unregister connectors. This
provides a common place to add and remove associated user space
interfaces.
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Universal plane support had placeholders for cursor planes, but didn't
actually do anything with them. Save the cursor plane reference inside
the crtc and update the cursor plane parameter from void* to drm_plane.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Pallavi G<pallavi.g@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If drivers support universal planes and have registered a cursor plane
with the DRM core, we should use that universal plane support when
handling legacy cursor ioctls. Drivers that transition to universal
planes won't have to maintain separate legacy ioctl handling; drivers
that don't transition to universal planes will continue to operate
without any change to behavior.
Note that there's a bit of a mismatch between the legacy cursor ioctls
and the universal plane API's --- legacy ioctl's use driver buffer
handles directly whereas the universal plane API takes drm_framebuffers.
Since there's no way to recover the driver handle from a
drm_framebuffer, we can implement legacy ioctl's in terms of universal
plane interfaces, but cannot implement universal plane interfaces in
terms of legacy ioctls. Specifically, there's no way to create a
general cursor helper in the way we previously created a primary plane
helper.
It's important to land this patch before any patches that add universal
cursor support to individual drivers so that drivers don't have to worry
about juggling two different styles of reference counting for cursor
buffers when userspace mixes and matches legacy and universal cursor
calls. With this patch, a driver that switches to universal cursor
support may assume that all cursor buffers are wrapped in a
drm_framebuffer and can rely on framebuffer reference counting for all
cursor operations.
v4:
- Add comments pointing out setplane_internal's reference-eating
semantics.
v3:
- Drop drm_mode_rmfb() call that is no longer needed now that we're
using setplane_internal(), which takes care of deref'ing the
appropriate framebuffer.
v2:
- Use new add_framebuffer_internal() function to create framebuffer
rather than trying to call directly into the ioctl interface and
look up the handle returned.
- Use new setplane_internal() function to update the cursor plane
rather than calling through the ioctl interface. Note that since
we're no longer looking up an fb_id, no extra reference will be
taken here.
- Grab extra reference to fb under lock in !BO case to avoid issues
where racing userspace could cause the fb to be destroyed out from
under us after we grab the fb pointer.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Pallavi G<pallavi.g@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Refactor DRM setplane code into a new setplane_internal() function that
takes DRM objects directly as parameters rather than looking them up by
ID. We'll use this in a future patch when we implement legacy cursor
ioctls on top of the universal plane interface.
v3:
- Move integer overflow checking from setplane_internal to setplane
ioctl. The upcoming legacy cursor support via universal planes needs
to maintain current cursor ioctl semantics and not return error for
these extreme values (found via intel-gpu-tools kms_cursor_crc test).
v2:
- Allow planes to be disabled without a valid crtc again (and add
mention of this to setplane's kerneldoc, since it doesn't seem to be
mentioned anywhere else).
- Reformat some parameter line wrap
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Pallavi G<pallavi.g@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Refactor DRM framebuffer creation into a new function that returns a
struct drm_framebuffer directly. The upcoming universal cursor support
will want to create framebuffers internally to wrap cursor buffers, so
we want to be able to share that framebuffer creation with the
drm_mode_addfb2 ioctl handler.
v2: Take struct drm_mode_fb_cmd2 parameter directly rather than void*
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Pallavi G<pallavi.g@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I've fumbled my own idea and enthusiastically wrapped all the
getconnector code with the connection_mutex. But we only need it to
chase the connector->encoder link. Even there it's not really needed
since races with userspace won't matter, but better paranoid and
consistent about this stuff.
If we grap it everywhere connector probe callbacks can't grab it
themselves, which means they'll deadlock. i915 does that for the load
detect pipe. Furthermore i915 needs to do a ww dance since we also
need to grab the mutex of the load detect crtc.
This is a regression from
commit 6e9f798d91
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu May 29 23:54:47 2014 +0200
drm: Split connection_mutex out of mode_config.mutex (v3)
Cc: Rob Clark <robdclark@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.
Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For atomic, it will be quite necessary to not need to care so much
about locking order. And 'state' gives us a convenient place to stash a
ww_ctx for any sort of update that needs to grab multiple crtc locks.
Because we will want to eventually make locking even more fine grained
(giving locks to planes, connectors, etc), split out drm_modeset_lock
and drm_modeset_acquire_ctx to track acquired locks.
Atomic will use this to keep track of which locks have been acquired
in a transaction.
v1: original
v2: remove a few things not needed until atomic, for now
v3: update for v3 of connection_mutex patch..
v4: squash in docbook
v5: doc tweaks/fixes
Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
After the split-out of crtc locks from the big mode_config.mutex
there's still two major areas it protects:
- Various connector probe states, like connector->status, EDID
properties, probed mode lists and similar information.
- The links from connector->encoder and encoder->crtc and other
modeset-relevant connector state (e.g. properties which control the
panel fitter).
The later is used by modeset operations. But they don't really care
about the former since it's allowed to e.g. enable a disconnected VGA
output or with a mode not in the probed list.
Thus far this hasn't been a problem, but for the atomic modeset
conversion Rob Clark needs to convert all modeset relevant locks into
w/w locks. This is required because the order of acquisition is
determined by how userspace supplies the atomic modeset data. This has
run into troubles in the detect path since the i915 load detect code
needs _both_ protections offered by the mode_config.mutex: It updates
probe state and it needs to change the modeset configuration to enable
the temporary load detect pipe.
The big deal here is that for the probe/detect users of this lock a
plain mutex fits best, but for atomic modesets we really want a w/w
mutex. To fix this lets split out a new connection_mutex lock for the
modeset relevant parts.
For simplicity I've decided to only add one additional lock for all
connector/encoder links and modeset configuration states. We have
piles of different modeset objects in addition to those (like bridges
or panels), so adding per-object locks would be much more effort.
Also, we're guaranteed (at least for now) to do a full modeset if we
need to acquire this lock. Which means that fine-grained locking is
fairly irrelevant compared to the amount of time the full modeset will
take.
I've done a full audit, and there's just a few things that justify
special focus:
- Locking in drm_sysfs.c is almost completely absent. We should
sprinkle mode_config.connection_mutex over this file a bit, but
since it already lacks mode_config.mutex this patch wont make the
situation any worse. This is material for a follow-up patch.
- omap has a omap_framebuffer_flush function which walks the
connector->encoder->crtc links and is called from many contexts.
Some look like they don't acquire mode_config.mutex, so this is
already racy. Again fixing this is material for a separate patch.
- The radeon hot_plug function to retrain DP links looks at
connector->dpms. Currently this happens without any locking, so is
already racy. I think radeon_hotplug_work_func should gain
mutex_lock/unlock calls for the mode_config.connection_mutex.
- Same applies to i915's intel_dp_hot_plug. But again, this is already
racy.
- i915 load_detect code needs to acquire this lock. Which means the
w/w dance due to Rob's work will be nicely contained to _just_ this
function.
I've added fixme comments everywhere where it looks suspicious but in
the sysfs code. After a quick irc discussion with Dave Airlie it
sounds like the lack of locking in there is due to sysfs cleanup fun
at module unload.
v1: original (only compile tested)
v2: missing mutex_init(), etc (from Rob Clark)
v3: i915 needs more care in the conversion:
- Protect the edp pp logic with the connection_mutex.
- Use connection_mutex in the backlight code due to
get_pipe_from_connector.
- Use drm_modeset_lock_all in suspend/resume paths.
- Update lock checks in the overlay code.
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Rob Clark <robdclark@gmail.com>
I find myself making this change locally whenever debugging FB reference
counting. Which seems a bit silly.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
An object property is an id (idr) for a drm mode object. This
will allow a property to be used set/get a framebuffer, CRTC, etc.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we continue to use bitmask for type, we will quickly run out of room
to add new types. Split this up so existing part of bitmask range
continues to function as before, but reserve a chunk of the remaining
space for an integer type-id. Wrap this all up in some type-check
helpers to keep the backwards-compat uglyness contained.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
No longer used or needed as the structs have a name field.
Acked-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This makes drm_get_encoder_name() thread safe.
Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01\
.ptsecurity.ru
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Update pull request with drm core patches. Mostly some polish for the
primary plane stuff and a pile of patches all over from Thierry. Has
survived a few days in drm-intel-nightly without causing ill.
I've frobbed my scripts a bit to also tag my topic branches so that you
have something stable to pull - I've accidentally pushed a bunch more
patches onto this branch before you've taken the old pull request.
* tag 'topic/core-stuff-2014-05-05' of git://anongit.freedesktop.org/drm-intel:
drm: Make drm_crtc_helper_disable() return void
drm: Fix indentation of closing brace
drm/dp: Fix typo in comment
drm: Fixup flip-work kerneldoc
drm/fb: Fix typos
drm/edid: Cleanup kerneldoc
drm/edid: Drop revision argument for drm_mode_std()
drm: Try to acquire modeset lock on panic or sysrq
drm: remove unused argument from drm_open_helper
drm: Handle ->disable_plane failures correctly
drm: Simplify fb refcounting rules around ->update_plane
drm/crtc-helper: gc usless connector loop in disable_unused_functions
drm/plane_helper: don't disable plane in destroy function
drm/plane-helper: Fix primary plane scaling check
drm: make mode_valid callback optional
drm/edid: Fill PAR in AVI infoframe based on CEA mode list
The ->disable_plane hook always had a return value, but only since the
introduction of primary planes was there any implementation that
actually failed.
So handle such failures correctly.
Note that drm_plane_force_disable is special: In the modeset cleanup
case we first disable all crtc, so primary planes should all be freed
already. And in the fb helper we only reset non-primary planes. Still
better be paranoid and add an early return.
I don't see how this could happen, but it might fix the fb refcount
underrun Thierry is seeing. Matt Roper spotted this issue.
Cc: Thierry Reding <treding@nvidia.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The introduction of primary planes has apparently caused a bit of fb
refcounting fun for people. That makes it a good time to clean up the
arcane rules and slight differences between ->update_plane and
->set_config. The new rules are:
- The core holds a reference for both the new and the old fb (if
they're non-NULL of course) while calling into the driver through
either ->update_plane or ->set_config.
- Drivers may not clobber plane->fb if their callback fails. If they
do that, they need to store a pointer to the old fb in it again.
When calling into the driver plane->fb still points at the current
(old) framebuffer.
- The core will update the plane->fb pointer on success. Drivers can
do that themselves too, but aren't required to any more for the
primary plane.
- The core will update fb refcounts for the plane->fb pointer,
presuming the drivers hold up their end of the bargain.
v2: Remove now unused tmpfb (Thierry)
v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
the commit message a bit.
v4: Also fix up the handling of ->disable_plane in
drm_plane_force_disable. The issue was that we didn't save plane->fb
over the ->disable_plane call. Just paranoia, nothing relies on this.
v5: Keep still useful comments about directly calling ->set_config,
which I should have done for v4 already. Requested by Matt.
Cc: Thierry Reding <treding@nvidia.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Userspace clients which wish to receive all DRM planes (primary and
cursor planes in addition to the traditional overlay planes) may set the
DRM_CLIENT_CAP_UNIVERSAL_PLANES capability.
v2: Hide behind drm.universal_planes module option [suggested by
Daniel Vetter]
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Now that CRTC's have a primary plane, there's no need to track the
framebuffer in the CRTC. Replace all references to the CRTC fb with the
primary plane's fb.
This patch was generated by the Coccinelle semantic patching tool using
the following rules:
@@ struct drm_crtc C; @@
- (C).fb
+ C.primary->fb
@@ struct drm_crtc *C; @@
- (C)->fb
+ C->primary->fb
v3: Generate patch via coccinelle. Actual removal of crtc->fb has been
moved to a subsequent patch.
v2: Fixup several lingering crtc->fb instances that were missed in the
first patch iteration. [Rob Clark]
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Add a new drm_crtc_init_with_planes() to allow drivers to provide
specific primary and cursor planes at CRTC initialization. The existing
drm_crtc_init() interface remains to avoid driver churn in existing
drivers; it will initialize the CRTC with a plane helper-created primary
plane and no cursor plane.
v2:
- Move drm_crtc_init() to plane helper file so that nothing in the DRM
core depends on helpers. [suggested by Daniel Vetter]
- Keep cursor parameter to drm_crtc_init_with_planes() a void* until
we actually add cursor support. [suggested by Daniel Vetter]
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Add a plane type property to allow userspace to distinguish plane types.
v2: Driver-specific churn eliminated now that drm_plane_init() and
drm_universal_plane_init() were separated out in a previous patch.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Add a new plane initialization interface for universal plane support
that allows a specific plane type (primary, cursor, or overlay) to
be specified.
drm_plane_init() remains as a compatibility API to reduce churn in
existing drivers. The 'bool priv' parameter has been changed to
'bool is_primary' under the assumption that all existing uses of
private planes were representing primary planes.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
This function will be used by the universal plane helpers and may also
be useful for individual drivers.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
The DRM core currently only tracks "overlay"-style planes. Start
refactoring the plane handling to allow other plane types (primary and
cursor) to also be placed on the DRM plane list.
v2: Add drm_for_each_legacy_plane() iterator to smooth transition
of drivers with plane loops.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Add a drm_is_legacy() helper, constify argument to drm_is_render_client(),
and use / change helpers where appropriate.
v2: s/drm_is_legacy/drm_is_legacy_client/ and adapt to new code context.
v3: s/legacy_client/primary_client/
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
control- and render nodes are intended to be master-less.
v2: Replace tests for !legacy with tests for !mode_group for readability.
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Here's my drm documentation update and driver api polish pull request.
Alex reviewed the entire pile, I've applied a little bit of spelling
polish in a few places since then and otherwise the Usual Suspects (David,
Rob, ...) don't seem up to have another look at it (I've poked them on
irc). So I think it's as good as it gets ;-)
Note that I've dropped the final imx breaker patch since that's blocked on
imx getting sane. Once that's landed I'll ping you to pick up that
straggler.
* 'drm-docs' of ssh://people.freedesktop.org/~danvet/drm: (34 commits)
drm/imx: remove drm_mode_connector_detach_encoder harder
drm: kerneldoc polish for drm_crtc.c
drm: kerneldoc polish for drm_crtc_helper.c
drm: drop error code for drm_helper_resume_force_mode
drm/crtc-helper: remove LOCKING from kerneldoc
drm: remove return value from drm_helper_mode_fill_fb_struct
drm/doc: Fix misplaced </para>
drm: remove drm_display_mode->private_size
drm: polish function kerneldoc for drm_modes.[hc]
drm/modes: drop maxPitch from drm_mode_validate_size
drm/modes: drop return value from drm_display_mode_from_videomode
drm/modes: remove drm_mode_height/width
drm: extract drm_modes.h for drm_crtc.h functions
drm: move drm_mode related functions into drm_modes.c
drm/doc: Repleace LOCKING kerneldoc sections in drm_modes.c
drm/doc: Integrate drm_modes.c kerneldoc
drm/kms: rip out drm_mode_connector_detach_encoder
drm/doc: Add function reference documentation for drm_mm.c
drm/doc: Overview documentation for drm_mm.c
drm/mm: Remove MM_UNUSED_TARGET
...
Lets make sure some basic expressions are always true:
bpp != NULL
width != NULL
height != NULL
stride = bpp * width < 2^32
size = stride * height < 2^32
PAGE_ALIGN(size) < 2^32
At least the udl driver doesn't check for multiplication-overflows, so
lets just make sure it will never happen. These checks allow drivers to do
any 32bit math without having to test for mult-overflows themselves.
The two divisions might hurt performance a bit, but dumb_create() is only
used for scanout-buffers, so that should be fine. We could use 64bit math
to avoid the divisions, but that may be slow on 32bit machines.. Or maybe
there should just be a "safe_mult32()" helper, which currently doesn't
exist (I think?).
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
- Standardized on "Returns:" Block.
- Sprinkle missing kerneldoc over all exported functions and all
ioctls.
- Add a stern warning that driver's really shouldn't use
drm_mode_group_init_legacy_group.
- Usual attempt at more consistency.
- Add warnings that drm_mode_object_get/put don't do refcounting,
despite what the names might lead to believe.
- Try to clarify the framebuffer setup/cleanup functions wrt driver
private framebuffers - I've fallen recently over this when reviewing
i915 fbdev patches.
- Align function parameters where the kerneldoc has been updated.
- Most of the drm_get_*_name functions aren't thread safe. Add stern
warnings where this is the case.
Since a lot of the functions in drm_crtc.c are boilerplate to handle
properties and create default sets of them it might be useful to
extract all that code into a new file drm_property.c. Especially since
properties will be used a lot more in the future.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Makes more sense and gives better grouping in the DocBook function
reference sections. To make this possible we need to expose two
functions from drm_crtc.c though. To avoid further namespace pollution
in the system wide headers create a new internal header for such drm
internal symbols.
I expect that longer-term we'll add tons more, but since my goal here
is to polish the kerneldoc that's for another day.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There's not really any value in stating that no locking is needed. And
even if the comment is useful, a check for the right mutex at the
beginning of the function is better since that can't be ingored as
easily as a bit of documentation.
Note that drm_mode_probed_add in drm_crtc.c is also changed, the next
patch will move this into drm_modes.c
v2: Don't add locking WARN_ONs where it is not strictly required (i.e.
the two functions to validate/prune mode lists).
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's only used by imx, and that one gets it wrong - there's no need
to deteach the encoder before removing it.
And really, neither current drm modesetting code nor all the userspace
we have can handle dynamic changes in the set of possible encoders for
a given connector. So let's just remove this before someone starts
doing something really nasty with it.
As a plus, one less kerneldoc comment to write.
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Just like we have for connector type etc.
v2: drop static array (Chris)
v3: add kdoc (Daniel)
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These patches fix some issues caused by the DRM panel support from the
previous pull request and add two more panels (for the Toshiba AC100 as
well as the Seaboard and Ventana).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAABAgAGBQJS4TK6AAoJEN0jrNd/PrOhwXoP/i09xa9CKOnpy8Ewdmgi5gBt
vnjzqjwmHzZRpNeWw2xvcqzv5kKgXkpPdMhNUjErlJTUC1S6uCf/Uj+ExppYYysW
7uxNjPQQc6jgX+0342MabKITeZyTk9iEWPqTDtHDP5O2vXi4XSaQTMm9HpWGe1qZ
1CTb0yJc1epXGJPutlqhJSZgKwzrs6Pm2Q/nsTdxpGPAoTQj9J0NebDaZwJmmFGi
qNyfI4DVBp1sj8AgtUkQQn9x5oLDpdvHKLPHsAWgHnNTKqRxY8C0EL+S2TYGa812
sx8jvZgjvBoPuwaOr4hcOx91OFdfOoKVap9D/corU+UjstF9lTd+PvAHzZMO5qte
yQhWmYFn1gGr/I9b9K/mAB98s83W7lMmSs/7HtiawvLtogZ/t8xvSp94R7X8SGd1
wYcw3NqzgDxrcpJjDOsLIw1Pa4PxXoChtV0SfiaEoVlGn5WSONS/8cKeNwbO6e9D
gKkd/zvCMsYqHt7p8zDizcQKi4W+8zYiZySOfLohZzhf0qaoQnnD5yirfiIpljqA
Ql40F4XP3OF3CliikcSwFhvrTUCLb08Ol0MV6FX1H1iMae0dgiFqtz7AihFTbKHZ
PQeskMgEemxhIqKEsH0JcpV3+YxLRlWy/3I+OO7QczsL0xKD30FQ9Xhsu9EFx8gu
jXVvMDldeefaVBuTdqCb
=NfNj
-----END PGP SIGNATURE-----
Merge tag 'drm/for-3.14-rc1-20140123' of git://anongit.freedesktop.org/tegra/linux into drm-next
drm/tegra: Changes for v3.14-rc1 (update)
These patches fix some issues caused by the DRM panel support from the
previous pull request and add two more panels (for the Toshiba AC100 as
well as the Seaboard and Ventana).
* tag 'drm/for-3.14-rc1-20140123' of git://anongit.freedesktop.org/tegra/linux:
drm/tegra: Obtain head number from DT
drm/panel: update EDID BLOB in panel_simple_get_modes()
gpu: host1x: Remove unnecessary include
drm/tegra: Use proper data type
drm/tegra: Clarify how panel modes override others
drm/tegra: Fix possible CRTC mask for RGB outputs
drm/i915: Use drm_encoder_crtc_ok()
drm: Move drm_encoder_crtc_ok() to core
drm: provide a helper for the encoder possible_crtcs mask
drm/tegra: Don't check resource with devm_ioremap_resource()
drm/panel: Add support for Chunghwa CLAA101WA01A panel
drm/panel: Add support for Samsung LTN101NT05 panel
The encoder possible_crtcs mask identifies which CRTCs can be bound to
a particular encoder. Each bit from bit 0 defines an index in the list
of CRTCs held in the DRM mode_config crtc_list. Rather than having
drivers trying to track the position of their CRTCs in the list, expose
the code which already exists for calculating the appropriate mask bit
for a CRTC.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
[treding@nvidia.com: add drm_crtc_index(), move to core]
Signed-off-by: Thierry Reding <treding@nvidia.com>
Not all drivers will need take all the modeset locks for dirtyfb, so
push the locking down to the drivers.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_fb_get_bpp_depth() likes to complain about unsupported pixel formats
but doesn't bother telling us what the format was. Also format_check()
just returns an error when it encouters an invalid format, leaving the
user scratching his head trying to figure out why addfb failed. Make
life a bit easier by using drm_get_format_name() in both places.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Return -ENOENT for framebuffers like we do for other mode objects that
can't be found.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
We tend to return -EINVAL for everything. Let's try to help poor
userland developers a bit by at least returning -ENONET for missing
objects.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Fix a little spelling of drm_crtc_convert_umode() comment.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The kernel shouldn't accept invalid modes, just say No.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that the coding of stereo layout has changed from a bit field to an
enum, we need remove that check.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Some stereo modes, like frame packing, need a larger CRTC viewport than
the "natural" underlying 2D mode and thus drm_crtc_check_viewport()
needs to query the adjusted mode to use the correct h/vdisplay.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Both setcrtc and page_flip are checking that the framebuffer is big
enough for the defined crtc viewport (x, y, hdisplay, vdisplay). Factor
that code out in a single function.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When setting a stereo 3D mode, there can be only one bit set describing
the layout of the frambuffer(s). So reject invalid modes early.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This capability allows user space to control the delivery of modes with
the 3D flags set. This is to not play games with current user space
users not knowing anything about stereo 3D flags and that could try
to set a mode with one or several of those bits set.
So, the plan is to remove the stereo modes from the list of modes we
give to DRM clients by default, and let them through if we are being
told otherwise.
stereo_allowed is bound to the drm_file structure to make it a
per-client setting, not a global one.
v2: Replace clearing 3D flags by discarding the stereo modes now that
they are regular modes.
v3: SET_CAP -> SET_CLIENT_CAP rename (Chris Wilson)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
DRM_IOCTL_MODE_GETFB is used to retrieve information about a given
framebuffer ID. It is a read-only helper and was thus declassified for
unprivileged access in:
commit a14b1b4247
Author: Mandeep Singh Baines <mandeep.baines@gmail.com>
Date: Fri Jan 20 12:11:16 2012 -0800
drm: remove master fd restriction on mode setting getters
However, alongside width, height and stride information,
DRM_IOCTL_MODE_GETFB also passes back a handle to the underlying buffer of
the framebuffer. This handle allows users to mmap() it and read or write
into it. Obviously, this should be restricted to DRM-Master.
With the current setup, *any* process with access to /dev/dri/card0 (which
means any process with access to hardware-accelerated rendering) can
access the current screen framebuffer and modify it ad libitum.
For backwards-compatibility reasons we want to keep the
DRM_IOCTL_MODE_GETFB call unprivileged. Besides, it provides quite useful
information regarding screen setup. So we simply test whether the caller
is the current DRM-Master and if not, we return 0 as handle, which is
always invalid. A following DRM_IOCTL_GEM_CLOSE on this handle will fail
with EINVAL, but we accept this. Users shouldn't test for errors during
GEM_CLOSE, anyway. And it is still better as a failing MODE_GETFB call.
v2: add capable(CAP_SYS_ADMIN) check for compatibility with i-g-t
Cc: <stable@vger.kernel.org>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This patch adds the notion of a drm_bridge. A bridge is a chained
device which hangs off an encoder. The drm driver using the bridge
should provide the association between encoder and bridge. Once a
bridge is associated with an encoder, it will participate in mode
set, and dpms (via the enable/disable hooks).
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Let applications know whether the kernel supports asynchronous page
flipping.
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
This lets drivers see the flags requested by the application
[airlied: fixup for rcar/imx/msm]
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
It's only used in drm_crtc.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This was last used by nouveau, replaced by a driver-specific property
in:
commit de69185573
Author: Ben Skeggs <bskeggs@redhat.com>
Date: Mon Oct 17 12:23:41 2011 +1000
drm/nouveau: improve dithering properties, and implement proper auto mode
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This makes it so that reloading a module does not cause all the
connector ids to change, which are user-visible and sometimes used
for configuration.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
So it looks like for virtual hw cursors on QXL we need to inform
the "hw" device what the cursor hotspot parameters are. This
makes sense if you think the host has to draw the cursor and interpret
clicks from it. However the current modesetting interface doesn't support
passing the hotspot information from userspace.
This implements a new cursor ioctl, that takes the hotspot info as well,
userspace can try calling the new interface and if it gets -ENOSYS it means
its on an older kernel and can just fallback.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Drivers are allowed (actually have to) disable unrelated crtcs in
their ->set_config callback (when we steal all the connectors from
that crtc). If they do that they'll clear crtc->fb to NULL.
Which results in a refcount leak, since the drm core is keeping track
of that reference.
To fix this track the old fb of all crtcs and adjust references for
all of them. Of course, since we only hold an additional reference for
the fb for the current crtc we need to increase refcounts before we
drop the old one.
This approach has the benefit that it inches us a bit closer to an
atomic modeset world, where we want to update the config of all crtcs
in one step.
This regression has been introduce in the framebuffer refcount
conversion, specifically in
commit b0d1232589
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Tue Dec 11 01:07:12 2012 +0100
drm: refcounting for crtc framebuffers
Reported-by: Russell King <linux@arm.linux.org.uk>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Historically drm lacked fb refcounting, so the updating of crtc->fb
was done by the lower levels at a point convenient to get their own
refcounting (e.g. refcounts for the underlying gem bo, pinning
refcounts) right. With the introduction of refcounted fbs the drm core
handled the fb refcounts, but still relied on drivers to update the
crtc->fb pointer (this approach required the least invasive changes in
drivers).
Enforce this contract with a WARN_ON.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
There's a bunch of unused members inside drm_plane, bloating the size of
the structure needlessly. Eliminate them.
v2: Remove all of it from kernel-doc too
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
v2: Follow the drm_crtc documentation fixes
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
drm_plane_force_disable() will forcibly disable the plane even if user
had previously requested the plane to be enabled.
This can be used to force planes to be off when restoring the fbdev
mode.
The code was simply pulled from drm_framebuffer_remove(), which now
calls the new function as well.
v2: Check plane->fb in drm_plane_force_disable(), drop bogus comment
about disabling crtc
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Keeping the modes in the same order as we probe them makes it a bit
easier to track what's happening.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The structures and strings involved with various pretty-print functions
aren't meant to be modified, so make them all const. The exception is
drm_connector_enum_list which does get modified in drm_connector_init().
While at it move the drm_get_connector_status_name() prototype from
drmP.h to drm_crtc.h where it belongs.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Rather than just printing the pixel format as a hex number, decode the
fourcc into human readable form, and also decode the LE vs. BE flag.
Keep printing the raw hex number too in case it contains non-printable
characters.
Some examples what the new drm_get_format_name() produces:
DRM_FORMAT_XRGB8888: "XR24 little-endian (0x34325258)"
DRM_FORMAT_YUYV: "YUYV little-endian (0x56595559)"
DRM_FORMAT_RGB565|DRM_FORMAT_BIG_ENDIAN: "RG16 big-endian (0xb6314752)"
Unprintable characters: "D??? big-endian (0xff7f0244)"
v2: Fix patch author
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Instead of just printing "status updated from 1 to 2", make those enum
numbers immediately readable.
v2: Also patch output_poll_execute() (Daniel Vetter)
v3: Use drm_get_connector_status_name (Ville Syrjälä)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (for v1)
Signed-off-by: Dave Airlie <airlied@redhat.com>
Daniel writes:
A few intel fixes for smaller issues and one revert for an sdv hack which
we've wanted to kill anyway. Plus two drm patches included for your
convenience, both regression fixers for mine own screw-ups.
+ both fixes for stolen mem handling.
* 'for-linux-next' of git://people.freedesktop.org/~danvet/drm-intel:
drm/i915: clear the stolen fb before resuming
Revert "drm/i915: Calculate correct stolen size for GEN7+"
drm/i915: hsw: fix link training for eDP on port-A
Revert "drm/i915: revert eDP bpp clamping code changes"
drm: don't check modeset locks in panic handler
drm/i915: Fix pipe enabled mask for pipe C in WM calculations
drm/mm: fix dump table BUG
drm/i915: Always normalize return timeout for wait_timeout_ioctl
Since we know that locking is broken in that case and it's more
important to not flood the dmesg with random gunk.
Cc: Dave Airlie <airlied@gmail.com>
Cc: Borislav Petkov <bp@alien8.de>
References: http://lkml.kernel.org/r/20130502000206.GH15623@pd.tnic
Cc: stable@vger.kernel.org
Reported-and-tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There is no way to use modes added to the user_modes list. We never
look at the contents of said list in the kernel, and the only operations
userspace can do are attach and detach. So the only "benefit" of this
interface is wasting kernel memory.
Fortunately it seems no real user space application ever used these
ioctls. So just kill them.
Also remove the prototypes for the non-existing drm_mode_addmode_ioctl()
and drm_mode_rmmode_ioctl() functions.
v2: Use drm_noop instead of completely removing the ioctls
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drivers/gpu/drm/drm_pci.c:155:5: warning: symbol 'drm_pci_set_busid' was not declared. Should it be static?
drivers/gpu/drm/drm_pci.c:197:5: warning: symbol 'drm_pci_set_unique' was not declared. Should it be static?
drivers/gpu/drm/drm_pci.c:269:5: warning: symbol 'drm_pci_agp_init' was not declared. Should it be static?
drivers/gpu/drm/drm_crtc.c:181:1: warning: symbol 'drm_get_dirty_info_name' was not declared. Should it be static?
drivers/gpu/drm/drm_crtc.c:1123:5: warning: symbol 'drm_mode_group_init' was not declared. Should it be static?
drivers/gpu/drm/drm_modes.c:918:6: warning: symbol 'drm_mode_validate_clocks' was not declared. Should it be static?
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
A page flip is not a mode set, changing the frame buffer pixel format
doesn't make sense and isn't handled by most drivers anyway. Disallow
it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Property blob objects need to be destroyed when cleaning up to avoid
memory leaks. Go through the list of all blobs in the
drm_mode_config_cleanup() function and destroy them.
The drm_mode_config_cleanup() function needs to be moved after the
drm_property_destroy_blob() declaration. Move drm_mode_config_init() as
well to keep the functions together.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_framebuffer_lookup() does a kref_get() for the framebuffer if it finds one
corresponding to the fb id passed to it. Use drm_framebuffer_reference() instead
for clarity since it's the function used in other places to take a reference.
Signed-off-by: Archit Taneja <archit@ti.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
We don't grab the modeset locks any more since
commit 468174f748
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Tue Dec 11 00:09:12 2012 +0100
drm: push modeset_lock_all into ->fb_create driver callbacks
Reported-by: Ray Strode <rstrode@redhat.com>
Cc: Ray Strode <rstrode@redhat.com>
Cc: Dave Airlie <airlied@gmail.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
idr_destroy() can destroy idr by itself and idr_remove_all() is being
deprecated. Drop its usage.
* drm_ctxbitmap_cleanup() was calling idr_remove_all() but forgetting
idr_destroy() thus leaking all buffered free idr_layers. Replace it
with idr_destroy().
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: David Airlie <airlied@linux.ie>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Driver implementations of the drm_crtc's .page_flip() function are
required to update the crtc->fb field on success to reflect that the new
framebuffer is now in use. This is important to keep reference counting
on the framebuffers balanced.
While at it, document this requirement to keep others from falling into
the same trap.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
So here's my promised pile of fixes for 3.9. I've dropped the core prep
patches for vt-switchless suspend/resume as discussed on irc. Highlights:
- Fix dmar on g4x. Not really gfx related, but I'm fed up with getting
blamed for dmar crapouts.
- Disable wc ptes updates on ilk when dmar is enabled (Chris). So again,
dmar, but this time gfx related :(
- Reduced range support for hsw, using the pipe CSC (Ville).
- Fixup pll limits for gen3/4 (Patrick Jakobsson). The sdvo patch is
already confirmed to fix 2 bug reports, so added cc: stable on that one.
- Regression fix for 8bit fb console (Ville).
- Preserve lane reversal bits on DDI/FDI ports (Damien).
- Page flip vs. gpu hang fixes (Ville). Unfortuntely not quite all of
them, need to decide what to do with the currently still in-flight ones.
- Panel fitter regression fix from Mika Kuoppala (was accidentally left on
on some pipes with the new modset code since 3.7). This also improves
the modeset sequence and might help a few other unrelated issues with
lvds.
- Write backlight regs even harder ... another installement in our eternal
fight against the BIOS and backlights.
- Fixup lid notifier vs. suspend/resume races (Zhang Rui). Prep work for
new ACPI stuff, but closing the race itself seems worthwile on its own.
- A few other small fixes and tiny cleanups all over.
Lots of the patches are cc: stable since I've stalled on a few
not-so-important fixes for 3.8 due to the grumpy noise Linus made.
* 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel: (33 commits)
intel/iommu: force writebuffer-flush quirk on Gen 4 Chipsets
drm/i915: Disable WC PTE updates to w/a buggy IOMMU on ILK
drm/i915: Implement pipe CSC based limited range RGB output
drm/i915: inverted brightness quirk for Acer Aspire 4736Z
drm/i915: Print the hw context status is debugfs
drm/i915: Use HAS_L3_GPU_CACHE in i915_gem_l3_remap
drm/i915: Fix PIPE_CONTROL DW/QW write through global GTT on IVB+
drm/i915: Set i9xx sdvo clock limits according to specifications
drm/i915: Set i9xx lvds clock limits according to specifications
drm/i915: Preserve the DDI link reversal configuration
drm/i915: Preserve the FDI line reversal override bit on CPT
drm/i915: add missing \n to UTS_RELEASE in the error_state
drm: Use C8 instead of RGB332 when determining the format from depth/bpp
drm: Fill depth/bits_per_pixel for C8 format
drm/i915: don't clflush gem objects in stolen memory
drm/i915: Don't wait for page flips if there was GPU reset
drm/i915: Kill obj->pending_flip
drm/i915: Fix a typo in a intel_modeset_stage_output_state() comment
drm/i915: remove bogus mutex_unlock from error-path
drm/i915: Print the pipe control page GTT address
...
Support for real RGB332 is a rarity, most hardware only really support
C8. So use C8 instead of RGB332 when determining the format based on
depth/bpp.
This fixes 8bpp fbcon on i915, since i915 will only accept C8 and not
RGB332.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59572
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Tested-by: mlsemon35@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Set depth/bits_per_pixel to 8 for C8 format.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We need to clear the local variable to get the refcounting right
(since the reference drm_mode_setplane holds is transferred to the
plane->fb pointer). But should be done _after_ we update the pointer.
Breakage introduced in
commit 6c2a75325c
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Tue Dec 11 00:59:24 2012 +0100
drm: refcounting for sprite framebuffers
Reported-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Rob Clark <rob@ti.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
... it's required. Fix up exynos and the cma helper, and add a
corresponding WARN_ON to drm_fb_helper_restore_fbdev_mode.
Note that tegra calls the fbdev cma helper restore function also from
it's driver-load callback. Which is a bit against current practice,
since usually the call is only from ->lastclose, and initial setup is
done by drm_fb_helper_initial_config.
Also add the relevant drm DocBook entry.
v2: Add promised WARN to restore_fbdev_mode.
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The coup de grace of the entire journey. No more dropped frames every
10s on my testbox!
I've tried to audit all ->detect and ->get_modes callbacks, but things
became a bit fuzzy after trying to piece together the umpteenth
implemenation. Afaict most drivers just have bog-standard output
register frobbing with a notch of i2c edid reading, nothing which
could potentially race with the newly concurrent pageflip/set_cursor
code. The big exception is load-detection code which requires a
running pipe, but radeon/nouveau seem to to this without touching any
state which can be observed from page_flip (e.g. disabled crtcs
temporarily getting enabled and so a pageflip succeeding).
The only special case I could find is the i915 load detect code. That
uses the normal modeset interface to enable the load-detect crtc, and
so userspace could try to squeeze in a pageflip on the load-detect
pipe. So we need to grab the relevant crtc mutex in there, to avoid
the temporary crtc enabling to sneak out and be visible to userspace.
Note that the sysfs files already stopped grabbing the per-crtc locks,
since I didn't want to bother with doing a interruptible
modeset_lock_all. But since there's very little in-between breakage
(essentially just the ability for userspace to pageflip on load-detect
crtcs when it shouldn't on the i915 driver) I figured I don't need to
bother.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The pagelip ioctl itself is rather simply, so the hard work for this
patch is auditing all the drivers:
- exynos: Pageflip is protect with dev->struct_mutex and ...
synchronous. But nothing fancy going on, besides a check whether the
crtc is enabled, which should probably be somewhere in the drm core
so that we have unified behaviour across all drivers.
- i915: hw-state is protected with dev->struct_mutex, the delayed
unpin work together with the other stuff the pageflip complete irq
handler needs is protected by the event_lock spinlock.
- nouveau: With the pin/unpin functions fixed, everything looks safe:
A bit of ttm wrestling and refcounting, and a few channel accesses.
The later are either already proteced sufficiently, or are now safe
with the channel locking introduced to make cursor updates safe.
- radeon: The irq_get/put functions look a bit race, since the
atomic_inc/dec isn't protect with locks. Otoh they're all per-crtc,
so we should be safe with per-crtc locking from the drm core. Then
there's tons of per-crtc register access, which could potentially go
through the indirect reg acces. But that's fixed to make cursor
updates concurrent. Bookeeping for the drm even is also protected
with the even_lock, which also protects against the pageflip irq
handler since radeon hw seems to have no way to queue these up
asynchronously. Otherwise just a bit of ttm-based buffer handling
and fencing, which is now safe with the previous patch to hold
bdev->fence_lock while grabbing the ttm fence.
- shmob: Only one crtc. That's an easy one ...
- vmwgfx: As usual a bit special with tons different things:
- Flippable check using is_implicit and num_implicit. Changes to
those seem to be nicely covered with the global modeset lock, so
we should be fine.
- Some dirty cliprect handling stuff, or at least that is my guess.
Looks like it's fine since either it's per-crtc, invariant or
(like the execbuf stuff launched) protected otherwise.
- Adding the actual flip to the fence_event list. On a quick look
this seems to have solid locking in place, too.
... but generally this is all way over my head.
- imx: Impressive display of races between the page_flip
implementation and the irq handler. Also, ipu_drm_set_base which
gets eventually called from the irq handler to update the display
base isn't really protected against concurrent set_config calls from
process context. In any case, going for per-crtc locking won't make
this worse, so nothing to do.
- omap: The new async callback code merged into 3.8 seems to have
solid locking in place, and there doesn't seem to be any shared
state at risk. Especially since the callbacks still use
modeset_lock_all and are so not converted.
v2: Update omapdrm analysis to 3.8 code per the discussion with Rob
Clark.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that all framebuffer usage is properly refcounted, we are no
longer required to hold the modeset locks while dropping the last
reference. Hence implemented a fastpath which avoids the potential
stalls associated with grabbing mode_config.lock for the case where
there's no other reference around.
Explain in a big comment why it is safe. Also update kerneldocs with
the new locking rules around drm_framebuffer_remove.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the prep patch to encapsulate ->set_crtc calls, this is now
rather easy. Hooray for inconsistent semantics between ->set_crtc and
->page_flip, where the driver callback is supposed to update the fb
pointer, and ->update_plane, where the drm core does the same.
Also, since the drm core functions check crtc->fb before calling into
driver callbacks, we can't really reduce the critical sections
protected by the mode_config locks.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now plane->fb holds a reference onto it's framebuffer. Nothing too
fancy going on here:
- Extract __drm_framebuffer_unreference to be called when we know
we're not dropping the last reference, e.g. useful in the fb cleanup
code.
- Reduce the locked sections in the set_plane ioctl to only protect
plane->fb/plane->crtc and the driver callback (i.e. hw state).
Everything either doesn't disappear (crtc, plane) or is refcounted
(fb), and all the data we check is invariant over the respective
object's lifetimes.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We only need to ensure that the fb stays around for long enough. While
at it, only grab the modeset locks when we need them (since most
drivers don't implement the dirty callback, this should help jitter
and stalls when using the generic modeset driver).
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We only need to push the fb unreference a bit down. While at it,
properly pass the return value from ->create_handle back to userspace.
Most drivers either return -ENODEV if they don't have a concept of
buffer objects (ast, cirrus, ...) or just install a handle for the
underlying gem object (which is ok since we hold a reference on that
through the framebuffer).
v2: Split out the ->create_handle rework in the individual drivers.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And drop it where it's not needed. Most driver just lookup the gem
object, allocate an fb struct, fill in all the useful fields and then
register it with drm_framebuffer_init.
All of these operations are already separately locked, and since we
only put the fb into the fpriv->fbs list _after_ having called
->fb_create, we can't also race with rmfb. We can otoh race with other
ioctls that put the framebuffer to use, but all drivers have been
reorganized already to call drm_framebuffer_init last in the fb
creation sequence.
So essentially, we can completely remove any modeset locks from the
addfb ioctl paths. Yeah!
Also, reference-counting is solid - we get a reference from fb_create
which we transfer to the fpriv->fbs list. And after unlocking the
fpriv->fbs_lock we don't touch the framebuffer any longer. Furthermore
drm_framebuffer_init has added a 2nd reference for the idr lookup, and
any access through that table will do it's own refcounting.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Atm we still need to unconditionally take the modeset locks in the
rmfb paths. But eventually we only want to take them if there are
other users around as a slow-path. This way sane userspace avoids
blocking on edid reads and other stuff in rmfb if it ensures that the
fb isn't used anywhere by a crtc/plane.
We can do a quick check for such other users once framebuffers are
properly refcounting by locking at the refcount - if it's more than 1,
there are other users left. Again, rmfb racing against other ioctls
isn't a real problem, userspace is allowed to shoot its foot.
This patch just prepares this by moving the modeset locks to nest
within fpriv->fbs_lock. Now the distinction between the fbs_lock and
the device-global fb_lock is clear, since we need to hold the fbs_lock
outside of any modeset_locks in fb_release.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since otherwise looking and reference-counting around
drm_framebuffer_lookup will be an unmanageable mess. With this change,
an object can either be found in the idr and will stay around once we
incremented the reference counter. Or it will be gone for good and
can't be looked up using its id any more.
Atomicity is guaranteed by the dev->mode_config.fb_lock. The
newly-introduce fpriv->fbs_lock looks a bit redundant, but the next
patch will shuffle the locking order between these two locks and all
the modeset locks taken in modeset_lock_all, so we'll need it.
Also, since userspace could do really funky stuff and race e.g. a
getresources with an rmfb, we need to make sure that the kernel
doesn't fall over trying to look-up an inexistent fb, or causing
confusion by having two fbs around with the same id. Simply reset the
framebuffer id to 0, which marks it as reaped. Any lookups of that id
will fail, so the object is really gone for good from userspace's pov.
Note that we still need to protect the "remove framebuffer from all
use-cases" and the final unreference with the modeset-lock, since most
framebuffer use-sites don't implement proper reference counting yet.
We can only lift this once _all_ users are converted.
With this change, two references are held on alife, but unused
framebuffers:
- The reference for the idr lookup, created in this patch.
- For user-created framebuffers the fpriv->fbs reference, for
driver-private fbs the driver is supposed to hold it's own last
reference.
Note that the dev->mode_config.fb_list itself does _not_ hold a
reference onto the framebuffers (this list is essentially only used
for debugfs files). Hence if there's anything left there when the
driver has cleaned up all it's modeset resources, this is a ref-leak.
WARN about it.
Now we only need to fix up all other places to properly reference
count framebuffers.
v2: Fix spelling fail in a comment spotted by Rob Clark.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We have two classes of framebuffer
- Created by the driver (atm only for fbdev), and the driver holds
onto the last reference count until destruction.
- Created by userspace and associated with a given fd. These
framebuffers will be reaped when their assoiciated fb is closed.
Now these two cases are set up differently, the framebuffers are on
different lists and hence destruction needs to clean up different
things. Also, for userspace framebuffers we remove them from any
current usage, whereas for internal framebuffers it is assumed that
the driver has done this already.
Long story short, we need two different ways to cleanup such drivers.
Three functions are involved in total:
- drm_framebuffer_remove: Convenience function which removes the fb
from all active usage and then drops the passed-in reference.
- drm_framebuffer_unregister_private: Will remove driver-private
framebuffers from relevant lists and drop the corresponding
references. Should be called for driver-private framebuffers before
dropping the last reference (or like for a lot of the drivers where
the fbdev is embedded someplace else, before doing the cleanup
manually).
- drm_framebuffer_cleanup: Final cleanup for both classes of fbs,
should be called by the driver's ->destroy callback once the last
reference is gone.
This patch just rolls out the new interfaces and updates all drivers
(by adding calls to drm_framebuffer_unregister_private at all the
right places)- no functional changes yet. Follow-on patches will move
drm core code around and update the lifetime management for
framebuffers, so that we are no longer required to keep framebuffers
alive by locking mode_config.mutex.
I've also updated the kerneldoc already.
vmwgfx seems to again be a bit special, at least I haven't figured out
how the fbdev support in that driver works. It smells like it's
external though.
v2: The i915 driver creates another private framebuffer in the
load-detect code. Adjust its cleanup code, too.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And replace all fb lookups with it. Also add a WARN to
drm_mode_object_find since that is now no longer the blessed interface
to look up an fb. And add kerneldoc to both functions.
This only updates all callsites, but immediately drops the acquired
refence again. Hence all callers still rely on the fact that a mode fb
can't disappear while they're holding the struct mutex. Subsequent
patches will instate proper use of refcounts, and then rework the rmfb
and unref code to no longer serialize fb destruction with the
mode_config lock. We don't want that since otherwise a compositor
might end up stalling for a few frames in rmfb.
v2: Don't use kref_get_unless_zero - Greg KH doesn't like that kind of
interface.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Well, at least step 1. The goal here is that framebuffer objects can
survive outside of the mode_config lock, with just a reference held
as protection. The first step to get there is to introduce a special
fb_lock which protects fb lookup, creation and destruction, to make
them appear atomic.
This new fb_lock can nest within the mode_config lock. But the idea is
(once the reference counting part is completed) that we only quickly
take that fb_lock to lookup a framebuffer and grab a reference,
without any other locks involved.
vmwgfx is the only driver which does framebuffer lookups itself, also
wrap those calls to drm_mode_object_find with the new lock.
Also protect the fb_list walking in i915 and omapdrm with the new lock.
As a slight complication there's also the list of user-created fbs
attached to the file private. The problem now is that at fclose() time
we need to walk that list, eventually do a modeset call to remove the
fb from active usage (and are required to be able to take the
mode_config lock), but in the end we need to grab the new fb_lock to
remove the fb from the list. The easiest solution is to add another
mutex to protect this per-file list.
Currently that new fbs_lock nests within the modeset locks and so
appears redudant. But later patches will switch around this sequence
so that taking the modeset locks in the fb destruction path is
optional in the fastpath. Ultimately the goal is that addfb and rmfb
do not require the mode_config lock, since otherwise they have the
potential to introduce stalls in the pageflip sequence of a compositor
(if the compositor e.g. switches to a fullscreen client or if it
enables a plane). But that requires a few more steps and hoops to jump
through.
Note that framebuffer creation/destruction is now double-protected -
once by the fb_lock and in parts by the idr_lock. The later would be
unnecessariy if framebuffers would have their own idr allocator. But
that's material for another patch (series).
v2: Properly initialize the fb->filp_head list in _init, otherwise the
newly added WARN to check whether the fb isn't on a fpriv list any
more will fail for driver-private objects.
v3: Fixup two error-case unlock bugs spotted by Richard Wilbur.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
->cursor_move uses mostly the same facilities in drivers as
->cursor_set, so pretty much nothing to fix up:
- ast/gma500/i915: They all use per-crtc registers to update the
cursor position. ast again touches the global cursor cache, but
that's ok since there's only one crtc.
- nouveau: nv50+ is again special, updates happen through the per-crtc
channel (without pushbufs), so it's not protected by the new evo
lock introduced earlier. But since this channel is per-crtc, we
should be fine anyway.
- radeon: A bit a mess: avivo asics need a workaround when both output
pipes are enabled, which means it'll access the crtc list. Just
reading that flag is ok though as long as radeon _always_ grabs all
locks when changing the crtc configuration. Which means with the
current scheme it cannot do an optimized modeset which only locks
the relevant crtcs. This can be fixed though by introducing a bit of
global state with separate locks and ensure in the modeset code that
the cursor will be updated appropriately when enabling the 2nd pipe
(on affected asics).
- vmwgfx: I still don't understand what it's doing exactly, so apply
the same trick for now.
v2: Fixup unlocking for the error cases, spotted by Richard Wilbur.
v3: Another error-case fixup.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
First convert ->cursor_set to only take the crtc lock, since that
seems to be the function with the least amount of state - the core
ioctl function doesn't check anything which can change at runtime, so
we don't have any object lifetime issues to contend.
The only thing which is important is that the driver's implementation
doesn't touch any state outside of that single crtc which is not yet
properly protected by other locking:
- ast: access the global ast->cache_kmap. Luckily we only have on crtc
on this driver, so this is fine. Add a comment.
- gma500: calls gma_power_begin|and and psb_gtt_pin|unpin, both which
have their own locking to protect their state. Everything else is
crtc-local.
- i915: touches a bit of global gem state, all protected by the One
Lock to Rule Them All (dev->struct_mutex).
- nouveau: Pre-nv50 is all nice, nv50+ uses the evo channels to queue
up all display changes. And some of these channels are device
global. But this is fine now since the previous patch introduced an
evo channel mutex.
- radeon: Uses some indirect register access for cursor updates, but
with the previous patches to protect these indirect 2-register
access patterns with a spinlock, this should be fine now, too.
- vmwgfx: I have no idea how that works - update_cursor_position
doesn't take any per-crtc argument and I haven't figured out any
other place where this could be set in some form of a side-channel.
But vmwgfx definitely has more than one crtc (or at least can
register more than one), so I have no idea how this is supposed to
not fail with the current code already. Hence take the easy way out
and simply acquire all locks (which requires dropping the crtc lock
the core acquired for us). That way it's not worse off for
consistency than the old code.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
*drumroll*
The basic idea is to protect per-crtc state which can change without
touching the output configuration with separate mutexes, i.e. all the
input side state to a crtc like framebuffers, cursor settings or plane
configuration. Holding such a crtc lock gives a read-lock on all the
other crtc state which can be changed by e.g. a modeset.
All non-crtc state is still protected by the mode_config mutex.
Callers that need to change modeset state of a crtc (e.g. dpms or
set_mode) need to grab both the mode_config lock and nested within any
crtc locks.
Note that since there can only ever be one holder of the mode_config
lock we can grab the subordinate crtc locks in any order (if we need
to grab more than one of them). Lockdep can handle such nesting with
the mutex_lock_nest_lock call correctly.
With this functions that only touch connectors/encoders but not crtcs
only need to take the mode_config lock. The biggest such case is the
output probing, which means that we can now pageflip and move cursors
while the output probe code is reading an edid.
Most cases neatly fall into the three buckets:
- Only touches connectors and similar output state and so only needs
the mode_config lock.
- Touches the global configuration and so needs all locks.
- Only touches the crtc input side and so only needs the crtc lock.
But a few cases that need special consideration:
- Load detection which requires a crtc. The mode_config lock already
prevents a modeset change, so we can use any unused crtc as we like
to do load detection. The only thing to consider is that such
temporary state changes don't leak out to userspace through ioctls
that only take the crtc look (like a pageflip). Hence the load
detect code needs to grab the crtc of any output pipes it touches
(but only if it touches state used by the pageflip or cursor
ioctls).
- Atomic pageflip when moving planes. The first case is sane hw, where
planes have a fixed association with crtcs - nothing needs to be
done there. More insane^Wflexible hw needs to have plane->crtc
mapping which is separately protect with a lock that nests within
the crtc lock. If the plane is unused we can just assign it to the
current crtc and continue. But if a plane is already in use by
another crtc we can't just reassign it.
Two solution present themselves: Either go back to a slow-path which
takes all modeset locks, potentially incure quite a hefty delay. Or
simply disallowing such changes in one atomic pageflip - in general
the vblanks of two crtcs are not synced, so there's no sane way to
atomically flip such plane changes accross more than one crtc. I'd
heavily favour the later approach, going as far as mandating it as
part of the ABI of such a new a nuclear pageflip.
And if we _really_ want such semantics, we can always get them by
introducing another pageflip mutex between the mode_config.mutex and
the individual crtc locks. Pageflips crossing more than one crtc
would then need to take that lock first, to lock out concurrent
multi-crtc pageflips.
- Optimized global modeset operations: We could just take the
mode_config lock and then lazily lock all crtc which are affected by
a modeset operation. This has the advantage that pageflip could
continue unhampered on unaffected crtc. But if e.g. global resources
like plls need to be reassigned and so affect unrelated crtcs we can
still do that - nested locking works in any order.
This patch just adds the locks and takes them in drm_modeset_lock_all,
no real locking changes yet.
v2: Need to initialize the new lock in crtc_init and lock it righ
away, for otherwise the modeset_unlock_all below will try to unlock a
not-locked mutex.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is the first step towards introducing the new modeset locking
scheme. The plan is to put helper functions into place at all the
right places step-by-step, so that the final patch to switch on the
new locking scheme doesn't need to touch every single driver.
This helper here will serve as the shotgun solutions for all places
where a more fine-grained locking isn't (yet) implemented.
v2: Fixup kerneldoc for unlock_all.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With refcounting we need to adjust framebuffer refcounts at each
callsite - much easier to do if they all call the same little helper
function.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Some drivers don't have real ->create_handle callbacks.
- cirrus/ast/mga200: Returns either 0 or -EINVAL.
- udl: Didn't even bother with a callback, leading to a nice
userspace-triggerable OOPS.
- vmwgfx: This driver bothered with an implementation to return 0 as
the handle (which is the canonical no-obj gem handle).
All have in common that ->create_handle doesn't really make too much
sense for them - that ioctl is used only for seamless fb takeover in
the radeon/nouveau/i915 ddx drivers. So allow drivers to not implement
this and return a consistent -ENODEV.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Doing this within the fb->destroy callback leads to a locking
nightmare. And all other drm drivers that restore the fbcon do
it in lastclose, too.
With this adjustments all fb->destroy callbacks optionally drop
references to any gem objects used as backing storage, call
drm_framebuffer_cleanup and then kfree the struct. Which nicely
simplifies the locking for framebuffer unreferencing and freeing,
since this doesn't require that we hold the mode_config lock. A
slight exception is the vmwgfx surface backed framebuffer, it also
calls drm_master_put and removes the object from a device-private
framebuffer list. Both seem to have solid locking in place already.
Conclusion is that now it is no longer required to hold the
mode_config lock while freeing a framebuffer.
v2: Drop the corresponding mutex_lock WARN check from
drm_framebuffer_unreference.
v3: Use just the mode_config lock not modeset_lock_all, due to patch
reordering.
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And do a quick pass to adjust them to the last few (years?) of changes
...
This time actually compile-tested ;-)
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- config_cleanup was confused: It claimed that callers need to hold
the modeset lock, but the connector|encoder_cleanup helpers grabbed
that themselves (note that crtc_cleanup did _not_ grab the modeset
lock). Which resulted in all drivers _not_ hodling the lock. Since
this is for single-threaded cleanup code, drop the requirement from
docs and also drop the lock_grabbing from all _cleanup functions.
- Kill the LOCKING section in the doctype, since clearly we're not
good enough to keep them up-to-date. And misleading locking
documentation is worse than useless (see e.g. the comment in the
vmgfx driver about the cleanup mess). And since for most functions
the very first line either grabs the lock or has a WARN_ON(!locked)
the documentation doesn't really add anything.
- Instead put in some effort into explaining the only two special
cases a bit better: config_init and config_cleanup are both called
from single-threaded setup/teardown code, so don't do any locking.
It's the driver's job though to enforce this.
- Where lacking, add a WARN_ON(!is_locked). Not many places though,
since locking around fbdev setup/teardown is through-roughly screwed
up, and so will break almost every single WARN annotation I've tried
to add.
- Add a drm_modeset_is_locked helper - the Grate Modset Locking Rework
will use the compiler to assist in the big reorg by renaming the
mode lock, so start encapsulating things. Unfortunately this ended
up in the "wrong" header file since it needs the definition of
struct drm_device.
v2: Drop most WARNS again - we hit them all over the place, mostly in
the setup and teardown sequences. And trying to fix it up leads to
nice deadlocks, since the locking in the setup code is really
inconsistent.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Replace references to and remove the connector property fxns, which
have been superseded with the more general object property fxns:
+ drm_connector_attach_property -> drm_object_attach_property
+ drm_connector_property_set_value -> drm_object_property_set_value
+ drm_connector_property_get_value -> drm_object_property_get_value
Signed-off-by: Rob Clark <rob@ti.com>
This can help drivers to make somewhat intelligent decisions in their
->detect callback: If the connector is hpd capable and in the unknown
state, the driver needs to force a full detect cycle. Otherwise it
could just (if it chooses so) to update the connector state from it's
hpd handler directly, and always return that in the ->detect callback.
Atm only drm/i915 calls drm_mode_config_reset at resume time, so other
drivers would need to add that call first before using this facility.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_property_create_blob() could return NULL in which case NULL pointer
dereference error (on connector->edid_blob_ptr) is possible. Return if
connector->edid_blob_ptr is NULL.
Fixes the following smatch error:
drivers/gpu/drm/drm_crtc.c:3186 drm_mode_connector_update_edid_property()
error: potential null dereference 'connector->edid_blob_ptr'.
(drm_property_create_blob returns null)
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
kfree() on a NULL input is a no-op. Hence remove the check.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
In case of a blob property drm_property_change_is_valid() can't
tell whether the change is valid or not. So just return true
for all blob properties, and leave it up to someone else to
check it.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Pull drm merge (part 1) from Dave Airlie:
"So first of all my tree and uapi stuff has a conflict mess, its my
fault as the nouveau stuff didn't hit -next as were trying to rebase
regressions out of it before we merged.
Highlights:
- SH mobile modesetting driver and associated helpers
- some DRM core documentation
- i915 modesetting rework, haswell hdmi, haswell and vlv fixes, write
combined pte writing, ilk rc6 support,
- nouveau: major driver rework into a hw core driver, makes features
like SLI a lot saner to implement,
- psb: add eDP/DP support for Cedarview
- radeon: 2 layer page tables, async VM pte updates, better PLL
selection for > 2 screens, better ACPI interactions
The rest is general grab bag of fixes.
So why part 1? well I have the exynos pull req which came in a bit
late but was waiting for me to do something they shouldn't have and it
looks fairly safe, and David Howells has some more header cleanups
he'd like me to pull, that seem like a good idea, but I'd like to get
this merge out of the way so -next dosen't get blocked."
Tons of conflicts mostly due to silly include line changes, but mostly
mindless. A few other small semantic conflicts too, noted from Dave's
pre-merged branch.
* 'drm-next' of git://people.freedesktop.org/~airlied/linux: (447 commits)
drm/nv98/crypt: fix fuc build with latest envyas
drm/nouveau/devinit: fixup various issues with subdev ctor/init ordering
drm/nv41/vm: fix and enable use of "real" pciegart
drm/nv44/vm: fix and enable use of "real" pciegart
drm/nv04/dmaobj: fixup vm target handling in preparation for nv4x pcie
drm/nouveau: store supported dma mask in vmmgr
drm/nvc0/ibus: initial implementation of subdev
drm/nouveau/therm: add support for fan-control modes
drm/nouveau/hwmon: rename pwm0* to pmw1* to follow hwmon's rules
drm/nouveau/therm: calculate the pwm divisor on nv50+
drm/nouveau/fan: rewrite the fan tachometer driver to get more precision, faster
drm/nouveau/therm: move thermal-related functions to the therm subdev
drm/nouveau/bios: parse the pwm divisor from the perf table
drm/nouveau/therm: use the EXTDEV table to detect i2c monitoring devices
drm/nouveau/therm: rework thermal table parsing
drm/nouveau/gpio: expose the PWM/TOGGLE parameter found in the gpio vbios table
drm/nouveau: fix pm initialization order
drm/nouveau/bios: check that fixed tvdac gpio data is valid before using it
drm/nouveau: log channel debug/error messages from client object rather than drm client
drm/nouveau: have drm debugging macros build on top of core macros
...
Convert #include "..." to #include <path/...> in drivers/gpu/.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Dave Airlie <airlied@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Dave Jones <davej@redhat.com>
Remove redundant DRM UAPI header #inclusions from drivers/gpu/.
Remove redundant #inclusions of core DRM UAPI headers (drm.h, drm_mode.h and
drm_sarea.h). They are now #included via drmP.h and drm_crtc.h via a preceding
patch.
Without this patch and the patch to make include the UAPI headers from the core
headers, after the UAPI split, the DRM C sources cannot find these UAPI headers
because the DRM code relies on specific -I flags to make #include "..." work
on headers in include/drm/ - but that does not work after the UAPI split without
adding more -I flags.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Dave Airlie <airlied@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Dave Jones <davej@redhat.com>
For drivers that can support rotated scanout, the extra parameter
checking in drm-core, while nice, tends to get confused. To solve
this drivers can set the crtc or plane invert_dimensions field so
that the dimension checking takes into account the rotation that
the driver is performing.
v1: original
v2: remove invert_dimensions from plane, at Ville's suggestion.
Userspace can give rotated src coordinates, so invert_dimensions
is not required for planes.
Signed-off-by: Rob Clark <rob@ti.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This simplifies drm fb lifetime, and if the crtc/plane needs to hold
a ref to the fb when disabling a pipe until the next vblank, this
avoids the need to make disabling an overlay synchronous. This is a
problem that shows up when userspace is using a drm plane to
implement a hw cursor.. making overlay disable synchronous causes
a performance problem when x11 is rapidly enabling/disabling the
hw cursor. But not making it synchronous opens up a race condition
for crashing if userspace turns around and immediately deletes the
fb. Refcnt'ing the fb makes it possible to solve this problem.
v1: original
v2: add drm_framebuffer_remove() which is called in all paths where
fb->funcs->destroy() was directly called before. This cleans
up the CRTCs/planes that the fb was attached to. You should
only directly use drm_framebuffer_unreference() if you are also
using drm_framebuffer_reference() to keep a ref to the fb.
v3: add comment explaining the fb refcount
v4: remove duplicate 'list_del(&fb->filp_head)'
[airlied: v4.1: fix local rejection]
Signed-off-by: Rob Clark <rob@ti.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
As during the plane cleanup, we wish to disable the hardware and
so may modify state on the associated CRTC, that CRTC must continue to
exist until we are finished.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54101
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: stable@vger.kernel.org
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-by: lu hua <huax.lu@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)
iQEcBAABAgAGBQJQX7MuAAoJEHm+PkMAQRiG0h0IAJURkrMCAQUxA+Ik66ReH89s
LQcVd0U9uL4UUOi7f5WR64Vf9Cfu6VVGX9ZKSvjpNskvlQaUQPMIt4pMe6g4X4dI
u0bApEy4XZz3nGabUAghIU8jJ8cDmhCG6kPpSiS7pi7KHc0yIa4WFtJRrIpGaIWT
xuK38YOiOHcSDRlLyWZzainMncQp/ixJdxnqVMTonkVLk0q0b84XzOr4/qlLE5lU
i+TsK3PRKdQXgvZ4CebL+srPBwWX1dmgP3VkeBloQbSSenSeELICbFWavn2ml+sF
GXi4dO93oNquL/Oy5SwI666T4uNcrRPaS+5X+xSZgBW/y2aQVJVJuNZg6ZP/uWk=
=0v2l
-----END PGP SIGNATURE-----
Merge tag 'v3.6-rc7' into drm-intel-next-queued
Manual backmerge of -rc7 to resolve a silent conflict leading to
compile failure in drivers/gpu/drm/i915/intel_hdmi.c.
This is due to the bugfix in -rc7:
commit b98b601672
Author: Wang Xingchao <xingchao.wang@intel.com>
Date: Thu Sep 13 07:43:22 2012 +0800
drm/i915: HDMI - Clear Audio Enable bit for Hot Plug
Since this code moved around a lot in -next git put that snippet at
the wrong spot. I've tried to fix this by making the conflict explicit
by merging a version for next with:
commit 3cce574f01
Author: Wang Xingchao <xingchao.wang@intel.com>
Date: Thu Sep 13 11:19:00 2012 +0800
drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
But that failed to solve the entire problem. To avoid pushing out
further -nightly branch to our QA where this is broken, do the
backmerge and manually add the stuff git adds to -next from the patch
in -fixes.
Note that this doesn't show up in git's merge diff (and hence is also
not handled by git rerere), which adds to the reasons why I'd like to
fix this with a verbose backmerge. The git merge diff only shows a
bunch of trivial conflicts of the "code changed in lines next to each
another" kind.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The rest of the code uses stdint types, so use them in
drm_property_change_is_valid() as well.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The omapdrm driver uses this for setting per-overlay rotation. It
is likely also useful for setting YUV->RGB colorspace conversion
matrix, etc.
Signed-off-by: Rob Clark <rob@ti.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
A bitmask property is similar to an enum. The enum value is a bit
position (0-63), and valid property values consist of a mask of
zero or more of (1 << enum_val[n]).
[airlied: 1LL -> 1ULL]
Signed-off-by: Rob Clark <rob@ti.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Drivers for hardware without gamma support should not be forced to
implement a no-op gamma set operation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The i915 driver needs this for the rotation and overscan compensation
properties. Other drivers might need this too.
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This way, we don't need to count every time, so we're a little bit
faster and code is a little bit smaller.
Change suggested by Ville Syrjälä.
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
In the future, we may want to kill the internal functions:
- drm_connector_attach_property
- drm_connector_property_set_value
- drm_connector_property_get_value
It seems the IOCTL drm_mode_connector_property_set_ioctl will have to live, but
we may change libdrm to not use it anymore, if we want.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Useless for connector properties (since they already have their own
ioctls), but useful when we add properties to CRTCs, planes and other
objects.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
For now, only connectors have it. In the future, all objects that need
properties should use it. Since the structure is referenced inside
struct drm_mode_object, we will be able to deal with object properties
without knowing the real type of the object.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Also return void instead of int. We have more than 100 callers and
no one checks for the return value.
If this function fails the property won't be exposed by the get/set
ioctls, but we should probably survive. If this starts happening,
the solution will be to increase DRM_CONNECTOR_MAX_PROPERTY and
recompile the Kernel.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Move code from drm_mode_connector_property_set_ioctl to a new
function, so we can reuse this code when we add crtc properties.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Tested-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)
iQEcBAABAgAGBQJPpvY9AAoJEHm+PkMAQRiGpEoIAJgbu+Y8gITnBK/wh9O6zy3S
5jie5KK4YWdbJsvO58WbNr3CyVIwGIqQ2dUZLiU59aBVLarlGw8xor0MmW+cZwhp
6fBHaf0qDYAV0MZjD+mnnExOiCRyISa2lPmsfu9dAWywh5KGe6/oAP6/qcXIyok3
KZyl3qQf4ENpaZPHwZPXCEkUvtuyHgNiszN+QXEadA3s19Ot4VGe9A3VGw+GNrSm
JqFIq3acQAbKa5BYaqf7TQC02v2FI7//eqt6QHxTqbE6a7LGbTvLfX3HlJ2mnfqa
1R6QHhM4y4OZDHbaMT2raHZ8WuLXzhehJzhP8Co7AHFOKwVKOb5XbcUr2RrukMU=
=HkMd
-----END PGP SIGNATURE-----
Merge tag 'v3.4-rc6' into drm-intel-next
Conflicts:
drivers/gpu/drm/i915/intel_display.c
Ok, this is a fun story of git totally messing things up. There
/shouldn't/ be any conflict in here, because the fixes in -rc6 do only
touch functions that have not been changed in -next.
The offending commits in drm-next are 14415745b2..1fa611065 which
simply move a few functions from intel_display.c to intel_pm.c. The
problem seems to be that git diff gets completely confused:
$ git diff 14415745b2..1fa611065
is a nice mess in intel_display.c, and the diff leaks into totally
unrelated functions, whereas
$git diff --minimal 14415745b2..1fa611065
is exactly what we want.
Unfortunately there seems to be no way to teach similar smarts to the
merge diff and conflict generation code, because with the minimal diff
there really shouldn't be any conflicts. For added hilarity, every
time something in that area changes the + and - lines in the diff move
around like crazy, again resulting in new conflicts. So I fear this
mess will stay with us for a little longer (and might result in
another backmerge down the road).
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These can all be trigged from userspace if you pass the right values.
v2: rebase on later kernel.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The error handling code w.r.t. idr usage looks inconsistent.
In the case of drm_mode_object_get() and drm_ctxbitmap_next() the error
handling is also incomplete.
Unify the code to follow the same pattern always.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Perform some basic sanity check on some of the parameters in
drm_mode_fb_cmd2.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
These functions return the chroma subsampling factors for the specified
pixel format.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This function returns the bytes per pixel value based on the pixel
format and plane index.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
There will be a need for this function in drm_crtc.c later. This
avoids making drm_crtc.c depend on drm_crtc_helper.c.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Free event and restore event_space only when page_flip->flags has
DRM_MODE_PAGE_FLIP_EVENT if page_flip() is failed.
Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
In order to satisfy all the various Kconfig options between
USB and DRM, we need to split the USB code out into a separate module
and export symbols to it.
This fixes build problems in -next reported by sfr.
Signed-off-by: Dave Airlie <airlied@redhat.com>
In order to get correct ordering at hot-unplug for userspace,
we need to tear down all the sysfs bits at the correct time.
This adds a helper to allow drivers to remove the sysfs nodes
for all connectors.
Signed-off-by: Dave Airlie <airlied@redhat.com>
The blob property data is always allocated immediately after the object
header. No need for the extra indirection when accessing it, just use
a flexible array member.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Check drm_mode_object_get() return value everywhere.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_crtc_convert_umode() and drm_crtc_convert_to_umode() are never
used outside drm_crtc.c, so make them static. Also make the input
mode structure const for both functions.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Change drm_mode_attachmode_crtc() to take an "all or nothing" approach.
If an error is returned, there are no side effects visible.
Also change the function to always duplicate the mode passed in.
Also change the function to not give up when it finds the first
connector without and encoder.
A simpler approach would be to just remove the function completely as
it's unused currently.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Make sure the requested CRTC viewport fits inside the
framebuffer.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The internal mode representation drm_display_mode uses signed data
types. When converting the user mode to internal representation,
check that the unsigned values don't overflow the signed datatypes.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The mode passed to the .set_config() hook was never freed. The drivers
will make a copy of the mode, so simply free it when done.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_mode_attachmode() always returns 0. Change the return type to void.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The crtc x/y panning coordinates are stored as signed integers
internally. The user provides them as unsigned, so we should check
that the user provided values actually fit in the internal datatypes.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
When converting from a drm_display_mode to drm_mode_modeinfo, print a
warning if the the timings values don't fit into the __u16 datatype.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
When doing a mode set with the special fb id -1, reject the mode set if
no fb is currently bound to the crtc.
Also remove the pointless list traversal to find the current crtc based
on the current crtc :)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Creating a range property is a common pattern, so create
a convenience function for this and use it where appropriate.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Creating an enum property is a common pattern, so create
a convenience function for this and use it where appropriate.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Several comments above functions say that the caller must hold the
mode_config lock, but the functions take the lock themselves. Fix
the comments.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_mode_crtc_set_gamma_size returns boolean true for success
and false for failure. This is not very kernel conform, so
change it to return 0 for success and a propert error code
otherwise. Noone checks the return value, so no users have to
be fixed.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Modes are created using drm_mode_create which does a
drm_mode_object_get, so use drm_mode_destroy in drm_mode_remove
which does a drm_mode_object_put.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drm_mode_config_init initializes the idr with idr_init, so
add the missing counterparts in drm_mode_config_cleanup.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
In cases where the scanout hw is sufficiently similar between "overlay"
and traditional crtc layers, it might be convenient to allow the driver
to create internal drm_plane helper objects used by the drm_crtc
implementation, rather than duplicate code between the plane and crtc.
A private plane is not exposed to userspace.
Signed-off-by: Rob Clark <rob@ti.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Since plane->fb and plane->crtc are set in drm_mode_setplane()
after update_plane(), They should be cleared after disable().
Signed-off-by: Rob Clark <rob@ti.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Merge in the upstream tree to bring in the mainline fixes.
Conflicts:
drivers/gpu/drm/exynos/exynos_drm_fbdev.c
drivers/gpu/drm/nouveau/nouveau_sgdma.c
Otherwise each driver would need to keep the information inside
their own framebuffer object structure. Also add offsets[]. BOs
on the other hand are driver specific, so those can be kept in
driver specific structures.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Help drivers a little by guaranteeing that crtc_x+crtc_w and
crtc_y+crtc_h don't overflow.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Make sure the source coordinates stay within the buffer.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
These are the only indication to user space that the plane was disabled.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Several pointers and casts were missing __user annotations.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Unlock the mode_config mutex if drm_plane_init() fails.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Each of these error messages can be caused by a broken or malicious
userspace wanting to spam the dmesg with useless info. They're really
not worthy of DRM_DEBUG statements either; those are generally only
useful during bringup of new hardware or versions, and ought to be
removed before going upstream anyway.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Alex Deucher <alexdeucher@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
names, especially for the RGB formats. Component order and byte order are
now strictly specified for each format.
The RGB format naming follows a convention where the components names
and sizes are listed from left to right, matching the order within a
single pixel from most significant bit to least significant bit.
The YUV format names vary more. For the 4:2:2 packed formats and 2
plane formats use the fourcc. For the three plane formats the
name includes the plane order and subsampling information using the
standard subsampling notation. Some of those also happen to match
the official fourcc definition.
The fourccs for for all the RGB formats and some of the YUV formats
I invented myself. The idea was that looking at just the fourcc you
get some idea what the format is about without having to decode it
using some external reference.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
There is a potential integer overflow in drm_mode_dirtyfb_ioctl()
if userspace passes in a large num_clips. The call to kmalloc would
allocate a small buffer, and the call to fb->funcs->dirty may result
in a memory corruption.
Reported-by: Haogang Chen <haogangchen@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
To properly support the various plane formats supported by different
hardware, the kernel must know the pixel format of a framebuffer object.
So add a new ioctl taking a format argument corresponding to a fourcc
name from the new drm_fourcc.h header file. Implement the fb creation
hooks in terms of the new mode_fb_cmd2 using helpers where the old
bpp/depth values are needed.
v2: create DRM specific fourcc header file for sharing with libdrm etc
v3: fix rebase failure and use DRM fourcc codes in intel_display.c and
update commit message
v4: make fb_cmd2 handle field into an array for multi-object formats
pull in Ville's fix for the memcpy in drm_plane_init
apply Ville's cleanup to zero out fb_cmd2 arg in drm_mode_addfb
v5: add 'flags' field for interlaced support (from Ville)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Planes are a bit like half-CRTCs. They have a location and fb, but
don't drive outputs directly. Add support for handling them to the core
KMS code.
v2: fix ABI of get_plane - move format_type_ptr to the end
v3: add 'flags' field for interlaced support (from Ville)
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Reviewed-by: Rob Clark <rob.clark@linaro.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
* 'drm-fixes' of git://people.freedesktop.org/~airlied/linux: (40 commits)
vmwgfx: Snoop DMA transfers with non-covering sizes
vmwgfx: Move the prefered mode first in the list
vmwgfx: Unreference surface on cursor error path
vmwgfx: Free prefered mode on error path
vmwgfx: Use pointer return error codes
vmwgfx: Fix hw cursor position
vmwgfx: Infrastructure for explicit placement
vmwgfx: Make the preferred autofit mode have a 60Hz vrefresh
vmwgfx: Remove screen object active list
vmwgfx: Screen object cleanups
drm/radeon/kms: consolidate GART code, fix segfault after GPU lockup V2
drm/radeon/kms: don't poll forever if MC GDDR link training fails
drm/radeon/kms: fix DP setup on TRAVIS bridges
drm/radeon/kms: set HPD polarity in hpd_init()
drm/radeon/kms: add MSI module parameter
drm/radeon/kms: Add MSI quirk for Dell RS690
drm/radeon/kms: Add MSI quirk for HP RS690
drm/radeon/kms: split MSI check into a separate function
vmwgfx: Reinstate the update_layout ioctl
drm/radeon/kms: always do extended edid probe
...
This will allow us to attach various properties specific to virtual
monitors in the future.
Note that we don't export an EDID property for "Virtual" connectors.
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
It is left out the code to decrease the number of connector and encoder
to the cleanup functions.
Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Sometimes we could be controlling a device (such as an NVIDIA Tesla) that
has no crtcs/encoders/connectors.
One could argue that the driver should unset DRIVER_MODESET in this case,
but that changes a whole heap of the DRM's other behaviours, and it's much
easier to just be a modesetting driver without any outputs.
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The DRM_IOCTL_MODE_GETRESOURCES ioctl just returns bogus framebuffers.
That is because the framebuffers for each file are in the filp_head
member of struct drm_framebuffer, not in the head member.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Dave Airlie <airlied@redhat.com>
