Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Skipping a required pipe split when no free pipes are available 
silently accepts an invalid display configuration and allows bandwidth 
validation to succeed.
--

commit 933da7b5e47097aeef7c30116ceee6ba3c949c0a
Author: Dmitriy Chumachenko <[email protected]>

drm/amd/display: guard NULL secondary pipe in dcn_validate_bandwidth()

This commit adds NULL checks around the return value of
resource_find_free_secondary_pipe_legacy() in dcn_validate_bandwidth() to
prevent NULL pointer dereferences during pipe splits.

> diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c 
> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> --- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c

[ ... ]

> @@ -1263,10 +1263,12 @@ bool dcn_validate_bandwidth(
>                                               /* pipe not split previously 
> needs split */
>                                               hsplit_pipe = 
> resource_find_free_secondary_pipe_legacy(&context->res_ctx, pool, pipe);
>                                               ASSERT(hsplit_pipe);
> -                                             
> split_stream_across_pipes(&context->res_ctx, pool, pipe, hsplit_pipe);
> +                                             if (hsplit_pipe)
> +                                                     
> split_stream_across_pipes(&context->res_ctx, pool, pipe, hsplit_pipe);
>                                       }
>  
> -                                     dcn_bw_calc_rq_dlg_ttu(dc, v, 
> hsplit_pipe, input_idx);
> +                                     if (hsplit_pipe)
> +                                             dcn_bw_calc_rq_dlg_ttu(dc, v, 
> hsplit_pipe, input_idx);

[Severity: High]
Does skipping this pipe split allow bandwidth validation to incorrectly
succeed?

When resource_find_free_secondary_pipe_legacy() returns NULL because all
secondary pipes are in use, this change silently skips the required pipe
split. Since dcn_validate_bandwidth() evaluates the bw_limit_pass condition
independently at the end of the function, it can still return true even
though a required hardware resource was missing.

Could this allow the display framework to proceed with an invalid
configuration, potentially leading to hardware misprogramming or display
underflow? Should dcn_validate_bandwidth() return false or propagate an
error when hsplit_pipe is NULL rather than continuing silently?

>                               } else if (hsplit_pipe && 
> hsplit_pipe->plane_state == pipe->plane_state) {
>                                       /* merge previously split pipe */
>                                       pipe->bottom_pipe = 
> hsplit_pipe->bottom_pipe;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to