On 06/10, Markus Elfring wrote: > From: Markus Elfring <elfr...@users.sourceforge.net> > Date: Tue, 10 Jun 2025 07:42:40 +0200 > > The label “cleanup” was used to jump to another pointer check despite of > the detail in the implementation of the function > “dm_validate_stream_and_context” > that it was determined already that corresponding variables contained > still null pointers. > > 1. Thus return directly if > * a null pointer was passed for the function parameter “stream” > or > * a call of the function “dc_create_plane_state” failed. > > 2. Use a more appropriate label instead. > > 3. Delete two questionable checks. > > 4. Omit extra initialisations (for the variables “dc_state” and > “dc_plane_state”) > which became unnecessary with this refactoring. > > > This issue was detected by using the Coccinelle software. >
Hi Markus, Thanks for working on this improvement. Overall, LGTM. Some nits below. > Reported-by: kernel test robot <l...@intel.com> > Closes: > https://lore.kernel.org/oe-kbuild-all/202506100312.ms4xgazw-...@intel.com/ As the patch wasn't merged yet, don't add these two kernel-bot-related lines. You only need to add these lines "If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit)" > Fixes: 5468c36d6285 ("drm/amd/display: Filter Invalid 420 Modes for HDMI > TMDS") > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> > --- > > V3: > * Another function call was renamed. > > * Recipient lists were adjusted once more. > > V2: > * The change suggestion was rebased on source files of > the software “Linux next-20250606”. > > * Recipient lists were adjusted accordingly. > > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++----------- > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 78816712afbb..7dc80b2fbd30 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -7473,19 +7473,19 @@ static enum dc_status > dm_validate_stream_and_context(struct dc *dc, > struct dc_stream_state *stream) > { > enum dc_status dc_result = DC_ERROR_UNEXPECTED; > - struct dc_plane_state *dc_plane_state = NULL; > - struct dc_state *dc_state = NULL; > + struct dc_plane_state *dc_plane_state; > + struct dc_state *dc_state; > > if (!stream) > - goto cleanup; > + return dc_result; > > dc_plane_state = dc_create_plane_state(dc); > if (!dc_plane_state) > - goto cleanup; > + return dc_result; > > dc_state = dc_state_create(dc, NULL); > if (!dc_state) > - goto cleanup; > + goto release_plane_state; > > /* populate stream to plane */ > dc_plane_state->src_rect.height = stream->src.height; > @@ -7522,13 +7522,9 @@ static enum dc_status > dm_validate_stream_and_context(struct dc *dc, > if (dc_result == DC_OK) > dc_result = dc_validate_global_state(dc, dc_state, > DC_VALIDATE_MODE_ONLY); > > -cleanup: > - if (dc_state) > - dc_state_release(dc_state); > - > - if (dc_plane_state) > - dc_plane_state_release(dc_plane_state); > - > + dc_state_release(dc_state); For readability, I would add an extra line here... > +release_plane_state: > + dc_plane_state_release(dc_plane_state); and here. With that, you can add my Reviewed-by: Melissa Wen <m...@igalia.com> > return dc_result; > } > > -- > 2.49.0 >
