1941387 fix(render): drop exported targets on fallback, validate images on first enable, host-read barrier · neferwl

M internal/adapters/drm/output.go +50 -22


@@ -14,7 +14,7 @@ import (
14	14		"golang.org/x/sys/unix"
15	15		)
16	16
17		-	// Output is one KMS connector driven with two dumb buffers. One goroutine
	17	+	// Output is one KMS connector flipping between two images. One goroutine
18	18		// runs it (Run) and closes it.
19	19		type Output struct {
20	20		fd int

@@ -44,13 +44,16 @@ type Output struct {
44	44		shown, queued uint64
45	45		reason string // why the last frame was composed ("" = scanout)
46	46		unsent ports.OutputPresented
	47	+	// kind is how the images were made; validated once a modeset took them.
	48	+	kind imageKind
	49	+	validated bool
47	50		}
48	51
49	52		// CursorLoader returns the image of a cursor at an output scale, at most
50	53		// limit pixels on a side; an empty image hides the cursor.
51	54		type CursorLoader func(c ports.CursorChange, scale float64, limit int) (ports.CursorImage, error)
52	55
53		-	// newOutput allocates buffers and the cursor for a connector on crtc.
	56	+	// newOutput prepares the cursor and saves the CRTC for a connector on crtc.
54	57		func newOutput(card Card, c connector, mode modeInfo, crtc uint32) (Output, error) {
55	58		log := card.log
56	59		o := &Output{fd: card.fd, flipped: card.flips[crtc], crtc: crtc, conn: c, mode: mode, log: log, monitor: readMonitor(card.path, c.name), card: card, scanout: !card.want.NoScanout, clientFBs: map[uint64]*clientFB{}, reason: "start"}

@@ -180,23 +183,15 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend
180	183		case enabled = <-active:
181	184		default:
182	185		}
183		-	// Output images, from the best to the fallback (ADR 014). KMS may
184		-	// refuse an image only at modeset: then the next kind is tried.
185		-	for kind := imagesDriver; ; kind++ {
186		-	got, err := o.setupImages(r, kind)
187		-	if err == nil && enabled {
188		-	if err = o.modeset(); err != nil {
189		-	o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(got)).Msg("modeset refused the output images")
190		-	o.freeImages()
191		-	}
192		-	}
193		-	if err == nil {
194		-	break
195		-	}
196		-	if got == imagesDumb {
197		-	return err
198		-	}
199		-	kind = got
	186	+	// Output images, from the best to the fallback (ADR 014). While
	187	+	// switched away they are validated by the first modeset on enable.
	188	+	if enabled {
	189	+	err = o.showImages(r, imagesDriver, nil)
	190	+	} else {
	191	+	o.kind, err = o.setupImages(r, imagesDriver, nil)
	192	+	}
	193	+	if err != nil {
	194	+	return err
200	195		}
201	196		surfaces := make(map[ports.WindowID]ports.SurfaceContent)
202	197		var scene ports.Scene

@@ -231,10 +226,17 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend
231	226		case on := <-active:
232	227		// Modeset on every enable: a fast disable+enable can coalesce to one true.
233	228		if on {
234		-	if err := o.modeset(); err != nil {
	229	+	err := o.modeset()
	230	+	if err != nil && !o.validated && o.kind < imagesDumb {
	231	+	o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(o.kind)).Msg("modeset refused the output images")
	232	+	o.freeImages()
	233	+	err = o.showImages(r, o.kind+1, err)
	234	+	}
	235	+	if err != nil {
235	236		o.log.Error().Err(err).Msg("resume")
236	237		return err
237	238		}
	239	+	o.validated = true
238	240		dirty = haveScene
239	241		if o.cursor != nil {
240	242		if err := o.cursor.Reapply(); err != nil {

@@ -370,11 +372,34 @@ const (
370	372		imagesDumb // dumb buffers filled by CPU copies
371	373		)
372	374
	375	+	// showImages sets up images from kind on and modesets them. KMS may refuse
	376	+	// an image only at modeset: then the next kind is tried. cause is why the
	377	+	// previous kind failed, for the log.
	378	+	func (o *Output) showImages(r ports.Renderer, kind imageKind, cause error) error {
	379	+	for {
	380	+	got, err := o.setupImages(r, kind, cause)
	381	+	if err != nil {
	382	+	return err
	383	+	}
	384	+	o.kind = got
	385	+	if err = o.modeset(); err == nil {
	386	+	o.validated = true
	387	+	return nil
	388	+	}
	389	+	o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(got)).Msg("modeset refused the output images")
	390	+	o.freeImages()
	391	+	if got == imagesDumb {
	392	+	return err
	393	+	}
	394	+	kind, cause = got+1, err
	395	+	}
	396	+	}
	397	+
373	398		// setupImages gives the output the two images it flips between: renderer
374	399		// targets exported as dmabufs, or dumb buffers as a logged fallback. A
375	400		// kind that fails falls through to the next one.
376		-	func (o *Output) setupImages(r ports.Renderer, kind imageKind) (imageKind, error) {
377		-	var err error
	401	+	func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (imageKind, error) {
	402	+	err := cause
378	403		for ; kind < imagesDumb; kind++ {
379	404		mods := []uint64(nil)
380	405		if kind == imagesLinear {

@@ -386,6 +411,8 @@ func (o *Output) setupImages(r ports.Renderer, kind imageKind) (imageKind, error
386	411		o.log.Info().Err(err).Str("connector", o.conn.name).Int("kind", int(kind)).Msg("output image export")
387	412		}
388	413		o.log.Warn().Err(err).Str("connector", o.conn.name).Msg("output falls back to CPU copies (ADR 014)")
	414	+	// The renderer draws into its own image again, read back by CopyBGRX.
	415	+	_, _ = r.ExportTargets(0, nil)
389	416		for i := range o.bufs {
390	417		if o.bufs[i], err = newDumb(o.fd, o.Width(), o.Height()); err != nil {
391	418		o.freeImages()

@@ -419,6 +446,7 @@ func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
419	446		}
420	447		if err != nil {
421	448		o.freeImages()
	449	+	_, _ = r.ExportTargets(0, nil)
422	450		return err
423	451		}
424	452		o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Msg("zero-copy output")

 	"golang.org/x/sys/unix"
 )
-// Output is one KMS connector driven with two dumb buffers. One goroutine
+// Output is one KMS connector flipping between two images. One goroutine
 // runs it (Run) and closes it.
 type Output struct {
 	fd      int
 	shown, queued uint64
 	reason        string // why the last frame was composed ("" = scanout)
 	unsent        ports.OutputPresented
+	// kind is how the images were made; validated once a modeset took them.
+	kind      imageKind
+	validated bool
 }
 // CursorLoader returns the image of a cursor at an output scale, at most
 // limit pixels on a side; an empty image hides the cursor.
 type CursorLoader func(c ports.CursorChange, scale float64, limit int) (ports.CursorImage, error)
-// newOutput allocates buffers and the cursor for a connector on crtc.
+// newOutput prepares the cursor and saves the CRTC for a connector on crtc.
 func newOutput(card *Card, c connector, mode modeInfo, crtc uint32) (*Output, error) {
 	log := card.log
 	o := &Output{fd: card.fd, flipped: card.flips[crtc], crtc: crtc, conn: c, mode: mode, log: log, monitor: readMonitor(card.path, c.name), card: card, scanout: !card.want.NoScanout, clientFBs: map[uint64]*clientFB{}, reason: "start"}
 	case enabled = <-active:
 	default:
 	}
-	// Output images, from the best to the fallback (ADR 014). KMS may
+	// Output images, from the best to the fallback (ADR 014). While
-	// refuse an image only at modeset: then the next kind is tried.
+	// switched away they are validated by the first modeset on enable.
-	for kind := imagesDriver; ; kind++ {
+	if enabled {
-		got, err := o.setupImages(r, kind)
+		err = o.showImages(r, imagesDriver, nil)
-		if err == nil && enabled {
+	} else {
-			if err = o.modeset(); err != nil {
+		o.kind, err = o.setupImages(r, imagesDriver, nil)
-				o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(got)).Msg("modeset refused the output images")
+	}
-				o.freeImages()
+	if err != nil {
-			}
+		return err
-		}
-		if err == nil {
-			break
-		}
-		if got == imagesDumb {
-			return err
-		}
-		kind = got
 	}
 	surfaces := make(map[ports.WindowID]ports.SurfaceContent)
 	var scene ports.Scene
 		case on := <-active:
 			// Modeset on every enable: a fast disable+enable can coalesce to one true.
 			if on {
-				if err := o.modeset(); err != nil {
+				err := o.modeset()
+				if err != nil && !o.validated && o.kind < imagesDumb {
+					o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(o.kind)).Msg("modeset refused the output images")
+					o.freeImages()
+					err = o.showImages(r, o.kind+1, err)
+				}
+				if err != nil {
 					o.log.Error().Err(err).Msg("resume")
 					return err
 				}
+				o.validated = true
 				dirty = haveScene
 				if o.cursor != nil {
 					if err := o.cursor.Reapply(); err != nil {
 	imagesDumb                    // dumb buffers filled by CPU copies
 )
+// showImages sets up images from kind on and modesets them. KMS may refuse
+// an image only at modeset: then the next kind is tried. cause is why the
+// previous kind failed, for the log.
+func (o *Output) showImages(r ports.Renderer, kind imageKind, cause error) error {
+	for {
+		got, err := o.setupImages(r, kind, cause)
+		if err != nil {
+			return err
+		}
+		o.kind = got
+		if err = o.modeset(); err == nil {
+			o.validated = true
+			return nil
+		}
+		o.log.Warn().Err(err).Str("connector", o.conn.name).Int("kind", int(got)).Msg("modeset refused the output images")
+		o.freeImages()
+		if got == imagesDumb {
+			return err
+		}
+		kind, cause = got+1, err
+	}
+}
 // setupImages gives the output the two images it flips between: renderer
 // targets exported as dmabufs, or dumb buffers as a logged fallback. A
 // kind that fails falls through to the next one.
-func (o *Output) setupImages(r ports.Renderer, kind imageKind) (imageKind, error) {
+func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (imageKind, error) {
-	var err error
+	err := cause
 	for ; kind < imagesDumb; kind++ {
 		mods := []uint64(nil)
 		if kind == imagesLinear {
 		o.log.Info().Err(err).Str("connector", o.conn.name).Int("kind", int(kind)).Msg("output image export")
 	}
 	o.log.Warn().Err(err).Str("connector", o.conn.name).Msg("output falls back to CPU copies (ADR 014)")
+	// The renderer draws into its own image again, read back by CopyBGRX.
+	_, _ = r.ExportTargets(0, nil)
 	for i := range o.bufs {
 		if o.bufs[i], err = newDumb(o.fd, o.Width(), o.Height()); err != nil {
 			o.freeImages()
 	}
 	if err != nil {
 		o.freeImages()
+		_, _ = r.ExportTargets(0, nil)
 		return err
 	}
 	o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Msg("zero-copy output")

M internal/adapters/vulkan/renderer.go +9 -0


@@ -746,6 +746,12 @@ func (r *Renderer) CopyBGRX(dst []byte, pitch int) {
746	746		}
747	747
748	748		// readback copies the last frame into the host buffer, once per frame.
	749	+	// Missing from the bindings: VK_QUEUE_FAMILY_IGNORED, VK_WHOLE_SIZE.
	750	+	const (
	751	+	queueFamilyIgnored = ^uint32(0)
	752	+	wholeSize = ^vk.DeviceSize(0)
	753	+	)
	754	+
749	755		func (r *Renderer) readback() error {
750	756		if r.readBack \|\| r.last == nil \|\| r.last.image == 0 {
751	757		return nil

@@ -768,6 +774,9 @@ func (r *Renderer) readback() error {
768	774		d.CmdPipelineBarrier(r.command, vk.PipelineStageTransferBit, vk.PipelineStageTransferBit, 0, 0, nil, 0, nil, 1, &b)
769	775		region := vk.BufferImageCopy{ImageSubresource: vk.ImageSubresourceLayers{AspectMask: vk.ImageAspectColorBit, LayerCount: 1}, ImageExtent: vk.Extent3D{Width: uint32(r.width), Height: uint32(r.height), Depth: 1}}
770	776		d.CmdCopyImageToBuffer(r.command, t.image, vk.ImageLayoutTransferSrcOptimal, r.buffer, 1, &region)
	777	+	// Make the copy visible to host reads of the mapped buffer.
	778	+	hb := vk.BufferMemoryBarrier{SType: vk.StructureTypeBufferMemoryBarrier, SrcAccessMask: vk.AccessTransferWriteBit, DstAccessMask: vk.AccessHostReadBit, SrcQueueFamilyIndex: queueFamilyIgnored, DstQueueFamilyIndex: queueFamilyIgnored, Buffer: r.buffer, Size: wholeSize}
	779	+	d.CmdPipelineBarrier(r.command, vk.PipelineStageTransferBit, vk.PipelineStageHostBit, 0, 0, nil, 1, &hb, 0, nil)
771	780		if t.exported {
772	781		b.OldLayout, b.NewLayout = vk.ImageLayoutTransferSrcOptimal, vk.ImageLayoutGeneral
773	782		b.SrcAccessMask, b.DstAccessMask = vk.AccessTransferReadBit, 0


@@ -746,6 +746,12 @@ func (r *Renderer) CopyBGRX(dst []byte, pitch int) {
746	}	746		}
747		747
748	// readback copies the last frame into the host buffer, once per frame.	748		// readback copies the last frame into the host buffer, once per frame.
		749	+	// Missing from the bindings: VK_QUEUE_FAMILY_IGNORED, VK_WHOLE_SIZE.
		750	+	const (
		751	+	queueFamilyIgnored = ^uint32(0)
		752	+	wholeSize = ^vk.DeviceSize(0)
		753	+	)
		754	+
749	func (r *Renderer) readback() error {	755		func (r *Renderer) readback() error {
750	if r.readBack \|\| r.last == nil \|\| r.last.image == 0 {	756		if r.readBack \|\| r.last == nil \|\| r.last.image == 0 {
751	return nil	757		return nil

@@ -768,6 +774,9 @@ func (r *Renderer) readback() error {
768	d.CmdPipelineBarrier(r.command, vk.PipelineStageTransferBit, vk.PipelineStageTransferBit, 0, 0, nil, 0, nil, 1, &b)	774		d.CmdPipelineBarrier(r.command, vk.PipelineStageTransferBit, vk.PipelineStageTransferBit, 0, 0, nil, 0, nil, 1, &b)
769	region := vk.BufferImageCopy{ImageSubresource: vk.ImageSubresourceLayers{AspectMask: vk.ImageAspectColorBit, LayerCount: 1}, ImageExtent: vk.Extent3D{Width: uint32(r.width), Height: uint32(r.height), Depth: 1}}	775		region := vk.BufferImageCopy{ImageSubresource: vk.ImageSubresourceLayers{AspectMask: vk.ImageAspectColorBit, LayerCount: 1}, ImageExtent: vk.Extent3D{Width: uint32(r.width), Height: uint32(r.height), Depth: 1}}
770	d.CmdCopyImageToBuffer(r.command, t.image, vk.ImageLayoutTransferSrcOptimal, r.buffer, 1, &region)	776		d.CmdCopyImageToBuffer(r.command, t.image, vk.ImageLayoutTransferSrcOptimal, r.buffer, 1, &region)
		777	+	// Make the copy visible to host reads of the mapped buffer.
		778	+	hb := vk.BufferMemoryBarrier{SType: vk.StructureTypeBufferMemoryBarrier, SrcAccessMask: vk.AccessTransferWriteBit, DstAccessMask: vk.AccessHostReadBit, SrcQueueFamilyIndex: queueFamilyIgnored, DstQueueFamilyIndex: queueFamilyIgnored, Buffer: r.buffer, Size: wholeSize}
		779	+	d.CmdPipelineBarrier(r.command, vk.PipelineStageTransferBit, vk.PipelineStageHostBit, 0, 0, nil, 1, &hb, 0, nil)
771	if t.exported {	780		if t.exported {
772	b.OldLayout, b.NewLayout = vk.ImageLayoutTransferSrcOptimal, vk.ImageLayoutGeneral	781		b.OldLayout, b.NewLayout = vk.ImageLayoutTransferSrcOptimal, vk.ImageLayoutGeneral
773	b.SrcAccessMask, b.DstAccessMask = vk.AccessTransferReadBit, 0	782		b.SrcAccessMask, b.DstAccessMask = vk.AccessTransferReadBit, 0

M internal/adapters/vulkan/targets.go +9 -2


@@ -32,8 +32,13 @@ type target struct {
32	32		// fourccXRGB is DRM_FORMAT_XRGB8888: B8G8R8A8 in memory, alpha ignored.
33	33		const fourccXRGB = 'X' \| 'R'<<8 \| '2'<<16 \| '4'<<24
34	34
35		-	// ExportTargets allocates n exported images of the output size.
	35	+	// ExportTargets allocates n exported images of the output size, replacing
	36	+	// the previous ones; n = 0 only drops them (back to the internal image).
36	37		func (r *Renderer) ExportTargets(n int, modifiers []uint64) ([]ports.DMABuf, error) {
	38	+	r.dropTargets()
	39	+	if n == 0 {
	40	+	return nil, nil
	41	+	}
37	42		if r.dd.GetMemoryFdKHR == nil \|\| r.dd.GetImageDrmFormatModifierPropertiesEXT == nil \|\| len(r.dmabuf.Formats) == 0 {
38	43		return nil, errors.New("device cannot export dmabufs")
39	44		}

@@ -41,7 +46,6 @@ func (r *Renderer) ExportTargets(n int, modifiers []uint64) ([]ports.DMABuf, err
41	46		if len(mods) == 0 {
42	47		return nil, errors.New("no XRGB8888 modifier both the device and the display accept")
43	48		}
44		-	r.dropTargets()
45	49		var out []ports.DMABuf
46	50		for range n {
47	51		t, buf, err := r.exportTarget(mods)

@@ -197,6 +201,9 @@ func (r *Renderer) dropTargets() {
197	201		_ = checked("vkDeviceWaitIdle", r.dd.DeviceWaitIdle(r.device))
198	202		}
199	203		for _, t := range r.targets {
	204	+	if r.last == t {
	205	+	r.last = nil
	206	+	}
200	207		r.freeTarget(t)
201	208		}
202	209		r.targets, r.current = nil, 0

 // fourccXRGB is DRM_FORMAT_XRGB8888: B8G8R8A8 in memory, alpha ignored.
 const fourccXRGB = 'X' | 'R'<<8 | '2'<<16 | '4'<<24
-// ExportTargets allocates n exported images of the output size.
+// ExportTargets allocates n exported images of the output size, replacing
+// the previous ones; n = 0 only drops them (back to the internal image).
 func (r *Renderer) ExportTargets(n int, modifiers []uint64) ([]ports.DMABuf, error) {
+	r.dropTargets()
+	if n == 0 {
+		return nil, nil
+	}
 	if r.dd.GetMemoryFdKHR == nil || r.dd.GetImageDrmFormatModifierPropertiesEXT == nil || len(r.dmabuf.Formats) == 0 {
 		return nil, errors.New("device cannot export dmabufs")
 	}
 	if len(mods) == 0 {
 		return nil, errors.New("no XRGB8888 modifier both the device and the display accept")
 	}
-	r.dropTargets()
 	var out []ports.DMABuf
 	for range n {
 		t, buf, err := r.exportTarget(mods)
 		_ = checked("vkDeviceWaitIdle", r.dd.DeviceWaitIdle(r.device))
 	}
 	for _, t := range r.targets {
+		if r.last == t {
+			r.last = nil
+		}
 		r.freeTarget(t)
 	}
 	r.targets, r.current = nil, 0

M internal/core/screens_test.go +2 -2


@@ -494,8 +494,8 @@ func TestStateFollowsAppIDAndHiddenWorkspace(t *testing.T) {
494	494		// An app ID set after map reaches the state.
495	495		r.client <- ports.WindowAppID{ID: 1, AppID: "foot"}
496	496		receive(t, r.scenes)
497		-	if st := receive(t, r.state); st.Windows[0].AppID != "foot" {
498		-	t.Fatalf("%+v", st)
	497	+	// Snapshots from before the app ID may still be queued.
	498	+	for st := receive(t, r.state); st.Windows[0].AppID != "foot"; st = receive(t, r.state) {
499	499		}
500	500		// A hidden workspace on screen: no active number, its name, window 1 off screen.
501	501		r.key(t, "n", ports.ModAlt)


@@ -494,8 +494,8 @@ func TestStateFollowsAppIDAndHiddenWorkspace(t *testing.T) {
494		// An app ID set after map reaches the state.	494		// An app ID set after map reaches the state.
495		r.client <- ports.WindowAppID{ID: 1, AppID: "foot"}	495		r.client <- ports.WindowAppID{ID: 1, AppID: "foot"}
496		receive(t, r.scenes)	496		receive(t, r.scenes)
497	-	if st := receive(t, r.state); st.Windows[0].AppID != "foot" {	497	+	// Snapshots from before the app ID may still be queued.
498	-	t.Fatalf("%+v", st)	498	+	for st := receive(t, r.state); st.Windows[0].AppID != "foot"; st = receive(t, r.state) {
499		}	499		}
500		// A hidden workspace on screen: no active number, its name, window 1 off screen.	500		// A hidden workspace on screen: no active number, its name, window 1 off screen.
501		r.key(t, "n", ports.ModAlt)	501		r.key(t, "n", ports.ModAlt)

M internal/ports/devices.go +1 -0


@@ -10,6 +10,7 @@ type Renderer interface {
10	10		// ExportTargets allocates n images of the renderer's size that the
11	11		// display can scan out, with one of the given XRGB8888 modifiers
12	12		// (none: any the device exports), and returns them as dmabufs.
	13	+	// n = 0 drops the targets.
13	14		ExportTargets(n int, modifiers []uint64) ([]DMABuf, error)
14	15		// UseTarget selects the exported image the next Render draws into.
15	16		UseTarget(i int)


@@ -10,6 +10,7 @@ type Renderer interface {
10	// ExportTargets allocates n images of the renderer's size that the	10		// ExportTargets allocates n images of the renderer's size that the
11	// display can scan out, with one of the given XRGB8888 modifiers	11		// display can scan out, with one of the given XRGB8888 modifiers
12	// (none: any the device exports), and returns them as dmabufs.	12		// (none: any the device exports), and returns them as dmabufs.
		13	+	// n = 0 drops the targets.
13	ExportTargets(n int, modifiers []uint64) ([]DMABuf, error)	14		ExportTargets(n int, modifiers []uint64) ([]DMABuf, error)
14	// UseTarget selects the exported image the next Render draws into.	15		// UseTarget selects the exported image the next Render draws into.
15	UseTarget(i int)	16		UseTarget(i int)