In general having this can't hurt, and the atomic helpers will need
it to be able to reset the state objects properly. The overall idea
is to reset in the order pixels flow, so planes -> crtcs ->
encoders -> connectors.
v2: Squash in fixup from Ville to correctly deference struct drm_plane
instead of drm_crtc when walking the plane list. Fixes an oops in
driver init and resume.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Atomic implemenations for legacy ioctls must be able to drop locks.
Which doesn't cause havoc since we only do that while constructing
the new state, so no driver or hardware state change has happened.
The only troubling bit is the fb refcounting the core does - if
someone else has snuck in then it might potentially unref an
outdated framebuffer. To fix that move the old_fb temporary storage
into struct drm_plane for all ioctls, so that the atomic helpers can
update it.
v2: Fix up the error case handling as suggested by Matt Roper and just
grab locks uncoditionally - there's no point in optimizing the locking
for when userspace gets it wrong.
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
So drivers using the atomic interfaces expect that they can acquire
additional locks internal to the driver as-needed. Examples would be
locks to protect shared state like shared display PLLs.
Unfortunately the legacy ioctls assume that all locking is fully done
by the drm core. Now for those paths which grab all locks we already
have to keep around an acquire context in dev->mode_config. Helper
functions that implement legacy interfaces in terms of atomic support
can therefore grab this acquire contexts and reuse it.
The only interfaces left are the cursor and pageflip ioctls. So add
functions to grab the crtc lock these need using an acquire context
and preserve it for atomic drivers to reuse.
v2:
- Fixup comments&kerneldoc.
- Drop the WARNING from modeset_lock_all_crtcs since that can be used
in legacy paths with crtc locking.
v3: Fix a type on the kerneldoc Dave spotted.
Cc: Dave Airlie <airlied@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Somehow we've forgotten about this little bit of OCD.
Reviewed-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the atomic state we'll have an array of states for crtcs, planes
and connectors and need to be able to at them by their index. We
already have a drm_crtc_index function so add the missing ones for
planes and connectors.
If it later on turns out that the list walking is too expensive we can
add the index to the relevant modeset objects.
Rob Clark doesn't like the loops too much, but we can always add an
obj->idx parameter later on. And for now reiterating is actually safer
since nowadays we have hotpluggable connectors (thanks to DP MST).
v2: Fix embarrassing copypasta fail in kerneldoc and header
declarations, spotted by Matt Roper.
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>
i915.ko has a custom fbdev initialisation routine that aims to preserve
the current mode set by the BIOS, unless overruled by the user. The
user's wishes are determined by what, if any, mode is specified on the
command line (via the video= parameter). However, that command line mode
is first parsed by drm_fb_helper_initial_config() which is called after
i915.ko's custom initial_config() as a fallback method. So in order for
us to honour it, we need to move the cmdline parser earlier. If we
perform the connector cmdline parsing as soon as we initialise the
connector, that cmdline mode and forced status is then available even if
the fbdev helper is not compiled in or never called.
We also then expose the cmdline user mode in the connector mode lists.
v2: Rebase after connector->name upheaval.
v3: Adapt mga200 to look for the cmdline mode in the new place. Nicely
simplifies things while at that.
v4: Fix checkpatch.
v5: Select FB_CMDLINE to adapt to the changed fbdev patch.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73154
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v2)
Cc: dri-devel@lists.freedesktop.org
Cc: Julia Lemire <jlemire@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The current refcounting scheme is that the fb lookup idr also holds a
reference. This works out nicely bacause thus far we've always
explicitly cleaned up idr entries for framebuffers:
- Userspace fbs get removed in the rmfb ioctl or when the drm file
gets closed.
- Kernel fbs (for fbdev emulation) get cleaned up by the driver code
at module unload time.
But now i915 also reconstructs the bios fbs for a smooth transition.
And that fb is purely transitional and should get removed immmediately
once all crtcs stop using it. Of course if the i915 fbdev code decides
to reuse it as the main fbdev fb then it shouldn't be cleaned up, but
in that case the fbdev code will grab it's own reference.
The problem is now that we also want to register that takeover fb in
the idr, so that userspace can do a smooth transition (animated maybe
even!) itself. But currently we have no one who will clean up the idr
reference once that fb isn't useful any more, and so essentially leak
it.
Fix this by no longer holding a full fb reference for the idr, but
instead just have a weak reference using kref_get_unless_zero. But
that requires us to synchronize and clean up with the idr and fb_lock
in drm_framebuffer_free, so add that. It's a bit ugly that we have to
unconditionally grab the fb_lock, but without that someone might creep
through a race.
This leak was caught by the fb leak check in drm_mode_config_cleanup.
Originally the leak was introduced in
commit 46f297fb83
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Fri Mar 7 08:57:48 2014 -0800
drm/i915: add plane_config fetching infrastructure v2
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77511
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In my review of
commit 98f75de40e
Author: Rob Clark <robdclark@gmail.com>
Date: Fri May 30 11:37:03 2014 -0400
drm: add object property typ
I asked for a check to make sure that we never leak an fb from the
generic mode object lookup since those have completely different
lifetime rules. Rob added it, but outside of the idr mutex, which
means that our dereference of obj->type can already chase free'd
memory.
Somehow I didn't spot this, so fix this asap.
v2: Simplify the conditionals as suggested by Chris.
Cc: Rob Clark <robdclark@gmail.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Final feature pull for 3.17.
drm-intel-next-2014-07-25:
- Ditch UMS support (well just the config option for now)
- Prep work for future platforms (Sonika Jindal, Damien)
- runtime pm/soix fixes (Paulo, Jesse)
- psr tracking improvements, locking fixes, now enabled by default!
- rps fixes for chv (Deepak, Ville)
- drm core patches for rotation support (Ville, Sagar Kamble) - the i915 parts
unfortunately didn't make it yet
- userptr fixes (Chris)
- minimum backlight brightness (Jani), acked long ago by Matthew Garret on irc -
I've forgotten about this patch :(
QA is a bit unhappy about the DP MST stuff since it broke hpd testing a
bit, but otherwise looks sane. I've backmerged drm-next to resolve
conflicts with the mst stuff, which means the new tag itself doesn't
contain the overview as usual.
* tag 'drm-intel-next-2014-07-25-merged' of git://anongit.freedesktop.org/drm-intel: (75 commits)
drm/i915/userptr: Keep spin_lock/unlock in the same block
drm/i915: Allow overlapping userptr objects
drm/i915: Ditch UMS config option
drm/i915: respect the VBT minimum backlight brightness
drm/i915: extract backlight minimum brightness from VBT
drm/i915: Replace HAS_PCH_SPLIT which incorrectly lets some platforms in
drm/i915: Returning from increase/decrease of pllclock when invalid
drm/i915: Setting legacy palette correctly for different platforms
drm/i915: Avoid incorrect returning for some platforms
drm/i915: Writing proper check for reading of pipe status reg
drm/i915: Returning the right VGA control reg for platforms
drm/i915: Allowing changing of wm latencies for valid platforms
drm/i915: Adding HAS_GMCH_DISPLAY macro
drm/i915: Fix possible overflow when recording semaphore states.
drm/i915: Do not unmap object unless no other VMAs reference it
drm/i915: remove plane/cursor/pipe assertions from intel_crtc_disable
drm/i915: Reorder ctx unref on ppgtt cleanup
drm/i915/error: Check the potential ctx obj's vm
drm/i915: Fix printing proper min/min/rpe values in debugfs
drm/i915: BDW can also detect unclaimed registers
...
Daniel pointed out with hotplug that userspace could be trying to oops us
as root for lols, and that to be correct we shouldn't register the object
with the idr before we have fully set the connector object up.
His proposed solution was a lot more life changing, this seemed like a simpler
proposition to me, get the connector object id from the idr, but don't
register the object until the drm_connector_register callback.
The open question is whether the drm_mode_object_register needs a bigger lock
than just the idr one, but I can't see why it would, but I can be locking
challenged.
v2: fix bool noreg into sane - add comment.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Pull in drm-next with Dave's DP MST support so that I can merge some
conflicting patches which also touch the driver load sequencing around
interrupt handling.
Conflicts:
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_dp.c
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Added a property to enable user space to set aspect ratio.
This patch contains declaration of the property and code to create the
property.
v2: Thierry's review comments.
- Made aspect ratio enum generic instead of HDMI/CEA specfic
- Removed usage of temporary aspect_ratio variable
v3: Thierry's review comments.
- Fixed indentation
v4: Thierry's review comments.
- Return ENOMEM when property creation fails
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The drm_property_create_enum(), drm_property_create_bitmask() and
drm_property_create_range() contain the wrong name in the kerneldoc
comment. This is probably simply a copy/paste mistake.
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drm_rotation_simplify() can be used to eliminate unsupported rotation
flags. It will check if any unsupported flags are present, and if so
it will modify the rotation to an alternate form by adding 180 degrees
to rotation angle, and flipping the reflect x and y bits. The hope is
that this identity transform will eliminate the unsupported flags.
Of course that might not result in any more supported rotation, so
the caller is still responsible for checking the result afterwards.
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add a function to create a standards compliant rotation property.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make drm_property_create_bitmask() a bit more generic by allowing the
caller to specify which bits are in fact supported. This allows multiple
callers to use the same enum list, but still create different versions
of the same property with different list of supported bits.
v2: Populate values[] array as non-sparse
Make supported_bits 64bit
Fix up omapdrm call site (Rob)
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- 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>
