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 <[email protected]> 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/[email protected]?part=3
