For framebuffers with non-zero offset fields, shadow-plane helpers
provide a pointer to the first byte of the contained data. Use it in
vkms.
Also provide use the offset-adjusted data address for the writeback
job's output buffers. Output framebuffers with non-zero offsets now
have their content written to the correct location.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210803125928.27780-12-tzimmermann@suse.de
Add an additional argument to drm_gem_fb_vmap() to return each BO's
mapping adjusted by the respective offset. Update all callers.
The newly returned values point to the first byite of the data stored
in the framebuffer BOs. Drivers that access the BO data should use it.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210803125928.27780-2-tzimmermann@suse.de
Abstract the framebuffer details by mappings its BOs with a call
to drm_gem_fb_vmap(). Unmap with drm_gem_fb_vunamp().
Before, the output address with stored as raw pointer in the priv
field of struct drm_writeback_job. Introduce the new type
struct vkms_writeback_job, which holds the output mappings addresses
while the writeback job is active.
The patchset also cleans up some internal casting an setup of the
output addresses. No functional changes.
v3:
* free instances of struct vkms_writeback_job on cleanup
or errors
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210730183511.20080-6-tzimmermann@suse.de
Store the shadow-buffer mapping's address in struct vkms_composer and
use the value when composing the output. It's the same value as stored
in the GEM SHMEM BO, but frees the composer code from its dependency
on GEM SHMEM.
Using struct dma_buf_map is how framebuffer access is supposed to be.
The long-term plan is to perform all framebuffer access via struct
dma_buf_map and avoid the details of accessing I/O and system memory.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210705074633.9425-5-tzimmermann@suse.de
Replace vkms' prepare_fb and cleanup_fb functions with the generic
code for shadow-buffered planes. No functional changes.
This change also fixes a problem where IGT kms_flip tests would
create a segmentation fault within vkms. The driver's prepare_fb
function did not report an error if a BO's vmap operation failed.
The kernel later tried to operate on the non-mapped memory areas.
The shared shadow-plane helpers handle errors correctly, so that
the driver now avoids the segmantation fault.
v2:
* include paragraph about IGT tests in commit message (Melissa)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210705074633.9425-4-tzimmermann@suse.de
Creating a vkms_config debugfs file in vkms_drv.c to get/track vkms config
data, for the long-term plan of making vkms configurable and have multiple
different instances.
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210708092002.11847-1-martinsdecarvalhobeatriz@gmail.com
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vkms.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210625082222.3845-25-tzimmermann@suse.de
Add support to overlay plane, in addition to primary and cursor
planes. In this approach, the plane composition still requires an
active primary plane and planes are composed associatively in the
order: (primary <- overlay) <- cursor
It enables to run the following IGT tests successfully:
- kms_plane_cursor:
- pipe-A-[overlay, primary, viewport]-size-[64, 128, 256]
- kms_atomic:
- plane-overlay-legacy
and preserves the successful execution of kms_cursor_crc,
kms_writeback and kms_flip
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/8261bf93d8a0e3ffaf81d8e7c9b3e9c229116be3.1619250933.git.melissa.srw@gmail.com
Add support for composing XRGB888 planes in addition to the ARGB8888
format. In the case of an XRGB plane at the top, the composition consists
of copying the RGB values of a pixel from src to dst and clearing alpha
channel, without the need for alpha blending operations for each pixel.
Blend equations assume a completely opaque background, i.e., primary plane
is not cleared before pixel blending but alpha channel is explicitly
opaque (a = 0xff). Also, there is room for performance evaluation in
switching pixel blend operation according to the plane format.
v4:
- clear alpha channel (0xff) after blend color values by pixel
- improve comments on blend ops to reflect the current state
- describe in the commit message future improvements for plane composition
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/07bcf4643d11da9480599fe1b165e478bff58b25.1619250933.git.melissa.srw@gmail.com
Many drivers reference the plane->state pointer in order to get the
current plane state in their atomic_update or atomic_disable hooks,
which would be the new plane state in the global atomic state since
_swap_state happened when those hooks are run.
Use the drm_atomic_get_new_plane_state helper to get that state to make it
more obvious.
This was made using the coccinelle script below:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
(
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_disable = func,
...,
};
|
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_update = func,
...,
};
)
@ adds_new_state @
identifier plane_atomic_func.func;
identifier plane, state;
identifier new_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state)
{
...
- struct drm_plane_state *new_state = plane->state;
+ struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
...
}
@ include depends on adds_new_state @
@@
#include <drm/drm_atomic.h>
@ no_include depends on !include && adds_new_state @
@@
+ #include <drm/drm_atomic.h>
#include <drm/...>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://lore.kernel.org/r/20210219120032.260676-1-maxime@cerno.tech
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_check.
The conversion was done using the coccinelle script below plus some
manual changes for vmwgfx, built tested on all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_check(plane, plane_state)
+ FUNCS->atomic_check(plane, state)
...+>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *new_plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include <drm/drm_atomic.h>
@ no_include depends on !include && adds_new_state @
@@
+ #include <drm/drm_atomic.h>
#include <drm/...>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20210219120032.260676-4-maxime@cerno.tech
This is needed to signal the fences from page flips, annotate it
accordingly. We need to annotate entire timer callback since if we get
stuck anywhere in there, then the timer stops, and hence fences stop.
Just annotating the top part that does the vblank handling isn't
enough.
Tested-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-rdma@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210121152959.1725404-3-daniel.vetter@ffwll.ch
The function drm_gem_fb_prepare_fb() is a helper for atomic modesetting,
but currently located next to framebuffer helpers. Move it to GEM atomic
helpers, rename it slightly and adopt the drivers. Same for the rsp
simple-pipe helper.
Compile-tested with x86-64, aarch64 and arm. The patch is fairly large,
but there are no functional changes.
v3:
* remove out-comented line in drm_gem_framebuffer_helper.h
(Maxime)
v2:
* rename to drm_gem_plane_helper_prepare_fb() (Daniel)
* add tutorial-style documentation
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Maxime Ripard <mripard@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210222141756.7864-1-tzimmermann@suse.de
Switch DRM drivers from drm_get_format_name() to %p4cc. This gets rid of a
large number of temporary variables at the same time.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20210216155723.17109-4-sakari.ailus@linux.intel.com
Currently the kmalloc allocation for config is not being null
checked and could potentially lead to a null pointer dereference.
Fix this by adding the missing null check.
Addresses-Coverity: ("Dereference null return value")
Fixes: 2df7af93fd ("drm/vkms: Add vkms_config type")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210115130911.71073-1-colin.king@canonical.com
Currently, data for the device instance is held by vkms_device.
Add a separate type, vkms_config to contain configuration details
for the device and various modes to be later used by configfs.
This config data stays constant once the device is created.
Accordingly, add vkms_create and vkms_destroy to initialize/destroy
device through configfs. Currently, they are being called from vkms_init
and vkms_exit, but will be evoked from configfs later on. When configfs
is added, device configuration will be tracked by configfs and only vkms
device lifetime will be handled by vkms_init and vkms_exit functions.
Modify usage of enable_cursor feature to reflect the changes in
relevant files.
Co-developed-by: Daniel Vetter <danvet.vetter@ffwl.ch>
Signed-off-by: Daniel Vetter <danvet.vetter@ffwl.ch>
Signed-off-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/a090ad29b826185df30f80c66932dd2173d7b060.1610391685.git.sylphrenadin@gmail.com
There's a confusion between the preferred_depth uapi and the generic
fbdev helpers. Former wants depth, latter wants bpp, and for XRGB8888
they don't match. Which hit me with vkms, which wants that.
All other drivers setting this and using the generic fbdev helpers use
16, where both numbers match, for RGB565.
Since fixing this is a bit involved (I think for atomic drivers we
should just compute this all internally from the format list of the
first primary plane) paper over the issue in vkms by using defaults
everywhere. Then userspace will pick XRGB8888, and fbdev helpers will
do the same, and we have what we want.
Reported-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Simon Ser <contact@emersion.fr>
Cc: Simon Ser <contact@emersion.fr>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20201211161113.3350061-1-daniel.vetter@ffwll.ch
This patch replaces the vmap/vunmap's use of raw pointers in GEM object
functions with instances of struct dma_buf_map. GEM backends are
converted as well. For most of them, this simply changes the returned type.
TTM-based drivers now return information about the location of the memory,
either system or I/O memory. GEM VRAM helpers and qxl now use ttm_bo_vmap()
et al. Amdgpu, nouveau and radeon use drm_gem_ttm_vmap() et al instead of
implementing their own vmap callbacks.
v7:
* init QXL cursor to mapped BO buffer (kernel test robot)
v5:
* update vkms after switch to shmem
v4:
* use ttm_bo_vmap(), drm_gem_ttm_vmap(), et al. (Daniel, Christian)
* fix a trailing { in drm_gem_vmap()
* remove several empty functions instead of converting them (Daniel)
* comment uses of raw pointers with a TODO (Daniel)
* TODO list: convert more helpers to use struct dma_buf_map
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Christian König <christian.koenig@amd.com>
Tested-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20201103093015.1063-7-tzimmermann@suse.de
Only the following drivers aren't converted:
- amdgpu, because of the driver_feature mangling due to virt support.
Subsequent patch will address this.
- nouveau, because DRIVER_ATOMIC uapi is still not the default on the
platforms where it's supported (i.e. again driver_feature mangling)
- vc4, again because of driver_feature mangling
- qxl, because the ioctl table is somewhere else and moving that is
maybe a bit too much, hence the num_ioctls assignment prevents a
const driver structure.
- arcpgu, because that is stuck behind a pending tiny-fication series
from me.
- legacy drivers, because legacy requires non-const drm_driver.
Note that for armada I also went ahead and made the ioctl array const.
Only cc'ing the driver people who've not been converted (everyone else
is way too much).
v2: Fix one misplaced const static, should be static const (0day)
v3:
- Improve commit message (Sam)
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: kernel test robot <lkp@intel.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201104100425.1922351-5-daniel.vetter@ffwll.ch
Hooray for generic fbdev support, making this a oneliner. We just
needed to fix preferred_depth fixed and the vmap support added first.
This is useful for testing fbdev testcases in igt. Right now we only
have a simple one in the fbdev testcases, which passes both info and
mmap subtests.
v2: Augment commit message as suggested by Melissa.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20201009232156.3916879-3-daniel.vetter@ffwll.ch
Inspired by a patch by Chris Wilson for vgem. Plus this gives us vmap
at the gem bo level, which we need for generic fbdev emulation.
Luckily shmem also tracks ->vaddr, so we just need to adjust the code
all over a bit to make this fit.
Also wire up handle_to_fd, dunno why that was missing.
v2:
- Drop now unused container_of #define (Melissa)
- Make sure we keep creating cached objects, this is for testing
(Thomas)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201013111027.375999-1-daniel.vetter@ffwll.ch
The only thing we support is xrgb8888.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20201009232156.3916879-1-daniel.vetter@ffwll.ch
GEM object functions deprecate several similar callback interfaces in
struct drm_driver. This patch replaces the per-driver callbacks with
per-instance callbacks in vkms.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Acked-by: Christian König <christian.koenig@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200923102159.24084-20-tzimmermann@suse.de
This means we also need to slightly restructure the exit code, so that
final cleanup of the drm_device is triggered by unregistering the
platform device. Note that devres is both clean up when the driver is
unbound (not the case for vkms, we don't bind), and also when unregistering
the device (very much the case for vkms). Therefore we can rely on devres
even though vkms isn't a proper platform device driver.
This also somewhat untangles the load code, since the drm and platform device
setup are no longer interleaved, but two distinct steps.
v2: use devres_open/release_group so we can use devm without real
hacks in the driver core or having to create an entire fake bus for
testing drivers. Might want to extract this into helpers eventually,
maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
v3: Only deref vkms_device after checking it (Melissa)
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Cc: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20200909091833.440548-1-daniel.vetter@ffwll.ch
This patch implements the necessary functions to add writeback support
for vkms. This feature is useful for testing compositors if you don't
have hardware with writeback support.
Change in V4 (Emil and Melissa):
- Move signal completion above drm_crtc_add_crc_entry()
- Make writeback always available
- Use appropriate namespace
- Drop fb check in vkms_wb_atomic_commit
- Make vkms_set_composer visible for writeback code
- Enable composer operation on prepare_job and disable it on cleanup_job
- Drop extra space at the end of the file
- Rebase
Change in V3 (Daniel):
- If writeback is enabled, compose everything into the writeback buffer
instead of CRC private buffer
- Guarantees that the CRC will match exactly what we have in the
writeback buffer.
Change in V2:
- Rework signal completion (Brian)
- Integrates writeback with active_planes (Daniel)
- Compose cursor (Daniel)
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200830142000.146706-4-rodrigosiqueiramelo@gmail.com
This commit decouples pixel manipulation from compute_crc() for avoiding
any pixel change during the CRC calculation. This commits represents a
preparation work for making VKMS able to support the writeback feature.
Change in V5 (Melissa):
- Rebase and drop bitmap for alpha
Change in V4 (Emil):
- Move bitmap_clear operation and comments to get_pixel function
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200830142000.146706-3-rodrigosiqueiramelo@gmail.com
In the vkms_composer.c, some of the functions related to CRC and compose
have interdependence between each other. This patch reworks some
functions inside vkms_composer to make crc and composer computation
decoupled.
This patch is preparation work for making vkms able to support new
features.
Tested-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200830142000.146706-2-rodrigosiqueiramelo@gmail.com
The VKMS blend function was ignoring the alpha channel and just
overwriting vaddr_src with vaddr_dst. This XRGB approach triggers a
warning when running the kms_cursor_crc/cursor-alpha-transparent test
case. In IGT, cairo_format_argb32 uses premultiplied alpha (according to
documentation). Also current DRM assumption is that alpha is
premultiplied. Therefore, this patch considers premultiplied alpha
blending eq to compose vaddr_src with vaddr_dst.
This change removes the following cursor-alpha-transparent warning:
"Suspicious CRC: All values are 0."
V2:
- static for local functions
- const for the read-only variable argb_src
- replaces variable names
- drops unnecessary comment
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200825114532.abzdooluny2ekzvm@smtp.gmail.com
VKMS needs vblank interrupts enabled to capture CRC. When vblank is
disabled, tests like kms_cursor_crc and kms_pipe_crc_basic getting stuck
waiting for a capture that will not occur until vkms wakes up. This patch
adds a helper to set composer and ensure that vblank remains enabled as
long as the CRC capture is needed.
It clears the execution of the following kms_cursor_crc subtests:
1. pipe-A-cursor-[size,alpha-opaque, NxN-(on-screen, off-screen, sliding,
random, fast-moving])] - successful when running individually.
2. pipe-A-cursor-dpms passes again
3. pipe-A-cursor-suspend also passes
The issue was initially tracked in the sequential execution of IGT
kms_cursor_crc subtests: when running the test sequence or one of its
subtests twice, the odd execs complete and the pairs get stuck in an
endless wait. In the IGT code, calling a wait_for_vblank on preparing for
CRC capture prevented the busy-wait. But the problem persisted in the
pipe-A-cursor-dpms and -suspend subtests.
Checking the history, the pipe-A-cursor-dpms subtest was successful when,
in vkms_atomic_commit_tail, instead of using the flip_done op, it used
wait_for_vblanks. Another way to prevent blocking was wait_one_vblank when
enabling crtc. However, in both cases, pipe-A-cursor-suspend persisted
blocking in the 2nd start of CRC capture, which may indicate that
something got stuck in the step of CRC setup. Indeed, wait_one_vblank in
the crc setup was able to sync things and free all kms_cursor_crc
subtests. Besides, other alternatives to force enabling vblanks or prevent
disabling them such as calling drm_crtc_put_vblank or modeset_enables
before commit_planes + offdelay = 0, also unlock all subtests executions.
Finally, due to vkms's dependence on vblank interruptions to perform
tasks, this patch uses refcount to ensure that vblanks happen when
enabling composer and while crc capture is needed.
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
v2:
- extract a vkms_set_composer helper
- fix vblank refcounting for the disabling case
v3:
- make the vkms_set_composer helper static
- review the credit tags
Co-debugged-by: Sidong Yang <realwakka@gmail.com>
Signed-off-by: Sidong Yang <realwakka@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[danvet: add changelog back in]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20200808120900.pudwwrfz44g3rqx7@smtp.gmail.com
The previous memset operation was not correctly zeroing the alpha
channel to compute the crc, and as a result, the IGT subtest
kms_cursor_crc/pipe-A-cursor-alpha-transparent fails.
Fixes: db7f419c06 ("drm/vkms: Compute CRC with Cursor Plane")
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20200730202524.5upzuh4irboru7my@smtp.gmail.com
This change expands the coverage for the IGT kms_cursor_crc test, where
the size varies between 64 and 512 for a square cursor. With this, in
addition to the cursor 64x64, this patch enables the test of cursors with
sizes: 128x128, 256x256, and 512x512.
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200710160313.xjoz6ereyma5vkc3@smtp.gmail.com
Spelling out _unlocked for each and every driver is a annoying.
Especially if we consider how many drivers, do not know (or need to)
about the horror stories involving struct_mutex.
Just drop the suffix. It makes the API cleaner.
Done via the following script:
__from=drm_gem_object_put_unlocked
__to=drm_gem_object_put
for __file in $(git grep --name-only $__from); do
sed -i "s/$__from/$__to/g" $__file;
done
v2:
- Rebase
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org> (v1)
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20200515095118.2743122-37-emil.l.velikov@gmail.com
We need to keep the reference to the drm_gem_object
until the last access by vkms_dumb_create.
Therefore, the put the object after it is used.
This fixes a use-after-free issue reported by syzbot.
While here, change vkms_gem_create() symbol to static.
Reported-and-tested-by: syzbot+e3372a2afe1e7ef04bc7@syzkaller.appspotmail.com
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200427214405.13069-1-ezequiel@collabora.com
Thomas Zimmermann