Hi Harry,
Thanks for taking a stab at this!
I agree that your patch would be a good solution. And my review
suggested the code looked correct, but unfortunately in my testing on
my laptop with Polaris11 for the 8 bpc input fb + 8 bpc output on the
eDP panel of my laptop, the machine disagreed. It always gives me
dithering for my XRGB8888 aka 8 bpc input fb to my 8 bpc
COLOR_DEPTH_888 output. I'm visiting my parents atm., and therefore
don't have access to the 10 bpc DP monitor for output 10 bpc testing,
but at least the 8 bpc laptop panel is no success.
Reason is that max_plane_bpp = get_max_input_bpc_for_stream(dc,
stream); is reported as 0, so it hits the if (max_plane_bpp == 0 ||
...) condition and triggers DITHER_OPTION_SPATIAL8 when it should
choose the else branch with DITHER_OPTION_TRUN8.
Reason for get_max_input_bpc_for_stream() reporting 0 is that for the
only active pipe (with index i == 0) in
dc->current_state->res_ctx.pipe_ctx[i], the pipe->stream != stream
condition is true, so the whole maximum finding logic gets skipped.
Added debug statements showed that pipe->stream from res_ctx never
matches the stream pointer passed into
resource_build_bit_depth_reduction_params(). So something is amiss
there.
dc->current_state->res_ctx.pipe_ctx[0]->stream matches what the passed
in stream was during early amdgpu driver init during boot, but during
later bootup, login and session, the passed in stream pointer changes
multiple times but never matches pipe->stream.
Apart from that, style-wise it would probably make sense to rename
max_plane_bpp into max_plane_bpc for consistency?
And ideally we'd add...
Fixes: d5df648ec830 ("drm/amd/display: Change dither policy for 10bpc to round")
Cc: [email protected]
I hope getting this into drm-fixes for upcoming Linux 7.0 would still
be possible, as that will be the standard distribution kernel for
upcoming Ubuntu 26.04-LTS, Fedora 44 and other spring 2026
distribution updates, which will also ship with enhanced Mesa 26.0 and
GNOME mutter 50, both improved to make good use of 16 bpc framebuffers
on AMD hw.
Thanks
-mario
On Tue, Mar 24, 2026 at 10:06 PM Harry Wentland <[email protected]> wrote:
>
> We don't want to dither when a 10bpc buffer is output on a 10bpc
> connection as we'd get off-by-one errors. But we do want to dither
> if we have planes with a higher bit depth.
>
> In order to solve this, look at all planes, and pick TRUN dither
> when input bit depth doesn't exceed output bit depth, otherwise
> pick SPATIAL.
>
> Cc: Kovac, Krunoslav <[email protected]>
> Cc: Hung, Alex <[email protected]>
> Cc: Deucher, Alexander <[email protected]>
> Reported-by: Mario Kleiner <[email protected]>
> Signed-off-by: Harry Wentland <[email protected]>
> ---
>
> Mario, Kruno,
>
> this patch looks at planes and picks dither based on the max
> bit depth of all planes on the stream. Would this work for
> both of you?
>
> I've only made sure my Rembrandt system boots with this but
> haven't been able to confirm that the correct dither mode is
> selected.
>
> Harry
>
> drivers/gpu/drm/amd/display/dc/core/dc.c | 5 +-
> .../drm/amd/display/dc/core/dc_hw_sequencer.c | 3 +-
> .../gpu/drm/amd/display/dc/core/dc_resource.c | 110 +++++++++++++++++-
> drivers/gpu/drm/amd/display/dc/inc/resource.h | 3 +-
> .../display/dc/link/accessories/link_dp_cts.c | 3 +-
> .../dc/resource/dce110/dce110_resource.c | 3 +-
> .../dc/resource/dcn10/dcn10_resource.c | 3 +-
> .../dc/resource/dcn20/dcn20_resource.c | 3 +-
> 8 files changed, 122 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 8b21816cf7c8..56d6f9d2fcca 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -788,7 +788,7 @@ void dc_stream_set_dither_option(struct dc_stream_state
> *stream,
> stream->dither_option = option;
>
> memset(¶ms, 0, sizeof(params));
> - resource_build_bit_depth_reduction_params(stream, ¶ms);
> + resource_build_bit_depth_reduction_params(stream, ¶ms,
> stream->ctx->dc);
> stream->bit_depth_params = params;
>
> if (pipes->plane_res.xfm &&
> @@ -3841,7 +3841,8 @@ static void commit_planes_do_stream_update(struct dc
> *dc,
> if (stream_update->dither_option) {
> struct pipe_ctx *odm_pipe =
> pipe_ctx->next_odm_pipe;
>
> resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> -
> &pipe_ctx->stream->bit_depth_params);
> +
> &pipe_ctx->stream->bit_depth_params,
> +
> pipe_ctx->stream->ctx->dc);
>
> pipe_ctx->stream_res.opp->funcs->opp_program_fmt(pipe_ctx->stream_res.opp,
> &stream->bit_depth_params,
> &stream->clamping);
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> index 5b3695e72e19..a042b31b57ba 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> @@ -2485,7 +2485,8 @@ void hwss_opp_program_bit_depth_reduction(union
> block_sequence_params *params)
> if (use_default_params)
> memset(&bit_depth_params, 0, sizeof(bit_depth_params));
> else
> - resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> &bit_depth_params);
> + resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> + &bit_depth_params, pipe_ctx->stream->ctx->dc);
>
> if (opp->funcs->opp_program_bit_depth_reduction)
> opp->funcs->opp_program_bit_depth_reduction(opp,
> &bit_depth_params);
> 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 8271b12c1a66..10b7e14ef66f 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -5037,25 +5037,129 @@ bool pipe_need_reprogram(
> return false;
> }
>
> +/**
> + * get_bit_depth_from_surface_pixel_format - Get effective bit depth from
> surface format
> + * @format: Surface pixel format
> + *
> + * Returns the effective bit depth per channel for the given surface format.
> + * This is used to determine if input precision is higher than output
> precision
> + * for dithering decisions.
> + *
> + * Return: Bits per channel (6, 8, 10, 12, or 16)
> + */
> +static unsigned int get_bit_depth_from_surface_pixel_format(enum
> surface_pixel_format format)
> +{
> + switch (format) {
> + case SURFACE_PIXEL_FORMAT_GRPH_PALETA_256_COLORS:
> + return 8;
> + case SURFACE_PIXEL_FORMAT_GRPH_ARGB1555:
> + return 5; /* 5 bits per channel */
> + case SURFACE_PIXEL_FORMAT_GRPH_RGB565:
> + return 6;
> + case SURFACE_PIXEL_FORMAT_GRPH_ARGB8888:
> + case SURFACE_PIXEL_FORMAT_GRPH_ABGR8888:
> + case SURFACE_PIXEL_FORMAT_VIDEO_420_YCbCr:
> + case SURFACE_PIXEL_FORMAT_VIDEO_420_YCrCb:
> + return 8;
> + case SURFACE_PIXEL_FORMAT_GRPH_ARGB2101010:
> + case SURFACE_PIXEL_FORMAT_GRPH_ABGR2101010:
> + case SURFACE_PIXEL_FORMAT_GRPH_ABGR2101010_XR_BIAS:
> + case SURFACE_PIXEL_FORMAT_VIDEO_420_10bpc_YCbCr:
> + case SURFACE_PIXEL_FORMAT_VIDEO_420_10bpc_YCrCb:
> + return 10;
> + case SURFACE_PIXEL_FORMAT_GRPH_ARGB16161616:
> + case SURFACE_PIXEL_FORMAT_GRPH_ABGR16161616:
> + return 16; /* 16-bit fixed point */
> + case SURFACE_PIXEL_FORMAT_GRPH_ARGB16161616F:
> + case SURFACE_PIXEL_FORMAT_GRPH_ABGR16161616F:
> + return 16; /* FP16 has higher effective precision */
> + case SURFACE_PIXEL_FORMAT_GRPH_RGBE:
> + case SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA:
> + return 8; /* 8-bit mantissa per channel */
> + default:
> + return 8;
> + }
> +}
> +
> +/**
> + * get_max_input_bpc_for_stream - Get maximum input plane bit depth for a
> stream
> + * @dc: DC context
> + * @stream: The stream to check
> + *
> + * Iterate through all pipes in the current DC state to find planes attached
> + * to this stream, and return the maximum bit depth across all those planes.
> + * This is used to determine if spatial dithering (for higher precision
> inputs
> + * like FP16/RGBA16) or rounding (for bit-accurate matching precision like
> + * RGB10->10bpc) should be used.
> + *
> + * Returns: Maximum bits per channel across all planes for this stream,
> + * or 0 if no planes found or no current state.
> + */
> +static unsigned int get_max_input_bpc_for_stream(const struct dc *dc,
> + struct dc_stream_state
> *stream)
> +{
> + unsigned int max_bpc = 0;
> + int i;
> +
> + if (!dc || !dc->current_state || !stream)
> + return 0;
> +
> + /* Iterate through all pipes to find planes for this stream */
> + for (i = 0; i < MAX_PIPES; i++) {
> + struct pipe_ctx *pipe =
> &dc->current_state->res_ctx.pipe_ctx[i];
> + unsigned int plane_bpc;
> +
> + if (!pipe->plane_state || pipe->stream != stream)
> + continue;
> +
> + plane_bpc = get_bit_depth_from_surface_pixel_format(
> + pipe->plane_state->format);
> +
> + if (plane_bpc > max_bpc)
> + max_bpc = plane_bpc;
> + }
> +
> + return max_bpc;
> +}
> +
> void resource_build_bit_depth_reduction_params(struct dc_stream_state
> *stream,
> - struct bit_depth_reduction_params *fmt_bit_depth)
> + struct bit_depth_reduction_params *fmt_bit_depth,
> + const struct dc *dc)
> {
> enum dc_dither_option option = stream->dither_option;
> enum dc_pixel_encoding pixel_encoding =
> stream->timing.pixel_encoding;
> + unsigned int max_plane_bpp;
>
> memset(fmt_bit_depth, 0, sizeof(*fmt_bit_depth));
>
> + /* Get max input bpc from planes attached to this stream */
> + max_plane_bpp = get_max_input_bpc_for_stream(dc, stream);
> +
> if (option == DITHER_OPTION_DEFAULT) {
> switch (stream->timing.display_color_depth) {
> case COLOR_DEPTH_666:
> option = DITHER_OPTION_SPATIAL6;
> break;
> case COLOR_DEPTH_888:
> - option = DITHER_OPTION_SPATIAL8;
> + /* Use spatial dithering if we don't know plane bpp
> (0) or
> + * if plane precision > output precision, otherwise
> use
> + * rounding/truncation for bit accuracy.
> + */
> + if (max_plane_bpp == 0 || max_plane_bpp > 8)
> + option = DITHER_OPTION_SPATIAL8;
> + else
> + option = DITHER_OPTION_TRUN8;
> break;
> case COLOR_DEPTH_101010:
> - option = DITHER_OPTION_TRUN10;
> + /* Use spatial dithering if we don't know plane bpp
> (0) or
> + * if plane precision > output precision, otherwise
> use
> + * rounding for bit accuracy.
> + */
> + if (max_plane_bpp == 0 || max_plane_bpp > 10)
> + option = DITHER_OPTION_SPATIAL10;
> + else
> + option = DITHER_OPTION_TRUN10;
> break;
> default:
> option = DITHER_OPTION_DISABLE;
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> index cecd3282a29f..2d040e735521 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> @@ -591,7 +591,8 @@ bool pipe_need_reprogram(
> struct pipe_ctx *pipe_ctx);
>
> void resource_build_bit_depth_reduction_params(struct dc_stream_state
> *stream,
> - struct bit_depth_reduction_params *fmt_bit_depth);
> + struct bit_depth_reduction_params *fmt_bit_depth,
> + const struct dc *dc);
>
> void update_audio_usage(
> struct resource_context *res_ctx,
> diff --git a/drivers/gpu/drm/amd/display/dc/link/accessories/link_dp_cts.c
> b/drivers/gpu/drm/amd/display/dc/link/accessories/link_dp_cts.c
> index 693d852b1c40..377e02095867 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/accessories/link_dp_cts.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/accessories/link_dp_cts.c
> @@ -543,7 +543,8 @@ static void set_crtc_test_pattern(struct dc_link *link,
> case DP_TEST_PATTERN_VIDEO_MODE:
> {
> /* restore bitdepth reduction */
> - resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> ¶ms);
> + resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> ¶ms,
> + pipe_ctx->stream->ctx->dc);
> pipe_ctx->stream->bit_depth_params = params;
> if (pipe_ctx->stream_res.tg->funcs->set_test_pattern) {
> opp->funcs->opp_program_bit_depth_reduction(opp,
> ¶ms);
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
> index 7c09825cd9bd..6433f48e9158 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
> @@ -928,7 +928,8 @@ void dce110_resource_build_pipe_hw_param(struct pipe_ctx
> *pipe_ctx)
> &pipe_ctx->stream_res.pix_clk_params,
> &pipe_ctx->pll_settings);
> resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> - &pipe_ctx->stream->bit_depth_params);
> + &pipe_ctx->stream->bit_depth_params,
> + pipe_ctx->stream->ctx->dc);
> pipe_ctx->stream->clamping.pixel_encoding =
> pipe_ctx->stream->timing.pixel_encoding;
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn10/dcn10_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dcn10/dcn10_resource.c
> index 9c1a57a1f989..6cd16b64baae 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn10/dcn10_resource.c
> @@ -1042,7 +1042,8 @@ static void build_pipe_hw_param(struct pipe_ctx
> *pipe_ctx)
> pipe_ctx->stream->clamping.pixel_encoding =
> pipe_ctx->stream->timing.pixel_encoding;
>
> resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> - &pipe_ctx->stream->bit_depth_params);
> + &pipe_ctx->stream->bit_depth_params,
> + pipe_ctx->stream->ctx->dc);
> build_clamping_params(pipe_ctx->stream);
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dcn20/dcn20_resource.c
> index b28e877fb99d..f2786bfc87c1 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn20/dcn20_resource.c
> @@ -1302,7 +1302,8 @@ static enum dc_status build_pipe_hw_param(struct
> pipe_ctx *pipe_ctx)
> pipe_ctx->stream->clamping.pixel_encoding =
> pipe_ctx->stream->timing.pixel_encoding;
>
> resource_build_bit_depth_reduction_params(pipe_ctx->stream,
> - &pipe_ctx->stream->bit_depth_params);
> + &pipe_ctx->stream->bit_depth_params,
> + pipe_ctx->stream->ctx->dc);
> build_clamping_params(pipe_ctx->stream);
>
> return DC_OK;
> --
> 2.53.0
>
