4775988 fix(drm): move the hardware cursor once per vblank to stop tearing · neferwl

M internal/adapters/drm/cursor.go +30 -4


@@ -13,6 +13,10 @@ import (
13	13		const (
14	14		ioctlGetCap = 0xC010640C // DRM_IOWR('d', 0x0C, struct drm_get_cap)
15	15		ioctlCursor2 = 0xC02464BB // DRM_IOWR('d', 0xBB, struct drm_mode_cursor2)
	16	+	ioctlVblank = 0xC018643A // DRM_IOWR('d', 0x3A, union drm_wait_vblank)
	17	+	vblankRel = 0x1 // _DRM_VBLANK_RELATIVE
	18	+	vblankCrtcSh = 1 // _DRM_VBLANK_HIGH_CRTC_SHIFT
	19	+	vblankCrtcMk = 0x3e // _DRM_VBLANK_HIGH_CRTC_MASK
16	20		capCursorW = 0x8
17	21		capCursorH = 0x9
18	22		cursorBO = 0x01

@@ -21,6 +25,20 @@ const (
21	25
22	26		type getCap struct{ capability, value uint64 }
23	27
	28	+	// waitVblank is union drm_wait_vblank (request and reply share 24 bytes).
	29	+	type waitVblank struct {
	30	+	typ, sequence uint32
	31	+	sec, usec int64
	32	+	}
	33	+
	34	+	// vblankWaiter blocks until the next vblank of the CRTC at index pipe.
	35	+	func vblankWaiter(fd, pipe int) func() error {
	36	+	return func() error {
	37	+	v := waitVblank{typ: vblankRel \| uint32(pipe<<vblankCrtcSh)&vblankCrtcMk, sequence: 1}
	38	+	return ioctl(fd, ioctlVblank, unsafe.Pointer(&v))
	39	+	}
	40	+	}
	41	+
24	42		type modeCursor2 struct {
25	43		flags, crtcID uint32
26	44		x, y int32

@@ -32,9 +50,10 @@ type modeCursor2 struct {
32	50		// Cursor is the hardware cursor of one output. Move is safe to call from the
33	51		// input goroutine while the output goroutine changes the image.
34	52		//
35		-	// The legacy cursor ioctl can block until the next vblank (amdgpu does), so
36		-	// Move only records the position and a worker goroutine applies the latest
37		-	// one: input is never throttled to the refresh rate and motion is coalesced.
	53	+	// Move only records the position. A worker waits for the next vblank and then
	54	+	// applies the latest one, so the cursor moves at most once per frame, between
	55	+	// scanouts: moving it mid-scanout tears it into doubled images, and input is
	56	+	// never throttled by the ioctl.
38	57		type Cursor struct {
39	58		io sync.Mutex // serialises cursor ioctls and guards the fields below
40	59		fd int

@@ -44,6 +63,7 @@ type Cursor struct {
44	63		hotX, hotY int
45	64		shown bool
46	65		ioctl func(*modeCursor2) error
	66	+	vblank func() error // waits for the next vblank; nil moves at once
47	67
48	68		mu sync.Mutex // guards x, y and stats; never held across an ioctl
49	69		x, y int // physical position of the hotspot

@@ -72,7 +92,7 @@ func (c *Cursor) TakeStats() CursorStats {
72	92
73	93		// newCursor allocates a cursor buffer at the driver's preferred size (64 when
74	94		// the driver does not say).
75		-	func newCursor(fd int, crtc uint32) (*Cursor, error) {
	95	+	func newCursor(fd int, crtc uint32, pipe int) (*Cursor, error) {
76	96		// The plane is square: the smaller of the width and height caps.
77	97		size := 0
78	98		for _, c := range []uint64{capCursorW, capCursorH} {

@@ -97,6 +117,7 @@ func newCursor(fd int, crtc uint32) (*Cursor, error) {
97	117		}
98	118		c := &Cursor{fd: fd, crtc: crtc, buf: buf, size: size}
99	119		c.ioctl = func(v *modeCursor2) error { return ioctl(c.fd, ioctlCursor2, unsafe.Pointer(v)) }
	120	+	c.vblank = vblankWaiter(fd, pipe)
100	121		c.start()
101	122		return c, nil
102	123		}

@@ -113,6 +134,11 @@ func (c *Cursor) start() {
113	134		case <-c.stop:
114	135		return
115	136		case <-c.wake:
	137	+	// Errors (VT switched away) fall through: the move
	138	+	// fails too and Reapply restores the cursor.
	139	+	if c.vblank != nil {
	140	+	_ = c.vblank()
	141	+	}
116	142		c.io.Lock()
117	143		if c.shown && c.buf != nil {
118	144		x, y := c.position()

 const (
 	ioctlGetCap  = 0xC010640C // DRM_IOWR('d', 0x0C, struct drm_get_cap)
 	ioctlCursor2 = 0xC02464BB // DRM_IOWR('d', 0xBB, struct drm_mode_cursor2)
+	ioctlVblank  = 0xC018643A // DRM_IOWR('d', 0x3A, union drm_wait_vblank)
+	vblankRel    = 0x1        // _DRM_VBLANK_RELATIVE
+	vblankCrtcSh = 1          // _DRM_VBLANK_HIGH_CRTC_SHIFT
+	vblankCrtcMk = 0x3e       // _DRM_VBLANK_HIGH_CRTC_MASK
 	capCursorW   = 0x8
 	capCursorH   = 0x9
 	cursorBO     = 0x01
 type getCap struct{ capability, value uint64 }
+// waitVblank is union drm_wait_vblank (request and reply share 24 bytes).
+type waitVblank struct {
+	typ, sequence uint32
+	sec, usec     int64
+}
+// vblankWaiter blocks until the next vblank of the CRTC at index pipe.
+func vblankWaiter(fd, pipe int) func() error {
+	return func() error {
+		v := waitVblank{typ: vblankRel | uint32(pipe<<vblankCrtcSh)&vblankCrtcMk, sequence: 1}
+		return ioctl(fd, ioctlVblank, unsafe.Pointer(&v))
+	}
+}
 type modeCursor2 struct {
 	flags, crtcID uint32
 	x, y          int32
 // Cursor is the hardware cursor of one output. Move is safe to call from the
 // input goroutine while the output goroutine changes the image.
 //
-// The legacy cursor ioctl can block until the next vblank (amdgpu does), so
+// Move only records the position. A worker waits for the next vblank and then
-// Move only records the position and a worker goroutine applies the latest
+// applies the latest one, so the cursor moves at most once per frame, between
-// one: input is never throttled to the refresh rate and motion is coalesced.
+// scanouts: moving it mid-scanout tears it into doubled images, and input is
+// never throttled by the ioctl.
 type Cursor struct {
 	io         sync.Mutex // serialises cursor ioctls and guards the fields below
 	fd         int
 	hotX, hotY int
 	shown      bool
 	ioctl      func(*modeCursor2) error
+	vblank     func() error // waits for the next vblank; nil moves at once
 	mu    sync.Mutex // guards x, y and stats; never held across an ioctl
 	x, y  int        // physical position of the hotspot
 // newCursor allocates a cursor buffer at the driver's preferred size (64 when
 // the driver does not say).
-func newCursor(fd int, crtc uint32) (*Cursor, error) {
+func newCursor(fd int, crtc uint32, pipe int) (*Cursor, error) {
 	// The plane is square: the smaller of the width and height caps.
 	size := 0
 	for _, c := range []uint64{capCursorW, capCursorH} {
 	}
 	c := &Cursor{fd: fd, crtc: crtc, buf: buf, size: size}
 	c.ioctl = func(v *modeCursor2) error { return ioctl(c.fd, ioctlCursor2, unsafe.Pointer(v)) }
+	c.vblank = vblankWaiter(fd, pipe)
 	c.start()
 	return c, nil
 }
 			case <-c.stop:
 				return
 			case <-c.wake:
+				// Errors (VT switched away) fall through: the move
+				// fails too and Reapply restores the cursor.
+				if c.vblank != nil {
+					_ = c.vblank()
+				}
 				c.io.Lock()
 				if c.shown && c.buf != nil {
 					x, y := c.position()

M internal/adapters/drm/cursor_test.go +32 -0


@@ -15,6 +15,38 @@ func TestCursorABI(t *testing.T) {
15	15		if s := unsafe.Sizeof(getCap{}); s != 16 \|\| ioctlGetCap>>16&0x3fff != 16 {
16	16		t.Fatalf("drm_get_cap size %d", s)
17	17		}
	18	+	if s := unsafe.Sizeof(waitVblank{}); s != 24 \|\| ioctlVblank>>16&0x3fff != 24 {
	19	+	t.Fatalf("drm_wait_vblank size %d", s)
	20	+	}
	21	+	}
	22	+
	23	+	// The cursor moves once per vblank, to the latest position: moving it during
	24	+	// scanout tears it into doubled images.
	25	+	func TestCursorMovesOncePerVblank(t *testing.T) {
	26	+	vblank := make(chan struct{})
	27	+	moved := make(chan modeCursor2, 16)
	28	+	c := &Cursor{fd: -1, buf: &dumbBuffer{}, size: 64, shown: true}
	29	+	c.ioctl = func(v modeCursor2) error { moved <- v; return nil }
	30	+	c.vblank = func() error { <-vblank; return nil }
	31	+	c.start()
	32	+	defer func() { close(vblank); c.close() }()
	33	+	for i := range 100 {
	34	+	c.Move(float64(i), 0)
	35	+	}
	36	+	select {
	37	+	case v := <-moved:
	38	+	t.Fatalf("moved to %d before vblank", v.x)
	39	+	case <-time.After(20 * time.Millisecond):
	40	+	}
	41	+	vblank <- struct{}{}
	42	+	if v := <-moved; v.x != 99 {
	43	+	t.Fatalf("applied x=%d, want latest 99", v.x)
	44	+	}
	45	+	select {
	46	+	case v := <-moved:
	47	+	t.Fatalf("extra move to %d without new input", v.x)
	48	+	case <-time.After(20 * time.Millisecond):
	49	+	}
18	50		}
19	51
20	52		// The legacy cursor ioctl may wait for vblank; Move must not, or libinput


@@ -15,6 +15,38 @@ func TestCursorABI(t *testing.T) {
15	if s := unsafe.Sizeof(getCap{}); s != 16 \|\| ioctlGetCap>>16&0x3fff != 16 {	15		if s := unsafe.Sizeof(getCap{}); s != 16 \|\| ioctlGetCap>>16&0x3fff != 16 {
16	t.Fatalf("drm_get_cap size %d", s)	16		t.Fatalf("drm_get_cap size %d", s)
17	}	17		}
		18	+	if s := unsafe.Sizeof(waitVblank{}); s != 24 \|\| ioctlVblank>>16&0x3fff != 24 {
		19	+	t.Fatalf("drm_wait_vblank size %d", s)
		20	+	}
		21	+	}
		22	+
		23	+	// The cursor moves once per vblank, to the latest position: moving it during
		24	+	// scanout tears it into doubled images.
		25	+	func TestCursorMovesOncePerVblank(t *testing.T) {
		26	+	vblank := make(chan struct{})
		27	+	moved := make(chan modeCursor2, 16)
		28	+	c := &Cursor{fd: -1, buf: &dumbBuffer{}, size: 64, shown: true}
		29	+	c.ioctl = func(v modeCursor2) error { moved <- v; return nil }
		30	+	c.vblank = func() error { <-vblank; return nil }
		31	+	c.start()
		32	+	defer func() { close(vblank); c.close() }()
		33	+	for i := range 100 {
		34	+	c.Move(float64(i), 0)
		35	+	}
		36	+	select {
		37	+	case v := <-moved:
		38	+	t.Fatalf("moved to %d before vblank", v.x)
		39	+	case <-time.After(20 * time.Millisecond):
		40	+	}
		41	+	vblank <- struct{}{}
		42	+	if v := <-moved; v.x != 99 {
		43	+	t.Fatalf("applied x=%d, want latest 99", v.x)
		44	+	}
		45	+	select {
		46	+	case v := <-moved:
		47	+	t.Fatalf("extra move to %d without new input", v.x)
		48	+	case <-time.After(20 * time.Millisecond):
		49	+	}
18	}	50		}
19		51
20	// The legacy cursor ioctl may wait for vblank; Move must not, or libinput	52		// The legacy cursor ioctl may wait for vblank; Move must not, or libinput

M internal/adapters/drm/output.go +2 -1


@@ -4,6 +4,7 @@ import (
4	4		"context"
5	5		"errors"
6	6		"fmt"
	7	+	"slices"
7	8		"sync"
8	9		"time"
9	10

@@ -96,7 +97,7 @@ func Open(fd int, card string, want Want, log zerowrap.Logger) (*Output, error)
96	97		return nil, err
97	98		}
98	99		}
99		-	if o.cursor, err = newCursor(fd, crtc); err != nil {
	100	+	if o.cursor, err = newCursor(fd, crtc, slices.Index(crtcs, crtc)); err != nil {
100	101		log.Warn().Err(err).Msg("no hardware cursor")
101	102		}
102	103		log.Info().Str("card", card).Str("connector", c.name).Str("mode", mode.String()).Str("make", o.monitor.Make).Str("model", o.monitor.Model).Uint32("crtc", crtc).Msg("output")

 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"sync"
 	"time"
 			return nil, err
 		}
 	}
-	if o.cursor, err = newCursor(fd, crtc); err != nil {
+	if o.cursor, err = newCursor(fd, crtc, slices.Index(crtcs, crtc)); err != nil {
 		log.Warn().Err(err).Msg("no hardware cursor")
 	}
 	log.Info().Str("card", card).Str("connector", c.name).Str("mode", mode.String()).Str("make", o.monitor.Make).Str("model", o.monitor.Model).Uint32("crtc", crtc).Msg("output")