Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Type confusion between array of pointers and array of structs causes incorrect pipe updates for multi-pipe links. -- commit 2b93ced29eb3c89bb083129e3d48fc9f319bc125 Author: Ilya Bakoulin <[email protected]> Subject: drm/amd/display: Avoid DPMS-on for phantom stream This commit fixes a NULL pointer dereference in the DP link training code path. It skips phantom streams during stream caching for updates, as phantom streams are destroyed during the first update call. > 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 2676ae9f6fe83..5f70793b8372e 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 > @@ -143,6 +143,8 @@ static void dp_retrain_link_dp_test(struct dc_link *link, > // Set DPMS on with stream update > // Cache all streams on current link since dc_update_planes_and_stream > might kill current_state > for (i = 0; i < MAX_PIPES; i++) { > + if (state->streams[i] && state->streams[i]->is_phantom) > + continue; > if (state->streams[i] && state->streams[i]->link && > state->streams[i]->link == link) > streams_on_link[num_streams_on_link++] = > state->streams[i]; > } [Severity: High] This isn't a bug introduced by this patch, but looking earlier in dp_retrain_link_dp_test(), does this code pass an array of pointers to a function expecting a contiguous array of structs? dp_retrain_link_dp_test() populates pipes as an array of pointers to active master pipes: drivers/gpu/drm/amd/display/dc/link/accessories/link_dp_cts.c:dp_retrain_link_dp_test() { ... struct pipe_ctx *pipes[MAX_PIPES]; ... link_get_master_pipes_with_dpms_on(link, state, &count, pipes); ... if (needs_divider_update && link->dc->res_pool->funcs->update_dc_state_for_encoder_switch) { link->dc->res_pool->funcs->update_dc_state_for_encoder_switch(link, link_setting, count, *pipes, &audio_output[0]); ... } It passes *pipes (the first pointer). However, the callee expects a pointer to an array of contiguous struct pipe_ctx objects: drivers/gpu/drm/amd/display/dc/resource/dcn31/dcn31_resource.c:dcn31_update_dc_state_for_encoder_switch() { ... for (i = 0; i < pipe_count; i++) { link->dc->res_pool->funcs->build_pipe_pix_clk_params(&pipes[i]); ... } Because pipes are collected from active master pipes, they may not be contiguous in the state->res_ctx.pipe_ctx array for multi-pipe links (e.g. MST). Using pointer arithmetic (&pipes[i]) here will incorrectly modify adjacent memory that might belong to unrelated active displays, while failing to update the actual target pipes. Can this lead to state corruption and display malfunction during display hotplug, mode set, or automated DP compliance testing? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
