summaryrefslogtreecommitdiff
path: root/drivers/gpu/drm/vkms
AgeCommit message (Collapse)AuthorFilesLines
2021-01-15drm/vkms: Fix missing kmalloc allocation failure checkColin Ian King1-1/+5
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: 2df7af93fdad ("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
2021-01-12drm/vkms: Add support for writeback moduleSumera Priyadarsini3-3/+12
Add enable_writeback feature to vkms_config as a module. 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/15802da4f1cdfed2b728c3d35731732f161dd073.1610391685.git.sylphrenadin@gmail.com
2021-01-12drm/vkms: Add vkms_config typeSumera Priyadarsini3-12/+43
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
2020-12-11drm/vkms: Unset preferred_depthDaniel Vetter1-1/+4
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
2020-11-24drm/shmem-helper: Removed drm_gem_shmem_create_object_cached()Thomas Zimmermann1-1/+0
Cached page mappings are now the default for SHMEM GEM objects. Remove the obsolete create function for cached mappings. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: Maxime Ripard <mripard@kernel.org> Link: https://patchwork.freedesktop.org/patch/msgid/20201117133156.26822-3-tzimmermann@suse.de
2020-11-20drm: Pass the full state to connectors atomic functionsMaxime Ripard1-2/+5
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. Now that the CRTCs have been converted, let's move forward with the connectors to provide a consistent interface. The conversion was done using the coccinelle script below, and built tested on all the drivers. @@ identifier connector, connector_state; @@ struct drm_connector_helper_funcs { ... struct drm_encoder* (*atomic_best_encoder)(struct drm_connector *connector, - struct drm_connector_state *connector_state); + struct drm_atomic_state *state); ... } @@ identifier connector, connector_state; @@ struct drm_connector_helper_funcs { ... void (*atomic_commit)(struct drm_connector *connector, - struct drm_connector_state *connector_state); + struct drm_atomic_state *state); ... } @@ struct drm_connector_helper_funcs *FUNCS; identifier state; identifier connector, connector_state; identifier f; @@ f(..., struct drm_atomic_state *state, ...) { <+... - FUNCS->atomic_commit(connector, connector_state); + FUNCS->atomic_commit(connector, state); ...+> } @@ struct drm_connector_helper_funcs *FUNCS; identifier state; identifier connector, connector_state; identifier var, f; @@ f(struct drm_atomic_state *state, ...) { <+... - var = FUNCS->atomic_best_encoder(connector, connector_state); + var = FUNCS->atomic_best_encoder(connector, state); ...+> } @ connector_atomic_func @ identifier helpers; identifier func; @@ ( static struct drm_connector_helper_funcs helpers = { ..., .atomic_best_encoder = func, ..., }; | static struct drm_connector_helper_funcs helpers = { ..., .atomic_commit = func, ..., }; ) @@ identifier connector_atomic_func.func; identifier connector; symbol state; @@ func(struct drm_connector *connector, - struct drm_connector_state *state + struct drm_connector_state *connector_state ) { ... - state + connector_state ... } @ ignores_state @ identifier connector_atomic_func.func; identifier connector, connector_state; @@ func(struct drm_connector *connector, struct drm_connector_state *connector_state) { ... when != connector_state } @ adds_state depends on connector_atomic_func && !ignores_state @ identifier connector_atomic_func.func; identifier connector, connector_state; @@ func(struct drm_connector *connector, struct drm_connector_state *connector_state) { + struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state, connector); ... } @ depends on connector_atomic_func @ identifier connector_atomic_func.func; identifier connector_state; identifier connector; @@ func(struct drm_connector *connector, - struct drm_connector_state *connector_state + struct drm_atomic_state *state ) { ... } @ include depends on adds_state @ @@ #include <drm/drm_atomic.h> @ no_include depends on !include && adds_state @ @@ + #include <drm/drm_atomic.h> #include <drm/...> Signed-off-by: Maxime Ripard <maxime@cerno.tech> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Acked-by: 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: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Ben Skeggs <bskeggs@redhat.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Melissa Wen <melissa.srw@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20201118094758.506730-1-maxime@cerno.tech
2020-11-09drm/gem: Use struct dma_buf_map in GEM vmap ops and convert GEM backendsThomas Zimmermann2-13/+24
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
2020-11-06drm/<drivers>: Constify struct drm_driverDaniel Vetter1-1/+1
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
2020-11-02drm/atomic: Pass the full state to CRTC atomic begin and flushMaxime Ripard1-2/+2
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 start convert all the remaining helpers to provide a consistent interface, starting with the CRTC's atomic_begin and atomic_flush. The conversion was done using the coccinelle script below, built tested on all the drivers and actually tested on vc4. virtual report @@ struct drm_crtc_helper_funcs *FUNCS; identifier old_crtc_state, old_state; identifier crtc; identifier f; @@ f(struct drm_crtc_state *old_crtc_state) { ... struct drm_atomic_state *old_state = old_crtc_state->state; <... - FUNCS->atomic_begin(crtc, old_crtc_state); + FUNCS->atomic_begin(crtc, old_state); ...> } @@ struct drm_crtc_helper_funcs *FUNCS; identifier old_crtc_state, old_state; identifier crtc; identifier f; @@ f(struct drm_crtc_state *old_crtc_state) { ... struct drm_atomic_state *old_state = old_crtc_state->state; <... - FUNCS->atomic_flush(crtc, old_crtc_state); + FUNCS->atomic_flush(crtc, old_state); ...> } @@ struct drm_crtc_helper_funcs *FUNCS; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; identifier dev, state; identifier f; @@ f(struct drm_device *dev, struct drm_atomic_state *state, ...) { <... - FUNCS->atomic_begin(crtc, crtc_state); + FUNCS->atomic_begin(crtc, state); ...> } @@ struct drm_crtc_helper_funcs *FUNCS; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; identifier dev, state; identifier f; @@ f(struct drm_device *dev, struct drm_atomic_state *state, ...) { <... - FUNCS->atomic_flush(crtc, crtc_state); + FUNCS->atomic_flush(crtc, state); ...> } @@ identifier crtc, old_state; @@ struct drm_crtc_helper_funcs { ... - void (*atomic_begin)(struct drm_crtc *crtc, struct drm_crtc_state *old_state); + void (*atomic_begin)(struct drm_crtc *crtc, struct drm_atomic_state *state); ... - void (*atomic_flush)(struct drm_crtc *crtc, struct drm_crtc_state *old_state); + void (*atomic_flush)(struct drm_crtc *crtc, struct drm_atomic_state *state); ... } @ crtc_atomic_func @ identifier helpers; identifier func; @@ ( static struct drm_crtc_helper_funcs helpers = { ..., .atomic_begin = func, ..., }; | static struct drm_crtc_helper_funcs helpers = { ..., .atomic_flush = func, ..., }; ) @ ignores_old_state @ identifier crtc_atomic_func.func; identifier crtc, old_state; @@ void func(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { ... when != old_state } @ adds_old_state depends on crtc_atomic_func && !ignores_old_state @ identifier crtc_atomic_func.func; identifier crtc, old_state; @@ void func(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc); ... } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; expression E; type T; @@ void func(...) { ... - T state = E; + T crtc_state = E; <+... - state + crtc_state ...+> } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; type T; @@ void func(...) { ... - T state; + T crtc_state; <+... - state + crtc_state ...+> } @@ identifier old_state; identifier crtc; @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ) { + struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc); ... } @@ identifier old_state; identifier crtc; @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ); @@ identifier old_state; identifier crtc; @@ void vmw_du_crtc_atomic_begin(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ) { ... } @@ identifier old_state; identifier crtc; @@ void vmw_du_crtc_atomic_begin(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ); @@ identifier old_state; identifier crtc; @@ void vmw_du_crtc_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ) { ... } @@ identifier old_state; identifier crtc; @@ void vmw_du_crtc_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ); @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; identifier old_state; identifier crtc; @@ void func(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ) { ... } @ include depends on adds_old_state @ @@ #include <drm/drm_atomic.h> @ no_include depends on !include && adds_old_state @ @@ + #include <drm/drm_atomic.h> #include <drm/...> Signed-off-by: Maxime Ripard <maxime@cerno.tech> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://patchwork.freedesktop.org/patch/msgid/20201028123222.1732139-2-maxime@cerno.tech
2020-11-02drm/atomic: Pass the full state to CRTC atomic_checkMaxime Ripard1-7/+9
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 start convert all the remaining helpers to provide a consistent interface, starting with the CRTC's atomic_check. The conversion was done using the coccinelle script below, built tested on all the drivers and actually tested on vc4. virtual report @@ struct drm_crtc_helper_funcs *FUNCS; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; identifier dev, state; identifier ret, f; @@ f(struct drm_device *dev, struct drm_atomic_state *state) { <... - ret = FUNCS->atomic_check(crtc, crtc_state); + ret = FUNCS->atomic_check(crtc, state); ...> } @@ identifier crtc, new_state; @@ struct drm_crtc_helper_funcs { ... - int (*atomic_check)(struct drm_crtc *crtc, struct drm_crtc_state *new_state); + int (*atomic_check)(struct drm_crtc *crtc, struct drm_atomic_state *state); ... } @ crtc_atomic_func @ identifier helpers; identifier func; @@ static struct drm_crtc_helper_funcs helpers = { ..., .atomic_check = func, ..., }; @ ignores_new_state @ identifier crtc_atomic_func.func; identifier crtc, new_state; @@ int func(struct drm_crtc *crtc, struct drm_crtc_state *new_state) { ... when != new_state } @ adds_new_state depends on crtc_atomic_func && !ignores_new_state @ identifier crtc_atomic_func.func; identifier crtc, new_state; @@ int func(struct drm_crtc *crtc, struct drm_crtc_state *new_state) { + struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, crtc); ... } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; expression E; type T; @@ int func(...) { ... - T state = E; + T crtc_state = E; <+... - state + crtc_state ...+> } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; type T; @@ int func(...) { ... - T state; + T crtc_state; <+... - state + crtc_state ...+> } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; identifier new_state; identifier crtc; @@ int func(struct drm_crtc *crtc, - struct drm_crtc_state *new_state + struct drm_atomic_state *state ) { ... } @@ identifier new_state; identifier crtc; @@ int vmw_du_crtc_atomic_check(struct drm_crtc *crtc, - struct drm_crtc_state *new_state + struct drm_atomic_state *state ) { + struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, crtc); ... } @@ identifier new_state; identifier crtc; @@ int vmw_du_crtc_atomic_check(struct drm_crtc *crtc, - struct drm_crtc_state *new_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/...> Signed-off-by: Maxime Ripard <maxime@cerno.tech> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://patchwork.freedesktop.org/patch/msgid/20201028123222.1732139-1-maxime@cerno.tech
2020-10-20drm/vkms: fbdev emulation supportDaniel Vetter1-0/+2
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
2020-10-16drm/vkms: Switch to shmem helpersDaniel Vetter7-327/+29
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
2020-10-16drm/vkms: Set preferred depth correctlyDaniel Vetter1-1/+1
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
2020-10-09drm/atomic: Pass the full state to CRTC atomic enable/disableMaxime Ripard1-2/+2
If the CRTC driver ever needs to access the full DRM state, it can't do so at atomic_enable / atomic_disable time since drm_atomic_helper_swap_state will have cleared the pointer from the struct drm_crtc_state to the struct drm_atomic_state before calling those hooks. In order to allow that, let's pass the full DRM state to atomic_enable and atomic_disable. The conversion was done using the coccinelle script below, built tested on all the drivers and actually tested on vc4. virtual report @@ struct drm_crtc_helper_funcs *FUNCS; identifier dev, state; identifier crtc, crtc_state; @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *state) { <... - FUNCS->atomic_disable(crtc, crtc_state); + FUNCS->atomic_disable(crtc, state); ...> } @@ struct drm_crtc_helper_funcs *FUNCS; identifier dev, state; identifier crtc, crtc_state; @@ drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_atomic_state *state) { <... - FUNCS->atomic_enable(crtc, crtc_state); + FUNCS->atomic_enable(crtc, state); ...> } @@ identifier crtc, old_state; @@ struct drm_crtc_helper_funcs { ... - void (*atomic_enable)(struct drm_crtc *crtc, struct drm_crtc_state *old_state); + void (*atomic_enable)(struct drm_crtc *crtc, struct drm_atomic_state *state); ... - void (*atomic_disable)(struct drm_crtc *crtc, struct drm_crtc_state *old_state); + void (*atomic_disable)(struct drm_crtc *crtc, struct drm_atomic_state *state); ... } @ crtc_atomic_func @ identifier helpers; identifier func; @@ ( static struct drm_crtc_helper_funcs helpers = { ..., .atomic_enable = func, ..., }; | static struct drm_crtc_helper_funcs helpers = { ..., .atomic_disable = func, ..., }; ) @ ignores_old_state @ identifier crtc_atomic_func.func; identifier crtc, old_state; @@ void func(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { ... when != old_state } @ adds_old_state depends on crtc_atomic_func && !ignores_old_state @ identifier crtc_atomic_func.func; identifier crtc, old_state; @@ void func(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc); ... } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; expression E; type T; @@ void func(...) { ... - T state = E; + T crtc_state = E; <+... - state + crtc_state ...+> } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; type T; @@ void func(...) { ... - T state; + T crtc_state; <+... - state + crtc_state ...+> } @ depends on crtc_atomic_func @ identifier crtc_atomic_func.func; identifier old_state; identifier crtc; @@ void func(struct drm_crtc *crtc, - struct drm_crtc_state *old_state + struct drm_atomic_state *state ) { ... } @ include depends on adds_old_state @ @@ #include <drm/drm_atomic.h> @ no_include depends on !include && adds_old_state @ @@ + #include <drm/drm_atomic.h> #include <drm/...> Signed-off-by: Maxime Ripard <maxime@cerno.tech> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/845aa10ef171fc0ea060495efef142a0c13f7870.1602161031.git-series.maxime@cerno.tech
2020-09-25drm/vkms: Introduce GEM object functionsThomas Zimmermann2-8/+13
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
2020-09-11drm/vkms: Use devm_drm_dev_allocDaniel Vetter1-28/+28
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
2020-09-08drm/vkms: Add support for writebackRodrigo Siqueira5-7/+179
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
2020-09-08drm/vkms: Compute CRC without change input dataRodrigo Siqueira1-12/+22
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
2020-09-08drm/vkms: Decouple crc operations from composerRodrigo Siqueira1-20/+29
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
2020-08-30drm/vkms: avoid warning in vkms_get_vblank_timestampSidong Yang1-0/+5
This patch avoid the warning in vkms_get_vblank_timestamp when vblanks aren't enabled. When running igt test kms_cursor_crc just after vkms module, the warning raised like below. Initial value of vblank time is zero and hrtimer.node.expires is also zero if vblank aren't enabled before. vkms module isn't real hardware but just virtual hardware module. so vkms can't generate a resonable timestamp when hrtimer is off. it's best to grab the current time. [106444.464503] [IGT] kms_cursor_crc: starting subtest pipe-A-cursor-size-change [106444.471475] WARNING: CPU: 0 PID: 10109 at vkms_get_vblank_timestamp+0x42/0x50 [vkms] [106444.471511] CPU: 0 PID: 10109 Comm: kms_cursor_crc Tainted: G W OE 5.9.0-rc1+ #6 [106444.471514] RIP: 0010:vkms_get_vblank_timestamp+0x42/0x50 [vkms] [106444.471528] Call Trace: [106444.471551] drm_get_last_vbltimestamp+0xb9/0xd0 [drm] [106444.471566] drm_reset_vblank_timestamp+0x63/0xe0 [drm] [106444.471579] drm_crtc_vblank_on+0x85/0x150 [drm] [106444.471582] vkms_crtc_atomic_enable+0xe/0x10 [vkms] [106444.471592] drm_atomic_helper_commit_modeset_enables+0x1db/0x230 [drm_kms_helper] [106444.471594] vkms_atomic_commit_tail+0x38/0xc0 [vkms] [106444.471601] commit_tail+0x97/0x130 [drm_kms_helper] [106444.471608] drm_atomic_helper_commit+0x117/0x140 [drm_kms_helper] [106444.471622] drm_atomic_commit+0x4a/0x50 [drm] [106444.471629] drm_atomic_helper_set_config+0x63/0xb0 [drm_kms_helper] [106444.471642] drm_mode_setcrtc+0x1d9/0x7b0 [drm] [106444.471654] ? drm_mode_getcrtc+0x1a0/0x1a0 [drm] [106444.471666] drm_ioctl_kernel+0xb6/0x100 [drm] [106444.471677] drm_ioctl+0x3ad/0x470 [drm] [106444.471688] ? drm_mode_getcrtc+0x1a0/0x1a0 [drm] [106444.471692] ? tomoyo_file_ioctl+0x19/0x20 [106444.471694] __x64_sys_ioctl+0x96/0xd0 [106444.471697] do_syscall_64+0x37/0x80 [106444.471699] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Melissa Wen <melissa.srw@gmail.com> Signed-off-by: Sidong Yang <realwakka@gmail.com> Reviewed-by: Melissa Wen <melissa.srw@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200828124553.2178-1-realwakka@gmail.com
2020-08-30drm/vkms: add alpha-premultiplied color blendingMelissa Wen1-16/+39
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
2020-08-10drm/vkms: add missing platform_device_unregister() in vkms_init()Qinglang Miao1-1/+1
When vkms_init() get into out_put, the unregister call of vkms_device->platform is missing. So add it before return. Fixes: ac19f140bc27 ("drm/vkms: Use drmm_add_final_kfree") Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200810130011.187691-1-miaoqinglang@huawei.com
2020-08-10drm/vkms: guarantee vblank when capturing crcMelissa Wen1-3/+17
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
2020-08-02drm/vkms: fix xrgb on compute crcMelissa Wen1-1/+1
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: db7f419c06d7c ("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
2020-07-13drm/vkms: change the max cursor width/heightMelissa Wen1-0/+2
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
2020-05-20drm/vkms: remove _unlocked suffix in drm_gem_object_put_unlockedEmil Velikov1-1/+1
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
2020-05-07drm/vkms: Hold gem object while still in-useEzequiel Garcia2-10/+6
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
2020-05-07drm/vkms: enable cursor by defaultMelissa Wen1-1/+1
This patch proposes a change in the behavior of the cursor to enable it as soon as the vkms module is added. Enabling the cursor by default appears to be an expected and more friendly behavior, especially when running IGT tests. Signed-off-by: Melissa Wen <melissa.srw@gmail.com> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200321203740.pg3r7f4vybruowox@smtp.gmail.com
2020-04-02drm/vkms: Use simple encoderThomas Zimmermann1-6/+2
The vkms driver uses an empty implementation for its encoder. Replace the code with the generic simple encoder. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200305155950.2705-21-tzimmermann@suse.de
2020-03-26drm: Garbage collect drm_dev_finiDaniel Vetter1-1/+0
It has become empty. Given the few users I figured not much point splitting this up. v2: Rebase over i915 changes. v3: Rebase over patch split fix. Acked-by: Sam Ravnborg <sam@ravnborg.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-26-daniel.vetter@ffwll.ch
2020-03-26drm/vkms: Use drmm_add_final_kfreeDaniel Vetter1-9/+9
With this we can drop the final kfree from the release function. v2: After drm_dev_init/drmm_add_final_kfree we need to clean up everything through a drm_dev_put. Rework the unwind code to match that. Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@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/20200323144950.3018436-15-daniel.vetter@ffwll.ch
2020-02-13drm/vkms: Convert to CRTC VBLANK callbacksThomas Zimmermann3-8/+6
VBLANK callbacks in struct drm_driver are deprecated in favor of their equivalents in struct drm_crtc_funcs. Convert vkms over. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Rodrigo Siqueira <rodrigosiqueira@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueira@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200123135943.24140-20-tzimmermann@suse.de
2020-01-28drm/vkms: plane_state->fb iff plane_state->crtcDaniel Vetter1-1/+1
Checking both is one too much, so wrap a WARN_ON around it to stope the copypasta. Reviewed-by: Rodrigo Siqueira <rodrigosiqueira@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueira@gmail.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> 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/20191213172612.1514842-9-daniel.vetter@ffwll.ch
2019-12-06drm/vkms: Fix typo and preposion in function documentationGabriela Bittencourt1-4/+4
Fix typo in word 'blend' and in the word 'destination' and change preposition 'at' to 'of' in function 'blend' documentation. And change the task introduction word 'Todo' for the word all in uppercase - 'TODO'. With the TODO word all in uppercase (as it's the standard) it's easier to find the tasks that have to be done throughout the code. Changes since V3: Rodrigo: - Merge the patch series into a single patch since it contains one single logical change Changes since V2: - Add fix typo in word 'destination' - Add change of the preposition - Fix the name of the function in log message - Add the change in word 'Todo' Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Gabriela Bittencourt <gabrielabittencourt00@gmail.com> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191104161424.18105-1-gabrielabittencourt00@gmail.com
2019-11-04drm/vkms: Update VKMS documentationGabriela Bittencourt1-4/+4
Small changes in the driver documentation, clarifing the description. Signed-off-by: Gabriela Bittencourt <gabrielabittencourt00@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20191101223735.2425-1-gabrielabittencourt00@gmail.com
2019-10-10drm/vkms: Remove duplicated include from vkms_drv.cYueHaibing1-1/+0
Remove duplicated include. Signed-off-by: YueHaibing <yuehaibing@huawei.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20191010115213.115706-1-yuehaibing@huawei.com
2019-10-09drm/vkms: prime import supportOleg Vasilev3-0/+45
Bring dmabuf sharing through implementing prime_import_sg_table callback. This will help to validate userspace conformance in prime configurations without using any actual hardware (e.g. in the cloud). This enables kms_prime IGT testcase on vkms. V3: - Rodrigo: remove redundant vkms_gem_create_private V2: - Rodrigo: styleguide + return code check Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Simon Ser <simon.ser@intel.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com> Signed-off-by: Oleg Vasilev <omrigann@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190930155924.21845-1-oleg.vasilev@intel.com
2019-09-03drm/vkms: Reduce critical section in vblank_simulateDaniel Vetter1-5/+4
We can reduce the critical section in vkms_vblank_simulate under output->lock quite a lot: - hrtimer_forward_now just needs to be ordered correctly wrt drm_crtc_handle_vblank. We already access the hrtimer timestamp without locks. While auditing that I noticed that we don't correctly annotate the read there, so sprinkle a READ_ONCE to make sure the compiler doesn't do anything foolish. - drm_crtc_handle_vblank must stay under the lock to avoid races with drm_crtc_arm_vblank_event. - The access to vkms_ouptut->crc_state also must stay under the lock. - next problem is making sure the output->state structure doesn't get freed too early. First we rely on a given hrtimer being serialized: If we call drm_crtc_handle_vblank, then we are guaranteed that the previous call to vkms_vblank_simulate has completed. The other side of the coin is that the atomic updates waits for the vblank to happen before it releases the old state. Both taken together means that by the time the atomic update releases the old state, the hrtimer won't access it anymore (it might be accessing the new state at the same time, but that's ok). - state is invariant, except the few fields separate protected by state->crc_lock. So no need to hold the lock for that. - finally the queue_work. We need to make sure there's no races with the flush_work, i.e. when we call flush_work we need to guarantee that the hrtimer can't requeue the work again. This is guaranteed by the same vblank/hrtimer ordering guarantees like the reasoning above why state won't be freed too early: flush_work on the old state is called after wait_for_flip_done in the atomic commit code. Therefore we can also move everything after the output->crc_state out of the critical section. Motivated by suggestions from Rodrigo. 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> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190719152314.7706-3-daniel.vetter@ffwll.ch
2019-09-03drm/vkms: Use wait_for_flip_doneDaniel Vetter1-1/+1
It's the recommended version, wait_for_vblanks is a bit a hacky interim thing that predates all the flip_done tracking. It's unfortunately still the default ... 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> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190719152314.7706-2-daniel.vetter@ffwll.ch
2019-07-15drm/vkms: drop use of drmP.hSam Ravnborg6-8/+23
Drop use of the deprecated drmP.h header. Replace it with the necessary includes in the individual .c files. The header files was self-contained, and extra includes were not added there. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Acked-by: Emil Velikov <emil.velikov@collabora.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@linux.ie> Link: https://patchwork.freedesktop.org/patch/msgid/20190630061922.7254-13-sam@ravnborg.org
2019-07-12drm/vkms: Rename vkms_crc.c into vkms_composer.cRodrigo Siqueira6-97/+101
As a preparation work for introducing writeback to vkms, this patch renames the file vkms_crc.c into vkms_composer.c. Accordingly, it also adjusts the functions and data structures to match the changes. No functional change. Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/dea62063077ebf5cc1dfce8876e56788d15367e6.1561491964.git.rodrigosiqueiramelo@gmail.com
2019-07-12drm/vkms: Avoid assigning 0 for possible_crtcRodrigo Siqueira4-8/+8
When vkms invoke drm_universal_plane_init(), it sets 0 for possible_crtcs parameter which means that planes can't be attached to any CRTC. It currently works due to some safeguard in the drm_crtc file; however, it is possible to identify the problem by trying to append a second connector. This patch fixes this issue by modifying vkms_plane_init() to accept an index parameter which makes the code a little bit more flexible and avoid set zero to possible_crtcs. Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/d67849c62a8d8ace1a0af455998b588798a4c45f.1561491964.git.rodrigosiqueiramelo@gmail.com
2019-06-27drm/vkms: No need for ->pages_lock in crc work anymoreDaniel Vetter1-11/+2
We're now guaranteed to no longer race against prepare_fb/cleanup_fb, which means we can access ->vaddr without having to hold a lock. Before the previous patches it was fairly easy to observe the cursor ->vaddr being invalid, but that's now gone, so we can upgrade to a full WARN_ON. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-11-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: totally reworked crc data trackingDaniel Vetter3-23/+67
The crc computation worker needs to be able to get at some data structures and framebuffer mappings, while potentially more atomic updates are going on. The solution thus far is to copy relevant bits around, but that's very tedious. Here's a new approach, which tries to be more clever, but relies on a few not-so-obvious things: - crtc_state is always updated when a plane_state changes. Therefore we can just stuff plane_state pointers into a crtc_state. That solves the problem of easily getting at the needed plane_states. - with the flushing changes from previous patches the above also holds without races due to the next atomic update being a bit eager with cleaning up pending work - we always wait for all crc work items to complete before unmapping framebuffers. - we also need to make sure that the hrtimer fires off the right worker. Keep a new distinct crc_state pointer, under the vkms_output->lock protection for this. Note that crtc->state is updated very early in the atomic commit, way before we arm the vblank event - the vblank event should always match the buffers we use to compute the crc. This also solves an issue in the hrtimer, where we've accessed drm_crtc->state without holding the right locks (we held none - oops). - in the worker itself we can then just access the plane states we need, again solving a bunch of ordering and locking issues. Accessing plane->state requires locks, accessing the private vkms_crtc_state->active_planes pointer only requires that the memory doesn't get freed too early. The idea behind vkms_crtc_state->active_planes is that this would contain all visible planes, in z-order, as a first step towards a more generic blending implementation. Note that this patch also fixes races between prepare_fb/cleanup_fb and the crc worker accessing ->vaddr. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-10-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: No _irqsave within spin_lock_irq neededDaniel Vetter1-3/+2
irqs are already off. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-9-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: Dont flush crc worker when we change crc statusDaniel Vetter1-3/+0
The crc core code can cope with some late crc, the race is kinda unavoidable. So no need to flush pending workers, they'll complete in time. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-8-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: flush crc workers earlier in commit flowDaniel Vetter2-1/+11
Currently, we flush pending CRC workers very late in the commit flow, when we destroy all the old crtc states. Unfortunately, at that point, the framebuffers are already unpinned (and our vaddr possible gone), so this isn't good. Also, the plane_states we need might also already be cleaned up, since cleanup order of state structures isn't well defined. Fix this by waiting for all CRC workers of the old state to complete before we start any of the cleanup work. For correct ordering and avoiding races, we can only flush_work after drm_atomic_helper_wait_for_vblanks() since we know that all subsequent queue_work will be for the new state. Only once that's done is flush_work() useful, before that we might flush the work, and then right after the hrtimer that simulates vblank queues it again. Every time you have a flush_work before cleaning up the work structure, the following sequence must be obeyed, or it can go wrong: 1. Make sure no one else can re-queue the work anymore (in our case that's done by a combination of first updating output->crc_state and then waiting for the vblank to pass to make sure the hrtimer has noticed that change). 2. flush_work() 3. Actually clean up stuff (which isn't done here). Doing the flush_work before we even completed the output->state update, much less waited for the vblank to make sure that's happened, missed the point. Note that this is not yet race-free because of the hrtimer and crc worker look at the wrong state pointers, but that will be fixed in subsequent patches. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-7-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: Add our own commit_tailDaniel Vetter1-0/+25
Just prep work, more will be done here in following patches. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-6-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: Rename vkms_output.state_lock to crc_lockDaniel Vetter3-7/+8
Plus add a comment about what it actually protects. It's very little. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-4-daniel.vetter@ffwll.ch
2019-06-27drm/vkms: Use spin_lock_irq in process contextDaniel Vetter1-6/+4
The worker is always in process context, no need for the _irqsafe version. Same for the set_source callback, that's only called from the debugfs handler in a syscall. Cc: Shayenne Moura <shayenneluzmoura@gmail.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-3-daniel.vetter@ffwll.ch