New helper function to set the preferred video mode. Can be called
after drm_add_modes_noedid if you don't want the largest supported
video mode be used by default.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This adds support for the Armada 510 display subsystem found on the
Marvell Dove devices. This IP is re-used across several different Marvell
SoCs with various tweaks, and this driver has been structured to allow
the other IPs to re-use the bulk of this code; further work in this area
is expected from interested parties.
This has been extensively tested on the SolidRun Cubox platform and
appears to work well there.
[airlied: update for api changes merged previous to this]
So drm was abusing device lifetimes, by having embedded device structures
in the minor and connector it meant that the lifetime of the internal drm
objects (drm_minor and drm_connector) were tied to the lifetime of the device
files in sysfs, so if something kept those files opened the current code
would kfree the objects and things would go downhill from there.
Now in reality there is no need for these lifetimes to be so intertwined,
especailly with hotplugging of devices where we wish to remove the sysfs
and userspace facing pieces before we can unwind the internal objects due
to open userspace files or mmaps, so split the objects out so the struct
device is no longer embedded and do what fbdev does and just allocate
and remove the sysfs inodes separately.
Signed-off-by: Dave Airlie <airlied@redhat.com>
This patch adds support for the pair of LCD controllers on the Marvell
Armada 510 SoCs. This driver supports:
- multiple contiguous scanout buffers for video and graphics
- shm backed cacheable buffer objects for X pixmaps for Vivante GPU
acceleration
- dual lcd0 and lcd1 crt operation
- video overlay on each LCD crt via DRM planes
- page flipping of the main scanout buffers
- DRM prime for buffer export/import
This driver is trivial to extend to other Armada SoCs.
Included in this commit is the core driver with no output support; output
support is platform and encoder driver dependent.
Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
The conflict in intel_drv.h tripped me up a bit since a patch in dinq
moves all the functions around, but another one in drm-next removes a
single function. So I'ev figured backing this into a backmerge would
be good.
i915_dma.c is just adjacent lines changed, nothing nefarious there.
Conflicts:
drivers/gpu/drm/i915/i915_dma.c
drivers/gpu/drm/i915/intel_drv.h
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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>
This allows us to use fewer bits in the mode structure, leaving room for
future work while allowing more stereo layouts types than we could have
ever dreamt of.
I also exposed the previously private DRM_MODE_FLAG_3D_MASK to set in
stone that we are using 5 bits for the stereo layout enum, reserving 32
values.
Even with that reservation, we gain 3 bits from the previous encoding.
The code adding the mandatory stereo modes needeed to be adapted as it was
relying or being able to or stereo layouts together.
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
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>
When using the frame packing and a single big framebuffer, some hardware
requires that we do everything like if we were scanning out the big
buffer itself. Let's instrument drm_mode_set_crtcinfo() to be able to do
this adjustement if the driver is asking for it.
v2: Use crtc_vtotal and multiply the clock by 2 instead of
reconstructing it (Ville Syrjälä)
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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>
Just like the various timings, make it possible to have a clock field
what we can tweak before giving it to hardware.
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 field is unused. Garbage collect it.
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 field was only accessed by the nouveau driver, but never set. So
concluded we can rid of this one.
Acked-by: Ben Skeggs <bskeggs@redhat.com>
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>
Just like with interlaced or double scan modes, make stereo modes a
per-connector opt-in to give a chance to driver authors to make it work
before enabling it.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
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 scanning out a stereo mode, the AVI infoframe vic field has to be
the underlyng 2D VIC. Before that commit, we weren't matching the CEA
mode because of the extra stereo flag and then were setting the VIC
field in the AVI infoframe to 0.
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>
HDMI 1.4a defines a few layouts that we'd like to expose. This commits
add new modeinfo flags that can be used to list the supported stereo
layouts (when querying the list of modes) and to set a given stereo 3D
mode (when setting a mode).
v2: Add a drm_mode_is_stereo() helper
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>
We have some code duplication related to EDID duplication. Add a helper.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
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>
Unfortunately, I haven't been thorough enough in:
commit ddecb10cf4
Author: Lespiau, Damien <damien.lespiau@intel.com>
Date: Tue Aug 20 00:53:04 2013 +0100
drm: Remove drm_mode_create_dithering_property()
And forgot to remove the dithering_mode_property member of struct
drm_mode_config.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
This function is only used inside drm_edid.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dave Airlie <airlied@gmail.com>
These were introduced in the very first DRM commit:
commit f453ba0460
Author: Dave Airlie <airlied@redhat.com>
Date: Fri Nov 7 14:05:41 2008 -0800
DRM: add mode setting support
Add mode setting support to the DRM layer.
But are unused.
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>
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>
The last user was removed in
commit 575dc34ee0
Author: Dave Airlie <airlied@redhat.com>
Date: Mon Sep 7 18:43:26 2009 +1000
drm/kms: remove old std mode fallback code.
The new code adds modes in the helper, which makes more sense
I disliked the non-driver code adding modes.
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>
A few prototypes have been left in the headers, their function friends
long gone.
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>
This function is unused.
Signed-off-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This reverts commit 160954b7bc.
This was rearming the workqueue with a 0 timeout, causing
a WARN_ON, and possible loop.
Daniel writes:
"I've looked a bit into this and I think we need to have a separate
work struct for recovering these lost hotplug events since the
continuous self-rearming case is a real risk (e.g. if a connector
flip-flops all the time). At least I don't see a sane way to block out
re-arming with the current code in a simple way. So reverting the
offender seems like the right thing and I'll go back to the drawing
board for 3.12."
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>
There's a race window (small for hpd, 10s large for polled outputs)
where userspace could sneak in with an unrelated connnector probe
ioctl call and eat the hotplug event (since neither the hpd nor the
poll code see a state change).
To avoid this, check whether the connector state changes in all other
->detect calls (in the current helper code that's only probe_single)
and if that's the case, fire off a hotplug event. Note that we can't
directly call the hotplug event handler, since that expects that no
locks are held (due to reentrancy with the fb code to update the kms
console).
Also, this requires that drivers using the probe_single helper
function set up the poll work. All current drivers do that already,
and with the reworked hpd handling there'll be no downside to
unconditionally setting up the poll work any more.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-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>
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>
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>
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>
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>
drm_mode_equal_no_clocks() is like drm_mode_equal() except it doesn't
compare the clock or vrefresh values. drm_mode_equal() is now
implemented by first doing the clock checks, and then calling
drm_mode_equal_no_clocks().
v2: Add missing EXPORT_SYMBOL()
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
No need to zero initialize .vrefresh in DRM_MODE() since it's using
desgignated initializers.
This will also avoid some duplicate initialization warnings later.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The drm_file and drm_clip_rect structures are used throughout the file
but they are never declared nor pulled in through an include. Add
forward declarations to make them available.
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
The same function had already been merged with a different name. Remove
the duplicate one but reuse some of its kerneldoc fragments for the
existing implementation.
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.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>
Daniel writes:
"Probably the last feature pull for 3.9, there's some fixes outstanding
thought that I'd like to sneak in. And maybe 3.8 takes a bit longer ...
Anyway, highlights of this pull:
- Kill the horrible IS_DISPLAYREG hack to handle the mmio offset movements
on vlv, big thanks to Ville.
- Dynamic power well support for Haswell, shaves away a bit when only
using the eDP port on pipe A (Paulo). Plus unclaimed register fixes
uncovered by this.
- Clarifications of the gpu hang/reset state transitions, hopefully fixing
a few spurious -EIO deaths in userspace.
- Haswell ELD fixes.
- Some more (pp)gtt cleanups from Ben.
- A few smaller things all over.
Plus all the stuff from the previous rather small pull request:
- Broadcast RBG improvements and reduced color range fixes from Ville.
- Ben is on a "kill legacy gtt code for good" spree, first pile of patches
included.
- No-relocs and bo lut improvements for faster execbuf from Chris.
- Some refactorings from Imre."
* tag 'drm-intel-next-2013-02-01' of git://people.freedesktop.org/~danvet/drm-intel: (101 commits)
GPU/i915: Fix acpi_bus_get_device() check in drivers/gpu/drm/i915/intel_opregion.c
drm/i915: Set the SR01 "screen off" bit in i915_redisable_vga() too
drm/i915: Kill IS_DISPLAYREG()
drm/i915: Introduce i915_vgacntrl_reg()
drm/i915: gen6_gmch_remove can be static
drm/i915: dynamic Haswell display power well support
drm/i915: check the power down well on assert_pipe()
drm/i915: don't send DP "idle" pattern before "normal" on HSW PORT_A
drm/i915: don't run hsw power well code on !hsw
drm/i915: kill cargo-culted locking from power well code
drm/i915: Only run idle processing from i915_gem_retire_requests_worker
drm/i915: Fix CAGF for HSW
drm/i915: Reclaim GTT space for failed PPGTT
drm/i915: remove intel_gtt structure
drm/i915: Add probe and remove to the gtt ops
drm/i915: extract hw ppgtt setup/cleanup code
drm/i915: pte_encode is gen6+
drm/i915: vfuncs for ppgtt
drm/i915: vfuncs for gtt_clear_range/insert_entries
drm/i915: Error state should print /sys/kernel/debug
...
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>
*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>
