On 2026-03-24 17:04, Harry Wentland 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. >
Forgot to add, for completeness: Assisted-by: Claude:claude-sonnet-4.6 Harry > 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;
