Re: [PATCH 3/3] drm/amd/display: size frame-packed streams by the doubled timing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch breaks the KUnit test dm_test_helper_check_state_scaling_caps in amdgpu_dm_plane_test.c by introducing a dependency on mode.hdisplay and mode.vdisplay, which the test does not mock.
--

commit fe46ce41733ff5de8af87a321672d77c5c614c30
Author: Adrian Betschart <adrian.betschart@cinemaone.ch>

drm/amd/display: size frame-packed streams by the doubled timing

The commit uses drm_mode_get_hv_timing() to handle frame-packed 3D modes
which scan out double the vertical timing, fixing view stretching across eye
windows.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 0a5a7347238ec..8db656ef85f84 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1407,16 +1407,21 @@ int amdgpu_dm_plane_helper_check_state(struct drm_plane_state *state,
>  		if (state->plane->type != DRM_PLANE_TYPE_CURSOR) {
>  			int viewport_width = state->crtc_w;
>  			int viewport_height = state->crtc_h;
> +			int mode_hdisplay, mode_vdisplay;
> +
> +			/* frame-packed 3D scans out the doubled timing */
> +			drm_mode_get_hv_timing(&new_crtc_state->mode,
> +					       &mode_hdisplay, &mode_vdisplay);

[Severity: Medium]
Does this change break the KUnit test dm_test_helper_check_state_scaling_caps
in amdgpu_dm_plane_test.c?

The test mocks new_crtc_state->mode.crtc_hdisplay and crtc_vdisplay, but
leaves hdisplay and vdisplay as 0:

dm_test_helper_check_state_scaling_caps() {
	...
	new_crtc_state->mode.crtc_hdisplay = 1920;
	new_crtc_state->mode.crtc_vdisplay = 1080;

	KUNIT_EXPECT_EQ(test,
			amdgpu_dm_plane_helper_check_state(state, new_crtc_state),
			-ERANGE);
	...
}

When amdgpu_dm_plane_helper_check_state() calls drm_mode_get_hv_timing(), it
reads the uninitialized 0 from hdisplay and overwrites the mocked
crtc_hdisplay internally.

Could this cause viewport_width to become 0, leading to an early -EINVAL exit
instead of the -ERANGE expected by the test assertion?

>  
>  			if (state->crtc_x < 0)
>  				viewport_width += state->crtc_x;
> -			else if (state->crtc_x + state->crtc_w > new_crtc_state->mode.crtc_hdisplay)
> -				viewport_width = new_crtc_state->mode.crtc_hdisplay - state->crtc_x;
> +			else if (state->crtc_x + state->crtc_w > mode_hdisplay)
> +				viewport_width = mode_hdisplay - state->crtc_x;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907100626.54465-1-adrian.betschart@cinemaone.ch?part=3