Cursor Corruption Fix, Upstream Edition #4971

mripard · 2022-03-31T13:40:14Z

Hi,

Here's a PR with the fix suggested by upstream for the frame corruption we were seeing along the edges of display when the overscan margins were enabled. This was fixed by PR #4895.

The main issue was that we were ending up skipping the wait for vblank due to a hack meant to handle the legacy cursor APIs. The initial fix implemented by the PR was to work around that hack, but upstream suggested to remove it entirely instead.

This PR thus reverts the workaround and removes the legacy cursor hack.

The stuff never really worked, and leads to lots of fun because it out-of-order frees atomic states. Which upsets KASAN, among other things. For async updates we now have a more solid solution with the ->atomic_async_check and ->atomic_async_commit hooks. Support for that for msm and vc4 landed. nouveau and i915 have their own commit routines, doing something similar. For everyone else it's probably better to remove the use-after-free bug, and encourage folks to use the async support instead. The affected drivers which register a legacy cursor plane and don't either use the new async stuff or their own commit routine are: amdgpu, atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx. Inspired by an amdgpu bug report. v2: Drop RFC, I think with amdgpu converted over to use atomic_async_check/commit done in commit 674e78a Author: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> Date: Wed Dec 5 14:59:07 2018 -0500 drm/amd/display: Add fast path for cursor plane updates we don't have any driver anymore where we have userspace expecting solid legacy cursor support _and_ they are using the atomic helpers in their fully glory. So we can retire this. v3: Paper over msm and i915 regression. The complete_all is the only thing missing afaict. v4: Rebased on recent kernel, added extra link for vc4 bug. Link: https://bugzilla.kernel.org/show_bug.cgi?id=199425 Link: https://lore.kernel.org/all/20220221134155.125447-9-maxime@cerno.tech/ Cc: mikita.lipski@amd.com Cc: Michel Dänzer <michel@daenzer.net> Cc: harry.wentland@amd.com Cc: Rob Clark <robdclark@gmail.com> Cc: "Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com> Tested-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

This reverts commit e99a1b6.

popcornmix · 2022-04-05T12:42:57Z

This seems to work as well as #4895 (and is simpler which is good).
There is still the issue mentioned there: #4895 (comment),
but that seems unrelated to original "corrupt cursor" issue.

kernel: Cursor Corruption Fix, Upstream Edition See: raspberrypi/linux#4971 kernel: Add support for IMX258 See: raspberrypi/linux#4979 kernel: drm/vc4_hdmi: Force modeset when bpc changes See: raspberrypi/linux#4980

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: raspberrypi#4971 raspberrypi#4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: #4971 #4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: raspberrypi#4971 raspberrypi#4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: #4971 #4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: raspberrypi#4971 raspberrypi#4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: #4971 #4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: raspberrypi#4971 raspberrypi#4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: #4971 #4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com>

This is a squash of all firmware-kms related patches from previous branches, up to and including "drm/vc4: Set the possible crtcs mask correctly for planes with FKMS" plus a couple of minor fixups for the 5.9 branch. Please refer to earlier branches for full history. This patch includes work by Eric Anholt, James Hughes, Phil Elwell, Dave Stevenson, Dom Cobley, and Jonathon Bell. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fixup firmware-kms after "drm/atomic: Pass the full state to CRTC atomic enable/disable" Prototype for those calls changed, so amend fkms (which isn't upstream) to match. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fixup fkms for API change Atomic flush and check changed API, so fix up the downstream-only FKMS driver. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Make normalize_zpos conditional on using fkms Eric's view was that there was no point in having zpos support on vc4 as all the planes had the same functionality. Can be later squashed into (and fixes): drm/vc4: Add firmware-kms mode Signed-off-by: Dom Cobley <popcornmix@gmail.com> drm/vc4: FKMS: Change of Broadcast RGB mode needs a mode change The Broadcast RGB (aka HDMI limited/full range) property is only notified to the firmware on mode change, so this needs to be signalled when set. raspberrypi/firmware#1580 Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> vc4/drv: Only notify firmware of display done with kms fkms driver still wants firmware display to be active Signed-off-by: Dom Cobley <popcornmix@gmail.com> ydrm/vc4: fkms: Fix margin calculations for the right/bottom edges The calculations clipped the right/bottom edge of the clipped range based on the left/top margins. raspberrypi/linux#4447 Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: fkms: Use new devm_rpi_firmware_get api drm/kms: Add allow_fb_modifiers Signed-off-by: Dom Cobley <popcornmix@gmail.com> drm/vc4: Add async update support for cursor planes Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: raspberrypi/linux#4971 raspberrypi/linux#4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com> drivers/gpu/drm/vc4: Add missing 32-bit RGB formats The missing 32-bit per pixel ABGR and various "RGB with an X value" formats are added. Change sent by Dave Stevenson. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> drm: vc4: Fixup duplicated macro definition in vc4_firmware_kms Both vc4_drv.h and vc4_firmware_kms.c had definitions for to_vc4_crtc. Rename the fkms one to make it unique, and drop the magic define vc4_crtc vc4_kms_crtc define to_vc4_crtc to_vc4_kms_crtc that renamed half the variable and function names in a slightly unexpected way. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fix FKMS for when the YUV chroma planes are different buffers The code was assuming that it was a single buffer with offsets, when kmstest uses separate buffers and 0 offsets for each plane. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: fkms: Rename plane related functions The name collide with the Full KMS functions that are going to be made public. Signed-off-by: Maxime Ripard <maxime@cerno.tech> drm/vc4_fkms: Fix up interrupt handler for both 2835/2711 and 2712 2712 has switched from using the SMI peripheral to another interrupt source for the vsync interrupt, so handle both sources cleanly. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

This is a squash of all firmware-kms related patches from previous branches, up to and including "drm/vc4: Set the possible crtcs mask correctly for planes with FKMS" plus a couple of minor fixups for the 5.9 branch. Please refer to earlier branches for full history. This patch includes work by Eric Anholt, James Hughes, Phil Elwell, Dave Stevenson, Dom Cobley, and Jonathon Bell. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fixup firmware-kms after "drm/atomic: Pass the full state to CRTC atomic enable/disable" Prototype for those calls changed, so amend fkms (which isn't upstream) to match. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fixup fkms for API change Atomic flush and check changed API, so fix up the downstream-only FKMS driver. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Make normalize_zpos conditional on using fkms Eric's view was that there was no point in having zpos support on vc4 as all the planes had the same functionality. Can be later squashed into (and fixes): drm/vc4: Add firmware-kms mode Signed-off-by: Dom Cobley <popcornmix@gmail.com> drm/vc4: FKMS: Change of Broadcast RGB mode needs a mode change The Broadcast RGB (aka HDMI limited/full range) property is only notified to the firmware on mode change, so this needs to be signalled when set. raspberrypi/firmware#1580 Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> vc4/drv: Only notify firmware of display done with kms fkms driver still wants firmware display to be active Signed-off-by: Dom Cobley <popcornmix@gmail.com> ydrm/vc4: fkms: Fix margin calculations for the right/bottom edges The calculations clipped the right/bottom edge of the clipped range based on the left/top margins. #4447 Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: fkms: Use new devm_rpi_firmware_get api drm/kms: Add allow_fb_modifiers Signed-off-by: Dom Cobley <popcornmix@gmail.com> drm/vc4: Add async update support for cursor planes Now that cursors are implemented as regular planes, all cursor movements result in atomic updates. As the firmware-kms driver doesn't support asynchronous updates, these are synchronous, which limits the update rate to the screen refresh rate. Xorg seems unaware of this (or at least of the effect of this), because if the mouse is configured with a higher update rate than the screen then continuous mouse movement results in an increasing backlog of mouse events - cue extreme lag. Add minimal support for asynchronous updates - limited to cursor planes - to eliminate the lag. See: #4971 #4988 Signed-off-by: Phil Elwell <phil@raspberrypi.com> drivers/gpu/drm/vc4: Add missing 32-bit RGB formats The missing 32-bit per pixel ABGR and various "RGB with an X value" formats are added. Change sent by Dave Stevenson. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> drm: vc4: Fixup duplicated macro definition in vc4_firmware_kms Both vc4_drv.h and vc4_firmware_kms.c had definitions for to_vc4_crtc. Rename the fkms one to make it unique, and drop the magic define vc4_crtc vc4_kms_crtc define to_vc4_crtc to_vc4_kms_crtc that renamed half the variable and function names in a slightly unexpected way. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: Fix FKMS for when the YUV chroma planes are different buffers The code was assuming that it was a single buffer with offsets, when kmstest uses separate buffers and 0 offsets for each plane. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> drm/vc4: fkms: Rename plane related functions The name collide with the Full KMS functions that are going to be made public. Signed-off-by: Maxime Ripard <maxime@cerno.tech> drm/vc4_fkms: Fix up interrupt handler for both 2835/2711 and 2712 2712 has switched from using the SMI peripheral to another interrupt source for the vsync interrupt, so handle both sources cleanly. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

danvet and others added 2 commits March 31, 2022 14:35

Revert "drm/vc4: hvs: Defer dlist slots deallocation"

d62f36b

This reverts commit e99a1b6.

pelwell merged commit 6dafd55 into raspberrypi:rpi-5.15.y Apr 5, 2022

popcornmix mentioned this pull request Apr 11, 2022

Kernel 5.15.33 AARCH64 mouse movement with V3D #4988

Closed

davidplowman mentioned this pull request Apr 27, 2022

Ctrl+Alt+F1 does not switch out of the X Windows desktop to the console #5011

Closed

popcornmix mentioned this pull request Aug 23, 2022

drm/vc4: Add async update support for cursor planes #5141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cursor Corruption Fix, Upstream Edition #4971

Cursor Corruption Fix, Upstream Edition #4971

mripard commented Mar 31, 2022

popcornmix commented Apr 5, 2022

Cursor Corruption Fix, Upstream Edition #4971

Cursor Corruption Fix, Upstream Edition #4971

Conversation

mripard commented Mar 31, 2022

popcornmix commented Apr 5, 2022