Merge pull request 'fix(drm): clear inherited CRTC colour state on modeset and close' (#38) from fix/clear-inherited-gamma into main
Reviewed-on: https://git.bnema.dev/bnema/neferwl/pulls/38
3 files · +56 -0
M docs/config.md +1 -0
| @@ -199,6 +199,7 @@ Hardware-decoded video can use NV12 (8-bit) or P010 (10-bit) DMA-BUF subsurfaces | |||
| 199 | 199 | - Fullscreen HDR buffers (10-bit, PQ) go straight to the display unchanged. | |
| 200 | 200 | - Opaque SDR (sRGB) buffers also skip composition on HDR outputs when the KMS plane has a colour pipeline (`DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`): sRGB curve, `sdr-brightness`, BT.709 to BT.2020, PQ, the same maths as composition. This applies to a fullscreen window (direct scanout) and to a lone window on an overlay plane. The compositor looks for that chain on each plane, whatever the driver; amdgpu on recent kernels has it. Without it, with alpha, with YUV or extended-linear content, SDR windows are composed, and the log gives the reason (`hdr_sdr_content`, `yuv`). When the driver refuses a pipeline (tested once per plane, buffer format and cursor state), the window is composed with reason `color_refused`. | |
| 201 | 201 | - Composed frames and every modeset leave the planes' pipeline on Bypass. | |
| 202 | + | - Every modeset and exit clear the CRTC's `DEGAMMA_LUT`, `CTM` and `GAMMA_LUT`, which this compositor never uses: a gamma ramp left by another DRM master would otherwise distort every frame. | |
| 202 | 203 | - A VT switch resets the pipeline first: before the seat is disabled, each output that applies a pipeline commits Bypass (at most 100 ms are spent waiting for it, then the switch goes on and a warning is logged). The pipeline survives a master change, and a client without `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE` cannot clear it. A crash or `kill -9` cannot do this reset: the next DRM master inherits the pipeline, with its colours distorted, until a client that sets the cap commits a modeset (or this compositor starts again). | |
| 203 | 204 | - Windowed and otherwise composed HDR content keeps its full range in a linear fp16 image before PQ output; the display tone-maps it according to its EDID metadata. SDR surfaces are blended in linear light on HDR outputs, so antialiased edges may look slightly different than sRGB-space blending. Dim veils (`floating.dim`, `stash.dim`) darken SDR white as much as on an SDR output; brighter HDR highlights stay a little brighter. | |
| 204 | 205 | - Screenshots and capture use opaque 8-bit sRGB. HDR capture applies GPU tone mapping and reduces out-of-sRGB gamut: values below 90% of SDR reference white stay unchanged, while brighter values are compressed to preserve highlight differences (SDR reference white maps to about 245/255 when capturing an HDR output, versus 255/255 on an SDR output). This capture-only transform does not change HDR display output. | |
| @@ -199,6 +199,7 @@ Hardware-decoded video can use NV12 (8-bit) or P010 (10-bit) DMA-BUF subsurfaces | |||||
| 199 | - Fullscreen HDR buffers (10-bit, PQ) go straight to the display unchanged. | 199 | - Fullscreen HDR buffers (10-bit, PQ) go straight to the display unchanged. | ||
| 200 | - Opaque SDR (sRGB) buffers also skip composition on HDR outputs when the KMS plane has a colour pipeline (`DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`): sRGB curve, `sdr-brightness`, BT.709 to BT.2020, PQ, the same maths as composition. This applies to a fullscreen window (direct scanout) and to a lone window on an overlay plane. The compositor looks for that chain on each plane, whatever the driver; amdgpu on recent kernels has it. Without it, with alpha, with YUV or extended-linear content, SDR windows are composed, and the log gives the reason (`hdr_sdr_content`, `yuv`). When the driver refuses a pipeline (tested once per plane, buffer format and cursor state), the window is composed with reason `color_refused`. | 200 | - Opaque SDR (sRGB) buffers also skip composition on HDR outputs when the KMS plane has a colour pipeline (`DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`): sRGB curve, `sdr-brightness`, BT.709 to BT.2020, PQ, the same maths as composition. This applies to a fullscreen window (direct scanout) and to a lone window on an overlay plane. The compositor looks for that chain on each plane, whatever the driver; amdgpu on recent kernels has it. Without it, with alpha, with YUV or extended-linear content, SDR windows are composed, and the log gives the reason (`hdr_sdr_content`, `yuv`). When the driver refuses a pipeline (tested once per plane, buffer format and cursor state), the window is composed with reason `color_refused`. | ||
| 201 | - Composed frames and every modeset leave the planes' pipeline on Bypass. | 201 | - Composed frames and every modeset leave the planes' pipeline on Bypass. | ||
| 202 | + | - Every modeset and exit clear the CRTC's `DEGAMMA_LUT`, `CTM` and `GAMMA_LUT`, which this compositor never uses: a gamma ramp left by another DRM master would otherwise distort every frame. | |||
| 202 | - A VT switch resets the pipeline first: before the seat is disabled, each output that applies a pipeline commits Bypass (at most 100 ms are spent waiting for it, then the switch goes on and a warning is logged). The pipeline survives a master change, and a client without `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE` cannot clear it. A crash or `kill -9` cannot do this reset: the next DRM master inherits the pipeline, with its colours distorted, until a client that sets the cap commits a modeset (or this compositor starts again). | 203 | - A VT switch resets the pipeline first: before the seat is disabled, each output that applies a pipeline commits Bypass (at most 100 ms are spent waiting for it, then the switch goes on and a warning is logged). The pipeline survives a master change, and a client without `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE` cannot clear it. A crash or `kill -9` cannot do this reset: the next DRM master inherits the pipeline, with its colours distorted, until a client that sets the cap commits a modeset (or this compositor starts again). | ||
| 203 | - Windowed and otherwise composed HDR content keeps its full range in a linear fp16 image before PQ output; the display tone-maps it according to its EDID metadata. SDR surfaces are blended in linear light on HDR outputs, so antialiased edges may look slightly different than sRGB-space blending. Dim veils (`floating.dim`, `stash.dim`) darken SDR white as much as on an SDR output; brighter HDR highlights stay a little brighter. | 204 | - Windowed and otherwise composed HDR content keeps its full range in a linear fp16 image before PQ output; the display tone-maps it according to its EDID metadata. SDR surfaces are blended in linear light on HDR outputs, so antialiased edges may look slightly different than sRGB-space blending. Dim veils (`floating.dim`, `stash.dim`) darken SDR white as much as on an SDR output; brighter HDR highlights stay a little brighter. | ||
| 204 | - Screenshots and capture use opaque 8-bit sRGB. HDR capture applies GPU tone mapping and reduces out-of-sRGB gamut: values below 90% of SDR reference white stay unchanged, while brighter values are compressed to preserve highlight differences (SDR reference white maps to about 245/255 when capturing an HDR output, versus 255/255 on an SDR output). This capture-only transform does not change HDR display output. | 205 | - Screenshots and capture use opaque 8-bit sRGB. HDR capture applies GPU tone mapping and reduces out-of-sRGB gamut: values below 90% of SDR reference white stay unchanged, while brighter values are compressed to preserve highlight differences (SDR reference white maps to about 245/255 when capturing an HDR output, versus 255/255 on an SDR output). This capture-only transform does not change HDR display output. | ||
M internal/adapters/drm/colorop.go +6 -0
| @@ -90,9 +90,15 @@ func (o *Output) primaryColorProps(req *atomicReq, mode colorMode) { | |||
| 90 | 90 | // forceBypass sets Bypass on the primary and overlay planes whether or not | |
| 91 | 91 | // the applied state is known to differ: for requests that must leave a | |
| 92 | 92 | // known state (modeset, close). | |
| 93 | + | // The CRTC's DEGAMMA_LUT, CTM and GAMMA_LUT are cleared with them: this | |
| 94 | + | // compositor never sets them, and a ramp left by another DRM master would | |
| 95 | + | // apply on top of the PQ output. | |
| 93 | 96 | func (o *Output) forceBypass(req *atomicReq) { | |
| 94 | 97 | o.primary.colorProps(req, colorBypass, 0, 0) | |
| 95 | 98 | o.overlay.colorProps(req, colorBypass, 0, 0) | |
| 99 | + | for _, name := range [...]string{"DEGAMMA_LUT", "CTM", "GAMMA_LUT"} { | |
| 100 | + | req.set(o.crtc, o.crtcProps[name], 0) | |
| 101 | + | } | |
| 96 | 102 | } | |
| 97 | 103 | ||
| 98 | 104 | // overlayColorProps puts mode on the overlay plane if it applies another one. | |
| @@ -90,9 +90,15 @@ func (o *Output) primaryColorProps(req *atomicReq, mode colorMode) { | |||||
| 90 | // forceBypass sets Bypass on the primary and overlay planes whether or not | 90 | // forceBypass sets Bypass on the primary and overlay planes whether or not | ||
| 91 | // the applied state is known to differ: for requests that must leave a | 91 | // the applied state is known to differ: for requests that must leave a | ||
| 92 | // known state (modeset, close). | 92 | // known state (modeset, close). | ||
| 93 | + | // The CRTC's DEGAMMA_LUT, CTM and GAMMA_LUT are cleared with them: this | |||
| 94 | + | // compositor never sets them, and a ramp left by another DRM master would | |||
| 95 | + | // apply on top of the PQ output. | |||
| 93 | func (o *Output) forceBypass(req *atomicReq) { | 96 | func (o *Output) forceBypass(req *atomicReq) { | ||
| 94 | o.primary.colorProps(req, colorBypass, 0, 0) | 97 | o.primary.colorProps(req, colorBypass, 0, 0) | ||
| 95 | o.overlay.colorProps(req, colorBypass, 0, 0) | 98 | o.overlay.colorProps(req, colorBypass, 0, 0) | ||
| 99 | + | for _, name := range [...]string{"DEGAMMA_LUT", "CTM", "GAMMA_LUT"} { | |||
| 100 | + | req.set(o.crtc, o.crtcProps[name], 0) | |||
| 101 | + | } | |||
| 96 | } | 102 | } | ||
| 97 | 103 | ||||
| 98 | // overlayColorProps puts mode on the overlay plane if it applies another one. | 104 | // overlayColorProps puts mode on the overlay plane if it applies another one. | ||
M internal/adapters/drm/colorop_test.go +49 -0
| @@ -2,6 +2,7 @@ package drm | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/binary" | |
| 5 | + | "maps" | |
| 5 | 6 | "math" | |
| 6 | 7 | "os" | |
| 7 | 8 | "regexp" | |
| @@ -526,6 +527,54 @@ func TestColorBypassOnComposedProtectedOffCloseAndModeset(t *testing.T) { | |||
| 526 | 527 | }) | |
| 527 | 528 | } | |
| 528 | 529 | ||
| 530 | + | // A modeset and Close clear the CRTC's DEGAMMA_LUT, CTM and GAMMA_LUT, | |
| 531 | + | // which another DRM master may have left: a ramp kept there distorts every | |
| 532 | + | // frame, the PQ ones included. | |
| 533 | + | func TestCRTCColorClearedOnModesetAndClose(t *testing.T) { | |
| 534 | + | const pCTM, pGamma = 63, 64 | |
| 535 | + | cleared := func(t *testing.T, c commitRec) { | |
| 536 | + | t.Helper() | |
| 537 | + | for _, p := range []uint32{pDegamma, pCTM, pGamma} { | |
| 538 | + | if v, ok := c.req.value(tCrtc, p); !ok || v != 0 { | |
| 539 | + | t.Errorf("CRTC property %d = %d %v, want 0", p, v, ok) | |
| 540 | + | } | |
| 541 | + | } | |
| 542 | + | } | |
| 543 | + | setup := func(t *testing.T) (*Output, *mockkms, *[]commitRec) { | |
| 544 | + | o, k, commits := colorOutput(t, false, nil) | |
| 545 | + | o.crtcProps = maps.Clone(o.crtcProps) | |
| 546 | + | o.crtcProps["DEGAMMA_LUT"], o.crtcProps["CTM"], o.crtcProps["GAMMA_LUT"] = pDegamma, pCTM, pGamma | |
| 547 | + | return o, k, commits | |
| 548 | + | } | |
| 549 | + | t.Run("modeset", func(t *testing.T) { | |
| 550 | + | o, k, commits := setup(t) | |
| 551 | + | k.EXPECT().createBlob(mock.Anything).Return(99, nil).Once() | |
| 552 | + | if err := o.modeset(); err != nil { | |
| 553 | + | t.Fatal(err) | |
| 554 | + | } | |
| 555 | + | cleared(t, (*commits)[0]) | |
| 556 | + | }) | |
| 557 | + | t.Run("close", func(t *testing.T) { | |
| 558 | + | o, k, commits := setup(t) | |
| 559 | + | k.EXPECT().rmFB(mock.Anything).Return(nil) | |
| 560 | + | k.EXPECT().destroyBlob(uint32(61)).Return(nil).Once() | |
| 561 | + | o.Close() | |
| 562 | + | cleared(t, (*commits)[0]) | |
| 563 | + | }) | |
| 564 | + | t.Run("frame", func(t *testing.T) { | |
| 565 | + | // A frame commit leaves them alone: they stay as the modeset left them. | |
| 566 | + | o, _, commits := setup(t) | |
| 567 | + | if ok, err := o.commitScanoutRect(80, sdrContent(o), pendingFrame{}, fullPlaneRect(200, 100), colorSDRToPQ); !ok || err != nil { | |
| 568 | + | t.Fatalf("ok=%v err=%v", ok, err) | |
| 569 | + | } | |
| 570 | + | for _, p := range []uint32{pDegamma, pCTM, pGamma} { | |
| 571 | + | if _, ok := (*commits)[0].req.value(tCrtc, p); ok { | |
| 572 | + | t.Errorf("frame commit writes CRTC property %d", p) | |
| 573 | + | } | |
| 574 | + | } | |
| 575 | + | }) | |
| 576 | + | } | |
| 577 | + | ||
| 529 | 578 | // The overlay's pipeline is driven with the primary on Bypass; a frame | |
| 530 | 579 | // without the overlay turns it back to Bypass once. | |
| 531 | 580 | func TestColorOverlayTransitions(t *testing.T) { | |
| @@ -2,6 +2,7 @@ package drm | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/binary" | 4 | "encoding/binary" | ||
| 5 | + | "maps" | |||
| 5 | "math" | 6 | "math" | ||
| 6 | "os" | 7 | "os" | ||
| 7 | "regexp" | 8 | "regexp" | ||
| @@ -526,6 +527,54 @@ func TestColorBypassOnComposedProtectedOffCloseAndModeset(t *testing.T) { | |||||
| 526 | }) | 527 | }) | ||
| 527 | } | 528 | } | ||
| 528 | 529 | ||||
| 530 | + | // A modeset and Close clear the CRTC's DEGAMMA_LUT, CTM and GAMMA_LUT, | |||
| 531 | + | // which another DRM master may have left: a ramp kept there distorts every | |||
| 532 | + | // frame, the PQ ones included. | |||
| 533 | + | func TestCRTCColorClearedOnModesetAndClose(t *testing.T) { | |||
| 534 | + | const pCTM, pGamma = 63, 64 | |||
| 535 | + | cleared := func(t *testing.T, c commitRec) { | |||
| 536 | + | t.Helper() | |||
| 537 | + | for _, p := range []uint32{pDegamma, pCTM, pGamma} { | |||
| 538 | + | if v, ok := c.req.value(tCrtc, p); !ok || v != 0 { | |||
| 539 | + | t.Errorf("CRTC property %d = %d %v, want 0", p, v, ok) | |||
| 540 | + | } | |||
| 541 | + | } | |||
| 542 | + | } | |||
| 543 | + | setup := func(t *testing.T) (*Output, *mockkms, *[]commitRec) { | |||
| 544 | + | o, k, commits := colorOutput(t, false, nil) | |||
| 545 | + | o.crtcProps = maps.Clone(o.crtcProps) | |||
| 546 | + | o.crtcProps["DEGAMMA_LUT"], o.crtcProps["CTM"], o.crtcProps["GAMMA_LUT"] = pDegamma, pCTM, pGamma | |||
| 547 | + | return o, k, commits | |||
| 548 | + | } | |||
| 549 | + | t.Run("modeset", func(t *testing.T) { | |||
| 550 | + | o, k, commits := setup(t) | |||
| 551 | + | k.EXPECT().createBlob(mock.Anything).Return(99, nil).Once() | |||
| 552 | + | if err := o.modeset(); err != nil { | |||
| 553 | + | t.Fatal(err) | |||
| 554 | + | } | |||
| 555 | + | cleared(t, (*commits)[0]) | |||
| 556 | + | }) | |||
| 557 | + | t.Run("close", func(t *testing.T) { | |||
| 558 | + | o, k, commits := setup(t) | |||
| 559 | + | k.EXPECT().rmFB(mock.Anything).Return(nil) | |||
| 560 | + | k.EXPECT().destroyBlob(uint32(61)).Return(nil).Once() | |||
| 561 | + | o.Close() | |||
| 562 | + | cleared(t, (*commits)[0]) | |||
| 563 | + | }) | |||
| 564 | + | t.Run("frame", func(t *testing.T) { | |||
| 565 | + | // A frame commit leaves them alone: they stay as the modeset left them. | |||
| 566 | + | o, _, commits := setup(t) | |||
| 567 | + | if ok, err := o.commitScanoutRect(80, sdrContent(o), pendingFrame{}, fullPlaneRect(200, 100), colorSDRToPQ); !ok || err != nil { | |||
| 568 | + | t.Fatalf("ok=%v err=%v", ok, err) | |||
| 569 | + | } | |||
| 570 | + | for _, p := range []uint32{pDegamma, pCTM, pGamma} { | |||
| 571 | + | if _, ok := (*commits)[0].req.value(tCrtc, p); ok { | |||
| 572 | + | t.Errorf("frame commit writes CRTC property %d", p) | |||
| 573 | + | } | |||
| 574 | + | } | |||
| 575 | + | }) | |||
| 576 | + | } | |||
| 577 | + | ||||
| 529 | // The overlay's pipeline is driven with the primary on Bypass; a frame | 578 | // The overlay's pipeline is driven with the primary on Bypass; a frame | ||
| 530 | // without the overlay turns it back to Bypass once. | 579 | // without the overlay turns it back to Bypass once. | ||
| 531 | func TestColorOverlayTransitions(t *testing.T) { | 580 | func TestColorOverlayTransitions(t *testing.T) { | ||