detached 5491ddd

feat(drm): retry single-plane images before linear and close every plane

showImages tries driver images, then single-plane driver images, then
linear. exportImages closes every plane file of every buffer and rejects
buffers with more planes than a framebuffer holds.
commit
5491ddd253141abd596e35c3e33df63941555de5
author
brice · · 2026-10-04 20:02 UTC
parent
a13ea2f
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +73 -12
M internal/adapters/drm/commit_test.go +50 -5
@@ -2,6 +2,7 @@ package drm
22
33 import (
44 "context"
5+ "errors"
56 "os"
67 "testing"
78 "time"
@@ -111,10 +112,11 @@ func TestCursorRefusedDoesNotBlameScanoutBuffer(t *testing.T) {
111112 }
112113 }
113114
114-// showImages validates the images with a TEST_ONLY modeset and falls back
115-// to linear images when KMS refuses the driver's.
115+// showImages validates the images with a TEST_ONLY modeset, retries with
116+// single-plane driver images, then falls back to linear images when KMS
117+// refuses both.
116118 func TestShowImagesTestsBeforeModeset(t *testing.T) {
117- o, k, commits := testOutput(t, unix.EINVAL)
119+ o, k, commits := testOutput(t, unix.EINVAL, unix.EINVAL)
118120 o.cursor = nil
119121 r := portsmocks.NewMockRenderer(t)
120122 buf := func() ports.DMABuf {
@@ -124,7 +126,7 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
124126 }
125127 r.EXPECT().SetHDR(float64(0)).Return().Maybe()
126128 r.EXPECT().ExportTargets(2, []uint64(nil), false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
127- r.EXPECT().SetHDR(float64(0)).Return().Maybe()
129+ r.EXPECT().ExportTargets(2, []uint64(nil), true).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
128130 r.EXPECT().ExportTargets(2, []uint64{0}, false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
129131 r.EXPECT().UseTarget(mock.Anything).Return()
130132 r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil)
@@ -135,7 +137,7 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
135137 if err := o.showImages(r, imagesDriver, nil); err != nil {
136138 t.Fatal(err)
137139 }
138- want := []uint32{atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicAllowModes}
140+ want := []uint32{atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicAllowModes}
139141 if len(*commits) != len(want) {
140142 t.Fatalf("%d commits", len(*commits))
141143 }
@@ -149,6 +151,49 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
149151 }
150152 }
151153
154+// A refused multi-plane (DCC) driver image is retried with single-plane
155+// driver images before linear; every plane file of every buffer is closed
156+// once the framebuffers exist.
157+func TestShowImagesRetriesSinglePlaneBeforeLinear(t *testing.T) {
158+ o, k, _ := testOutput(t, unix.EINVAL)
159+ o.cursor = nil
160+ var files []*os.File
161+ buf := func(planes int) ports.DMABuf {
162+ b := ports.DMABuf{}
163+ for range planes {
164+ f, w, _ := os.Pipe()
165+ w.Close()
166+ files = append(files, f)
167+ b.Planes = append(b.Planes, ports.DMABufPlane{File: f})
168+ }
169+ return b
170+ }
171+ r := portsmocks.NewMockRenderer(t)
172+ r.EXPECT().SetHDR(float64(0)).Return().Maybe()
173+ r.EXPECT().ExportTargets(2, []uint64(nil), false).Return([]ports.DMABuf{buf(3), buf(3)}, nil).Once()
174+ r.EXPECT().ExportTargets(2, []uint64(nil), true).Return([]ports.DMABuf{buf(1), buf(1)}, nil).Once()
175+ r.EXPECT().UseTarget(mock.Anything).Return()
176+ r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil)
177+ k.EXPECT().addFB(mock.Anything, uint32(fourccXRGB)).Return(70, nil)
178+ k.EXPECT().rmFB(mock.Anything).Return(nil)
179+ k.EXPECT().createBlob(mock.Anything).Return(99, nil)
180+ k.EXPECT().destroyBlob(mock.Anything).Return(nil)
181+ if err := o.showImages(r, imagesDriver, nil); err != nil {
182+ t.Fatal(err)
183+ }
184+ if o.kind != imagesSinglePlane {
185+ t.Fatalf("kind %d", o.kind)
186+ }
187+ if len(files) != 8 {
188+ t.Fatalf("%d plane files", len(files))
189+ }
190+ for i, f := range files {
191+ if err := f.Close(); !errors.Is(err, os.ErrClosed) {
192+ t.Fatalf("plane file %d left open (close: %v)", i, err)
193+ }
194+ }
195+}
196+
152197 // Enabling an output whose images were not made yet (started switched
153198 // away) tests them and modesets once.
154199 func TestEnableAfterStartAwayModesetsOnce(t *testing.T) {
@@ -2,6 +2,7 @@ package drm
2 2
3 import (3 import (
4 "context"4 "context"
5+ "errors"
5 "os"6 "os"
6 "testing"7 "testing"
7 "time"8 "time"
@@ -111,10 +112,11 @@ func TestCursorRefusedDoesNotBlameScanoutBuffer(t *testing.T) {
111 }112 }
112 }113 }
113 114
114-// showImages validates the images with a TEST_ONLY modeset and falls back115+// showImages validates the images with a TEST_ONLY modeset, retries with
115-// to linear images when KMS refuses the driver's.116+// single-plane driver images, then falls back to linear images when KMS
117+// refuses both.
116 func TestShowImagesTestsBeforeModeset(t *testing.T) {118 func TestShowImagesTestsBeforeModeset(t *testing.T) {
117- o, k, commits := testOutput(t, unix.EINVAL)119+ o, k, commits := testOutput(t, unix.EINVAL, unix.EINVAL)
118 o.cursor = nil120 o.cursor = nil
119 r := portsmocks.NewMockRenderer(t)121 r := portsmocks.NewMockRenderer(t)
120 buf := func() ports.DMABuf {122 buf := func() ports.DMABuf {
@@ -124,7 +126,7 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
124 }126 }
125 r.EXPECT().SetHDR(float64(0)).Return().Maybe()127 r.EXPECT().SetHDR(float64(0)).Return().Maybe()
126 r.EXPECT().ExportTargets(2, []uint64(nil), false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()128 r.EXPECT().ExportTargets(2, []uint64(nil), false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
127- r.EXPECT().SetHDR(float64(0)).Return().Maybe()129+ r.EXPECT().ExportTargets(2, []uint64(nil), true).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
128 r.EXPECT().ExportTargets(2, []uint64{0}, false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()130 r.EXPECT().ExportTargets(2, []uint64{0}, false).Return([]ports.DMABuf{buf(), buf()}, nil).Once()
129 r.EXPECT().UseTarget(mock.Anything).Return()131 r.EXPECT().UseTarget(mock.Anything).Return()
130 r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil)132 r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil)
@@ -135,7 +137,7 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
135 if err := o.showImages(r, imagesDriver, nil); err != nil {137 if err := o.showImages(r, imagesDriver, nil); err != nil {
136 t.Fatal(err)138 t.Fatal(err)
137 }139 }
138- want := []uint32{atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicAllowModes}140+ want := []uint32{atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicTestOnly | atomicAllowModes, atomicAllowModes}
139 if len(*commits) != len(want) {141 if len(*commits) != len(want) {
140 t.Fatalf("%d commits", len(*commits))142 t.Fatalf("%d commits", len(*commits))
141 }143 }
@@ -149,6 +151,49 @@ func TestShowImagesTestsBeforeModeset(t *testing.T) {
149 }151 }
150 }152 }
151 153
154+// A refused multi-plane (DCC) driver image is retried with single-plane
155+// driver images before linear; every plane file of every buffer is closed
156+// once the framebuffers exist.
157+func TestShowImagesRetriesSinglePlaneBeforeLinear(t *testing.T) {
158+ o, k, _ := testOutput(t, unix.EINVAL)
159+ o.cursor = nil
160+ var files []*os.File
161+ buf := func(planes int) ports.DMABuf {
162+ b := ports.DMABuf{}
163+ for range planes {
164+ f, w, _ := os.Pipe()
165+ w.Close()
166+ files = append(files, f)
167+ b.Planes = append(b.Planes, ports.DMABufPlane{File: f})
168+ }
169+ return b
170+ }
171+ r := portsmocks.NewMockRenderer(t)
172+ r.EXPECT().SetHDR(float64(0)).Return().Maybe()
173+ r.EXPECT().ExportTargets(2, []uint64(nil), false).Return([]ports.DMABuf{buf(3), buf(3)}, nil).Once()
174+ r.EXPECT().ExportTargets(2, []uint64(nil), true).Return([]ports.DMABuf{buf(1), buf(1)}, nil).Once()
175+ r.EXPECT().UseTarget(mock.Anything).Return()
176+ r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil)
177+ k.EXPECT().addFB(mock.Anything, uint32(fourccXRGB)).Return(70, nil)
178+ k.EXPECT().rmFB(mock.Anything).Return(nil)
179+ k.EXPECT().createBlob(mock.Anything).Return(99, nil)
180+ k.EXPECT().destroyBlob(mock.Anything).Return(nil)
181+ if err := o.showImages(r, imagesDriver, nil); err != nil {
182+ t.Fatal(err)
183+ }
184+ if o.kind != imagesSinglePlane {
185+ t.Fatalf("kind %d", o.kind)
186+ }
187+ if len(files) != 8 {
188+ t.Fatalf("%d plane files", len(files))
189+ }
190+ for i, f := range files {
191+ if err := f.Close(); !errors.Is(err, os.ErrClosed) {
192+ t.Fatalf("plane file %d left open (close: %v)", i, err)
193+ }
194+ }
195+}
196+
152 // Enabling an output whose images were not made yet (started switched197 // Enabling an output whose images were not made yet (started switched
153 // away) tests them and modesets once.198 // away) tests them and modesets once.
154 func TestEnableAfterStartAwayModesetsOnce(t *testing.T) {199 func TestEnableAfterStartAwayModesetsOnce(t *testing.T) {
M internal/adapters/drm/images.go +23 -7
@@ -17,10 +17,14 @@ import (
1717 type imageKind int
1818
1919 const (
20- imagesDriver imageKind = iota // exported, modifier chosen by the driver
21- imagesLinear // exported, linear
20+ imagesDriver imageKind = iota // exported, modifier chosen by the driver (DCC planes allowed)
21+ imagesSinglePlane // exported, driver modifier with one memory plane
22+ imagesLinear // exported, linear
2223 )
2324
25+// maxFBPlanes is the number of planes a KMS framebuffer holds (fbCmd2).
26+const maxFBPlanes = 4
27+
2428 // showImages sets up images from kind on and modesets them. KMS may refuse
2529 // an image only in a modeset: a TEST_ONLY modeset checks it first, and the
2630 // next kind is tried on refusal. cause is why the
@@ -131,7 +135,12 @@ func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (ima
131135 } else if kind == imagesLinear {
132136 mods = []uint64{0}
133137 }
134- if err = o.exportImages(r, mods); err == nil {
138+ if kind == imagesSinglePlane && len(mods) == 1 && mods[0] == 0 {
139+ // The display only lists linear, already one plane: the driver
140+ // attempt was the single-plane one.
141+ continue
142+ }
143+ if err = o.exportImages(r, mods, kind == imagesSinglePlane); err == nil {
135144 return kind, nil
136145 }
137146 o.log.Info().Err(err).Str("connector", o.conn.name).Int("kind", int(kind)).Msg("output image export")
@@ -140,12 +149,15 @@ func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (ima
140149 }
141150
142151 // exportImages makes the renderer's exported targets the output images.
143-func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
144- bufs, err := r.ExportTargets(len(o.fbs), mods, false)
152+func (o *Output) exportImages(r ports.Renderer, mods []uint64, singlePlane bool) error {
153+ bufs, err := r.ExportTargets(len(o.fbs), mods, singlePlane)
145154 if err != nil {
146155 return err
147156 }
148157 for i := range bufs {
158+ if len(bufs[i].Planes) == 0 || len(bufs[i].Planes) > maxFBPlanes {
159+ err = errors.Join(err, fmt.Errorf("output image has %d planes", len(bufs[i].Planes)))
160+ }
149161 if err == nil {
150162 format := uint32(fourccXRGB)
151163 if o.hdr.on {
@@ -153,7 +165,11 @@ func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
153165 }
154166 o.fbs[i], err = o.k.addFB(&bufs[i], format)
155167 }
156- bufs[i].Planes[0].File.Close()
168+ // Every plane owns its file; the framebuffer keeps its own
169+ // reference to the buffer.
170+ for _, p := range bufs[i].Planes {
171+ p.File.Close()
172+ }
157173 }
158174 // GPU memory starts undefined (old VRAM contents): clear both images
159175 // before the modeset shows one.
@@ -178,7 +194,7 @@ func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
178194 _, _ = r.ExportTargets(0, nil, false)
179195 return err
180196 }
181- o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Msg("zero-copy output")
197+ o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Int("planes", len(bufs[0].Planes)).Msg("zero-copy output")
182198 return nil
183199 }
184200
@@ -17,10 +17,14 @@ import (
17 type imageKind int17 type imageKind int
18 18
19 const (19 const (
20- imagesDriver imageKind = iota // exported, modifier chosen by the driver20+ imagesDriver imageKind = iota // exported, modifier chosen by the driver (DCC planes allowed)
21- imagesLinear // exported, linear21+ imagesSinglePlane // exported, driver modifier with one memory plane
22+ imagesLinear // exported, linear
22 )23 )
23 24
25+// maxFBPlanes is the number of planes a KMS framebuffer holds (fbCmd2).
26+const maxFBPlanes = 4
27+
24 // showImages sets up images from kind on and modesets them. KMS may refuse28 // showImages sets up images from kind on and modesets them. KMS may refuse
25 // an image only in a modeset: a TEST_ONLY modeset checks it first, and the29 // an image only in a modeset: a TEST_ONLY modeset checks it first, and the
26 // next kind is tried on refusal. cause is why the30 // next kind is tried on refusal. cause is why the
@@ -131,7 +135,12 @@ func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (ima
131 } else if kind == imagesLinear {135 } else if kind == imagesLinear {
132 mods = []uint64{0}136 mods = []uint64{0}
133 }137 }
134- if err = o.exportImages(r, mods); err == nil {138+ if kind == imagesSinglePlane && len(mods) == 1 && mods[0] == 0 {
139+ // The display only lists linear, already one plane: the driver
140+ // attempt was the single-plane one.
141+ continue
142+ }
143+ if err = o.exportImages(r, mods, kind == imagesSinglePlane); err == nil {
135 return kind, nil144 return kind, nil
136 }145 }
137 o.log.Info().Err(err).Str("connector", o.conn.name).Int("kind", int(kind)).Msg("output image export")146 o.log.Info().Err(err).Str("connector", o.conn.name).Int("kind", int(kind)).Msg("output image export")
@@ -140,12 +149,15 @@ func (o *Output) setupImages(r ports.Renderer, kind imageKind, cause error) (ima
140 }149 }
141 150
142 // exportImages makes the renderer's exported targets the output images.151 // exportImages makes the renderer's exported targets the output images.
143-func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {152+func (o *Output) exportImages(r ports.Renderer, mods []uint64, singlePlane bool) error {
144- bufs, err := r.ExportTargets(len(o.fbs), mods, false)153+ bufs, err := r.ExportTargets(len(o.fbs), mods, singlePlane)
145 if err != nil {154 if err != nil {
146 return err155 return err
147 }156 }
148 for i := range bufs {157 for i := range bufs {
158+ if len(bufs[i].Planes) == 0 || len(bufs[i].Planes) > maxFBPlanes {
159+ err = errors.Join(err, fmt.Errorf("output image has %d planes", len(bufs[i].Planes)))
160+ }
149 if err == nil {161 if err == nil {
150 format := uint32(fourccXRGB)162 format := uint32(fourccXRGB)
151 if o.hdr.on {163 if o.hdr.on {
@@ -153,7 +165,11 @@ func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
153 }165 }
154 o.fbs[i], err = o.k.addFB(&bufs[i], format)166 o.fbs[i], err = o.k.addFB(&bufs[i], format)
155 }167 }
156- bufs[i].Planes[0].File.Close()168+ // Every plane owns its file; the framebuffer keeps its own
169+ // reference to the buffer.
170+ for _, p := range bufs[i].Planes {
171+ p.File.Close()
172+ }
157 }173 }
158 // GPU memory starts undefined (old VRAM contents): clear both images174 // GPU memory starts undefined (old VRAM contents): clear both images
159 // before the modeset shows one.175 // before the modeset shows one.
@@ -178,7 +194,7 @@ func (o *Output) exportImages(r ports.Renderer, mods []uint64) error {
178 _, _ = r.ExportTargets(0, nil, false)194 _, _ = r.ExportTargets(0, nil, false)
179 return err195 return err
180 }196 }
181- o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Msg("zero-copy output")197+ o.log.Info().Str("connector", o.conn.name).Uint64("modifier", bufs[0].Modifier).Int("planes", len(bufs[0].Planes)).Msg("zero-copy output")
182 return nil198 return nil
183 }199 }
184 200

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 23

Tags 10

↑↓ moveenter openesc close