Lots of code can be removed by relying on fb-helper:
- "struct drm_framebuffer" moves to fb_helper.fb.
- "struct drm_gem_object" moves to fb_helper.obj[0].
- "struct qxl_device" can be inferred as drm_fb_helper is embedded.
- qxl_user_framebuffer_create -> drm_gem_fb_create.
- qxl_user_framebuffer_destroy -> drm_gem_fb_destroy.
- qxl_fbdev_destroy -> drm_fb_helper_fbdev_teardown + vfree(shadow).
Remove unused code:
- qxl_fbdev_qobj_is_fb, qxl_fbdev_set_suspend.
- Unused fields of qxl_fbdev: delayed_ops, delayed_ops_lock, size.
Misc notes:
- The dirty callback is preserved as it is necessary to trigger update
commands in the hw (the screen stays black otherwise).
- No idea when .create_handle in drm_framebuffer_funcs is used, but use
the same drm_gem_fb_create_handle to match drm_gem_fb_funcs.
- I don't know why qxl_fb_find_or_create_single used to check for an
existing framebuffer and removed that check to match other drivers.
- Use of drm_fb_helper_fbdev_teardown also requires "info->fbdefio" to
be dynamically allocated. Replace the existing defio config by
drm_fb_helper_defio_init to accomodate this.
Testing results: startx with fbdev, modesetting and qxl all seems to
work. Tested also with CONFIG_DRM_FBDEV_EMULATION=n, fbdev obviously
fails but others are fine. QEMU -spice and QEMU -spice with vdagent and
multiple (resized) displays (via remote-viewer) also works.
unbind vtconsole and rmmod has *not* regressed (i.e. it still trips on a
use-after-free in qxl_check_idle via qxl_ttm_fini).
Ideally setup/teardown is replaced by drm_fbdev_generic_setup as that
would result in further code reduction, improve error handling (like not
leaking shadow memory), but unfortunately QXL has no implementation for
qxl_gem_prime_vmap.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Link: http://patchwork.freedesktop.org/patch/msgid/20180910132156.23201-1-peter@lekensteyn.nl
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
If qxl_alloc_client_monitors_config() fails to allocate
client_monitors_config then NULL pointer dereference occurs
in function qxl_display_copy_rom_client_monitors_config() after
qxl_alloc_client_monitors_config() call.
The patch adds return error from qxl_alloc_client_monitors_config()
and additional status for qxl_display_copy_rom_client_monitors_config
return value.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
Link: http://patchwork.freedesktop.org/patch/msgid/20180727153058.23620-1-vasilyev@ispras.ru
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEEuXvWqAysSYEJGuVH/lWMcqZwE8MFAlsfjRcACgkQ/lWMcqZw
E8O5ohAAkkaJkDOAAoKB3Tz0gaTR1lFFIB2yWcDkquMXANtAMhlyEtDxoRJv0Yo9
Mwbc3IGXRLJwrnSJUSTUTxbPfB0SedysUxm+5oY8sPdCKICnhG3bWmHuCiQjsUmA
UDklcxust1VWXiRO7oQJs1jlJ0KugyTrV0NKoEX0hr+HQjnT6xZmD4Y+w5ldWhmI
VpIlZumVKREni/lVRxSWzfPqjxOSFVSgJiPgQuJascX7IvhO0SAQeJcK/1O+YBUO
GnhRIxIFJ2kJZUN/AMeLyFHoUPmrw92lE+jNLzTgsWClpLCkUvZk42F3DKPlFUQa
XVRdZd1PLVw3h4EEbANjF0jJ60O6Qw5vP2RZJQewTLoqetgnagNPu1p9+SHLoqlA
UAgMxF62XUozq2zEJojvVEgaBCwXRhX/fcfc+d8vK1cdyVIHp0pC+L2ZQVb/cgOJ
t9oDHV6ymbdyAtUqES4Xn4NZImdD99kOd9wn5eWH1UvVE8kycRxGn8qryazpeg4O
INOXEnNiuYYKx3HFGBjV2N5hqLTtCUnDj+Jj9JZWRdZiYWDxModf2MY9QXp84b4L
bZWOtRFLt6pikLkZfDahNHvypl/ImAKC1Bv/j7I+4UQl9Rv63Rip3yoOCyUb80WW
TmtjcLa+owLdXH87bq2Iptc1e0S+r2sP+rN3NafrzfD+PQ13/oU=
=kCVp
-----END PGP SIGNATURE-----
Merge tag 'drm-misc-fixes-2018-06-12' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes
Only a small qxl fix that was queued for v4.17.
Signed-off-by: Dave Airlie <airlied@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/08dcbd63-ed18-6232-7bbd-547a3d63b90f@linux.intel.com
"qxl_bo_unref" may sleep, but calling "qxl_release_map" causes
"preempt_disable()" to be called and "preempt_enable()" isn't called
until "qxl_release_unmap" is used. Move the call to "qxl_bo_unref" out
from in between the two to avoid sleeping from an atomic context.
This issue can be demonstrated on a kernel with CONFIG_LOCKDEP=y by
creating a VM using QXL, using a desktop environment using Xorg, then
moving the cursor on or off a window.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1571128
Fixes: 9428088c90 ("drm/qxl: reapply cursor after resetting primary")
Cc: stable@vger.kernel.org
Signed-off-by: Jeremy Cline <jcline@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20180601200532.13619-1-jcline@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
These days drm core checks function pointers everywhere before calling
them. So we can drop a bunch of dummy functions now.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20180420071904.24276-5-kraxel@redhat.com
The encoder callbacks are only called in case the video mode changes.
So any layout changes without mode changes will go unnoticed.
Add qxl_crtc_update_monitors_config(), based on the old
qxl_write_monitors_config_for_encoder() function. Hook it into the
enable, disable and flush atomic crtc callbacks. Remove monitors_config
updates from all other places.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1544322
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20180420071904.24276-4-kraxel@redhat.com
qxl_io_log() sends messages over to the host (qemu) for logging.
Remove the function and all callers, we can just use standard
DRM_DEBUG calls (and if needed a serial console).
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20180420071904.24276-2-kraxel@redhat.com
The method struct drm_connector_helper_funcs::mode_valid is defined
as returning an 'enum drm_mode_status' but the driver implementation
for this method uses an 'int' for it.
Fix this by using 'enum drm_mode_status' in the driver too.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20180424131515.2360-1-luc.vanoostenryck@gmail.com
Replace drm_gem_object_reference/unreference function with *_get/put()
suffixes, because it is shorter and consistent with the kernel
kref_get/put() functions. The following Coccinelle script was used:
@@
expression e;
@@
(
-drm_gem_object_reference(e);
+drm_gem_object_get(e);
|
-drm_gem_object_unreference(e);
+drm_gem_object_put(e);
|
-drm_gem_object_unreference_unlocked(e);
+drm_gem_object_put_unlocked(e);
)
Signed-off-by: Santha Meena Ramamoorthy <santhameena13@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/1521570567-22519-1-git-send-email-santhameena13@gmail.com
QXL associates mouse state with its primary plane.
Destroying a primary plane and putting a new one in place has the side
effect of destroying the cursor as well.
This commit changes the driver to reapply the cursor any time a new
primary is created. It achieves this by keeping a reference to the
cursor bo on the qxl_crtc struct.
This fix is very similar to
commit 4532b241a4 ("drm/qxl: reapply cursor after SetCrtc calls")
which got implicitly reverted as part of implementing the atomic
modeset feature.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1512097
Fixes: 1277eed5fe ("drm: qxl: Atomic phase 1: convert cursor to universal plane")
Cc: stable@vger.kernel.org
Signed-off-by: Ray Strode <rstrode@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
qxl_cursor_atomic_update allocs a bo for the cursor that
it never frees up at the end of the function.
This commit fixes that.
Signed-off-by: Ray Strode <rstrode@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
This patch changes the way the primary surface is used for dumb
framebuffers. Instead of configuring the bo itself as primary surface
a shadow bo is created and used instead. Framebuffers can share the
shadow bo in case they have the same format and resolution.
On atomic plane updates we don't have to update the primary surface in
case we pageflip from one framebuffer to another framebuffer which
shares the same shadow. This in turn avoids the flicker caused by the
primary-destroy + primary-create cycle, which is very annonying when
running wayland on qxl.
The qxl driver never actually writes to the shadow bo. It sends qxl
blit commands which update it though, and the spice server might
actually execute them (and thereby write to the shadow) in case the
local rendering is kicked for some reason. This happens for example in
case qemu is asked to write out a dump of the guest display (screendump
monitor command).
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20171019062150.28090-3-kraxel@redhat.com
The atomic conversion of the qxl driver didn't got the primary surface
handling completely right. It works in the common simple cases, but
fails for example when changing the display resolution using xrandr or
in multihead setups.
The rules are simple: There is one primary surface. Before defining a
new one you have to destroy the old one.
This patch makes qxl_primary_atomic_update() destroy the primary surface
before defining a new one. It fixes is_primary flag updates. It adds
is_primary checks so we don't try to update the primary surface in case
it already has the state we want it being in.
Fixes: 3538e80a86 ("drm: qxl: Atomic phase 1: Implement mode_set_nofb")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102338
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196777
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/20170911093950.22401-1-kraxel@redhat.com
This is the plumbing for supporting fb modifiers on planes. Modifiers
have already been introduced to some extent, but this series will extend
this to allow querying modifiers per plane. Based on this, the client to
enable optimal modifications for framebuffers.
This patch simply allows the DRM drivers to initialize their list of
supported modifiers upon initializing the plane.
v2: A minor addition from Daniel
v3:
* Updated commit message
* s/INVALID/DRM_FORMAT_MOD_INVALID (Liviu)
* Remove some excess newlines (Liviu)
* Update comment for > 64 modifiers (Liviu)
v4: Minor comment adjustments (Liviu)
v5: Some new platforms added due to rebase
v6: Add some missed plane inits (or maybe they're new - who knows at
this point) (Daniel)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Daniel Stone <daniels@collabora.com> (v2)
Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Daniel Stone <daniels@collabora.com>
The CRTC .disable() helper operation is deprecated for atomic drivers,
the new .atomic_disable() helper operation being preferred. Convert all
atomic drivers to .atomic_disable() to avoid cargo-cult use of
.disable() in new drivers.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> # for sun4i
Acked-by: Philipp Zabel <p.zabel@pengutronix.de> # for mediatek
Acked-by: Alexey Brodkin <abrodkin@synopsys.com> # for arcpgu
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # for atmel-hlcdc
Tested-by: Philippe Cornu <philippe.cornu@st.com> # for stm
Acked-by: Philippe Cornu <philippe.cornu@st.com> # for stm
Acked-by: Vincent Abriou <vincent.abriou@st.com> # for sti
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> # for vmwgfx
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170630093646.7928-3-laurent.pinchart+renesas@ideasonboard.com
The old state is useful for drivers that need to perform operations at
enable time that depend on the transition between the old and new
states.
While at it, rename the operation to .atomic_enable() to be consistent
with .atomic_disable(), as the .enable() operation is used by atomic
helpers only.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> # for sun4i
Acked-by: Philipp Zabel <p.zabel@pengutronix.de> # for imx-drm and mediatek
Acked-by: Alexey Brodkin <abrodkin@synopsys.com> # for arcpgu
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # for atmel-hlcdc
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> # for hdlcd and mali-dp
Acked-by: Stefan Agner <stefan@agner.ch> # for fsl-dcu
Tested-by: Philippe Cornu <philippe.cornu@st.com> # for stm
Acked-by: Philippe Cornu <philippe.cornu@st.com> # for stm
Acked-by: Vincent Abriou <vincent.abriou@st.com> # for sti
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> # for vmwgfx
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170630093646.7928-2-laurent.pinchart+renesas@ideasonboard.com
The CRTC .dpms() helper operation is called by the atomic helpers only
when no .prepare(), .atomic_disable() or .disable() operation is
provided. As the qxl driver provides a .disable() operation, the .dpms()
operation is unused and can be removed.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170627211621.27767-3-laurent.pinchart+renesas@ideasonboard.com
-----BEGIN PGP SIGNATURE-----
iQEcBAABAgAGBQJZK2lrAAoJEHm+PkMAQRiGm3AH/13F1DlIk05aSXHoDr/idIpR
GMHmk3YF+EuFjsL463Sh6s/SSWmz0Lda8euaoB4wCWvQFX2ZjTE+aOd79XlRiZJQ
OTtLkV9I41eXIJUpEOHia7xZiCsbw+usqcHrm1aBoSh5KKV2iQmEOrnJdibqJVOF
eXUMphNK/zFtAd2bKtQSxkaBnOOqsQUgVQSkr2K9rSg25l0KokFC6c5K5IjLn4x9
QgDY4wmMvHrDz0CtpoqlNM4XqbsDJVrFeZGfg6hlMqSRDeXeg4h3Ol0VfIT496RP
QBdrDb6hWO+HKt9B0M+7Q+8a/Fsw+5dtpqv1W/Wlr0i4CS6euU8NChAmrpkrqGo=
=m5ba
-----END PGP SIGNATURE-----
Backmerge tag 'v4.12-rc3' into drm-next
Linux 4.12-rc3
Daniel has requested this for some drm-intel-next work.
My static checker complains that "release" is uninitialized if
qxl_alloc_release_reserved() fails, so let's add a check for that.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170314075410.GB5984@mwanda
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Call qxl_add_monitors_config_modes() unconditionally. Do all sanity
checks in that function.
Fix sanity checks. monitors_config is the current monitor
configuration, whereas client_monitors_config is the configuration
requested by the spice client. So when filling the mode list, based on
the spice client request, we need to look at
client_monitors_config->count not monitors_config->count.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1488363154-6889-5-git-send-email-kraxel@redhat.com
Try to read the client monitors config at driver load time, even without
explicit notification. So in case that info was filled before the driver
loaded and we've missed the notifications because of that the settings
will still be used.
With that place we now have to take care to properly handle a empty client
monitors config, so we don't trip over an uninitialized client monitors
config.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1488363154-6889-4-git-send-email-kraxel@redhat.com
When reading the monitor config fails, don't retry forever. If it fails
ten times in a row just give up to avoid the driver hangs. Also add a
small delay after each attempt, so the host has a chance to complete a
partial update.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1488363154-6889-3-git-send-email-kraxel@redhat.com
In the qxl atomic model, the primary doesn't stay pinned all the time,
instead it is only pinned/unpinned between prepare_fb and cleanup_fb.
So, we no longer need a final unpin of the primary framebuffer when
disabling the crtc.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/20170227204328.18761-9-krisman@collabora.co.uk
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Every attempt to pin/unpin objects in memory requires
qxl_bo_reserve/unreserve calls around the pinning operation to protect
the object from concurrent access, which causes that call sequence to be
reproduced every place where pinning is needed. In some cases, that
sequence was not executed correctly, resulting in potential unprotected
pinning operations.
This commit encapsulates the reservation inside a new wrapper to make
sure it is always handled properly. In cases where reservation must be
done beforehand, for some reason, one can use the unprotected version
__qxl_bo_pin/unpin.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170227204328.18761-3-krisman@collabora.co.uk
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The vblank hooks in struct drm_driver are deprecated and only meant for
legacy drivers. For modern drivers with DRIVER_MODESET flag, the hooks
in struct drm_crtc_funcs should be used instead.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: http://patchwork.freedesktop.org/patch/msgid/1486458995-31018-16-git-send-email-shawnguo@kernel.org
Registering the connector explicitly right after creation is not
necessary for modesetting drivers, because drm_dev_register already takes
care of this on the core side, by calling drm_modeset_register_all.
In addition, performing the initialization too early will get in the way
of the load() hook removal, because the connector interface cannot be
published prior to registering the minors.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
CC: Dave Airlie <airlied@redhat.com>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: dri-devel@lists.freedesktop.org
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170112161610.19924-1-krisman@collabora.co.uk
We want framebuffers to be mostly useable already before
drm_framebuffer_init() is called, and so we will start demanding that
all the interesting format/size/etc. information be filled in before
drm_framebuffer_init(). drm_helper_mode_fill_fb_struct() will do that
for us, so let's make sure it gets called before drm_framebuffer_init().
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1479498793-31021-16-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
the resolutions we are going to present to user-space are going to be
rounded down to a multiple of 8. In the QXL arbitrary resolution case,
this is not useful.
This commit forces the actual width/height that was requested by the
client in the drm_display_mode structure rather than keeping the
rounded version.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Message-id: 20161108091209.25568-8-cfergeau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt,
we currently always notify userspace that there was some hotplug event.
However, gnome-shell/mutter is reacting to this event by attempting a
resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
and then drmModeSetCrtc. This has the side-effect of causing
qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
surface was destroyed and created. After going through QEMU and then the
remote SPICE client, a new identical monitors config message will be
sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
be emitted, and the same scenario occurring again.
As destroying/creating the primary surface causes a visible screen
flicker, this makes the guest hard to use (
https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
This commit checks if the screen configuration we received is the same
one as the current one, and does not notify userspace about it if that's
the case.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Message-id: 20161108091209.25568-7-cfergeau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
