* [PATCH] drm/amd/display: Change dither policy for 10 bpc output back to dithering
@ 2026-03-21 5:20 Mario Kleiner
2026-03-21 17:13 ` Claude review: " Claude Code Review Bot
2026-03-21 17:13 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Mario Kleiner @ 2026-03-21 5:20 UTC (permalink / raw)
To: amd-gfx
Cc: dri-devel, mario.kleiner.de, stable, Aric Cyr, Anthony Koo,
Rodrigo Siqueira, Krunoslav Kovac, Alex Deucher
Commit d5df648ec830 ("drm/amd/display: Change dither policy for 10bpc to
round") degraded display of 12 bpc color precision output to 10 bpc sinks
by switching 10 bpc output from dithering to "truncate to 10 bpc".
I don't find the argumentation in that commit convincing, but the
consequences highly unfortunate, especially for applications that
require effective > 10 bpc precision output of > 10 bpc framebuffers.
The argument wasn't something strong like "there are hardware design
defects or limitations which require us to work around broken dithering
to 10 bpc", or "there are some special use cases which do require
truncation to 10 bpc", but essentially "at some point in the past we
used truncation in Polaris/Vega times and it looks like it got
inadvertently changed for Navi, so let's do that again". I couldn't find
evidence for that in the git commit logs for this. The commit message also
acknowledges that using dithering "...makes some sense for FP16...
...but not for ARGB2101010 surfaces..."
The problem with this is that it makes fp16 surfaces, and especially
rgba16 fixed point surfaces, less useful. These are now well
supported by Mesa 25.3 and later via OpenGL + EGL, Vulkan/WSI, and by
OSS AMDVLK Vulkan/WSI/display, and also by GNOME 50 mutter under Wayland,
and they used to provide more than 10 bpc effective precision at the
output.
Even for 8 or 10 bpc surfaces, the color pipeline behind the framebuffer,
e.g., gamma tables, CTM, can be used for color correction and will
benefit from an effective > 10 bpc output precision via dithering,
retaining some precision that would get lost on the way through the
pipeline, e.g., due to non-linear gamma functions.
Scientific apps rely on this for > 10 bpc display precision. Truncating
to 10 bpc, instead of dithering the pipeline internal 12 bpc precision
down to 10 bpc, causes a serious loss of precision. This also creates the
undesirable and slightly absurd situation that using a cheap monitor
with only 8 bpc input and display panel will yield roughly 12 bpc
precision via dithering from 12 -> 8 bpc, whereas investment into a
more expensive monitor with 10 bpc input and native 10 bpc display will
only yield 10 bpc, even if a fp16 or rgb16 framebuffer and/or a properly
set up color pipeline (gamma tables, CTM's etc. with more than 10 bpc out
precision) would allow effective 12 bpc precision output.
Therefore this patch proposes reverting that commit and going back to
dithering down to 10 bpc, consistent with the behaviour for 6 bpc or 8 bpc
output.
Successfully tested on AMD Polaris DCE 11.2 and Raven Ridge DCN 1.0 with
a native 10 bpc capable monitor, outputting a RGBA16 unorm framebuffer and
measuring resulting color precision with a photometer. No apparent visual
artifacts or problems were observed, and effective precision was measured
to be 12 bpc again, as expected.
Fixes: d5df648ec830 ("drm/amd/display: Change dither policy for 10bpc to round")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: stable@vger.kernel.org
Cc: Aric Cyr <aric.cyr@amd.com>
Cc: Anthony Koo <anthony.koo@amd.com>
Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
Cc: Krunoslav Kovac <krunoslav.kovac@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index c9fbb64d706a..29db5404c4a0 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -5056,7 +5056,7 @@ void resource_build_bit_depth_reduction_params(struct dc_stream_state *stream,
option = DITHER_OPTION_SPATIAL8;
break;
case COLOR_DEPTH_101010:
- option = DITHER_OPTION_TRUN10;
+ option = DITHER_OPTION_SPATIAL10;
break;
default:
option = DITHER_OPTION_DISABLE;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Claude review: drm/amd/display: Change dither policy for 10 bpc output back to dithering
2026-03-21 5:20 [PATCH] drm/amd/display: Change dither policy for 10 bpc output back to dithering Mario Kleiner
@ 2026-03-21 17:13 ` Claude Code Review Bot
2026-03-21 17:13 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:13 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/amd/display: Change dither policy for 10 bpc output back to dithering
Author: Mario Kleiner <mario.kleiner.de@gmail.com>
Patches: 1
Reviewed: 2026-03-22T03:13:38.863059
---
This is a single, well-motivated patch that reverts a prior policy change (commit d5df648ec830) which switched 10 bpc output from spatial dithering (`DITHER_OPTION_SPATIAL10`) to truncation (`DITHER_OPTION_TRUN10`). The patch is minimal (one-line change), well-tested by the author with actual hardware measurements, and makes the 10 bpc case consistent with 6 bpc and 8 bpc which both use spatial dithering.
**Verdict: The patch looks correct and should be accepted.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/amd/display: Change dither policy for 10 bpc output back to dithering
2026-03-21 5:20 [PATCH] drm/amd/display: Change dither policy for 10 bpc output back to dithering Mario Kleiner
2026-03-21 17:13 ` Claude review: " Claude Code Review Bot
@ 2026-03-21 17:13 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:13 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness:** The change at `dc_resource.c:5057-5058` from `DITHER_OPTION_TRUN10` to `DITHER_OPTION_SPATIAL10` is correct. Looking at the downstream handling:
- `DITHER_OPTION_SPATIAL10` is handled at lines 5125-5133, which sets `SPATIAL_DITHER_ENABLED = 1`, `SPATIAL_DITHER_DEPTH = 2`, along with `HIGHPASS_RANDOM` and conditional `RGB_RANDOM`. This is the proper spatial dithering path.
- The old `DITHER_OPTION_TRUN10` at lines 5076-5086 only set truncation with `TRUNCATE_MODE = 1` (rounding), losing the precision benefit of spatial dithering.
- The pattern is now consistent with `COLOR_DEPTH_666 → DITHER_OPTION_SPATIAL6` and `COLOR_DEPTH_888 → DITHER_OPTION_SPATIAL8`.
**Commit message quality:** Excellent. The author provides thorough technical justification for the revert, explains the real-world impact (scientific applications, fp16/rgba16 framebuffers, the 8 bpc vs 10 bpc absurdity), names specific software that benefits (Mesa 25.3, GNOME 50 mutter), and documents actual hardware testing (Polaris DCE 11.2 and Raven Ridge DCN 1.0 with photometer measurements).
**Minor observations:**
- The `Fixes:` tag and `Cc: stable` are appropriate since this is restoring a regression in precision for users of > 10 bpc pipelines.
- The only concern would be whether there exists some hardware where spatial dithering to 10 bpc causes visual artifacts — but the author explicitly tested on two different generations (Polaris and Raven Ridge) with no issues, and the original commit that introduced truncation did not cite any such hardware problems either.
**No issues found.** This is a clean, well-justified fix.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-21 17:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 5:20 [PATCH] drm/amd/display: Change dither policy for 10 bpc output back to dithering Mario Kleiner
2026-03-21 17:13 ` Claude review: " Claude Code Review Bot
2026-03-21 17:13 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox