Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/amd/display: early return in dc_dmub_srv_cmd_run_list() drops diagnostics --
commit a641a7a2bd113ef9410e77f8cb3dd3d38d42ca71 Author: Alex Hung <[email protected]> drm/amd/display: Validate function returns This commit adds checks for function return values to fix Coverity issues. It handles errors from dmub_srv_wait_for_idle, dc_get_edp_link_panel_inst, hubbub1_program_pstate_watermarks, and link_detect_connection_type. > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > index 30a79dffbd37a..b3f02f451535a 100644 > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > @@ -144,7 +144,9 @@ bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv > *dc_dmub_srv, unsigned int coun > if (status == DMUB_STATUS_QUEUE_FULL) { > /* Execute and wait for queue to become empty again. */ > dmub_srv_cmd_execute(dmub); > - dmub_srv_wait_for_idle(dmub, 100000); > + status = dmub_srv_wait_for_idle(dmub, 100000); > + if (status != DMUB_STATUS_OK) > + return false; > > /* Requeue the command. */ > status = dmub_srv_cmd_queue(dmub, &cmd_list[i]); > } [Severity: Medium] Does this new early return inadvertently bypass the diagnostic data collection? Looking at dc_dmub_srv_cmd_run_list(), right after this block there is an error handler for when the queue or wait fails: if (status != DMUB_STATUS_OK) { DC_ERROR("Error queueing DMUB command: status=%d\n", status); dc_dmub_srv_log_diagnostic_data(dc_dmub_srv); return false; } By returning false directly when dmub_srv_wait_for_idle() fails, we skip dc_dmub_srv_log_diagnostic_data() and the DC_ERROR logging. Could we instead let it fall through or break here so that the existing error handler can collect this diagnostic data? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
