On Fri, Apr 14, 2023 at 11:37:17AM -0300, Maíra Canal wrote: > On 4/14/23 11:24, Ville Syrjälä wrote: > > On Fri, Apr 14, 2023 at 10:51:50AM -0300, Maíra Canal wrote: > >> Currently, vkms only support the reflect-x property. Therefore, add the > >> reflect-y property to vkms through a software implementation of the > >> operation. This is possible by reverse reading the y axis during the > >> blending. > >> > >> Now, vkms support all possible rotation values. > >> > >> Tested with igt@kms_rotation_crc@primary-reflect-y and > >> igt@kms_rotation_crc@sprite-reflect-y [1]. > >> > >> [1] https://patchwork.freedesktop.org/series/116025/ > >> > >> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx> > >> --- > >> drivers/gpu/drm/vkms/vkms_composer.c | 7 ++++++- > >> drivers/gpu/drm/vkms/vkms_plane.c | 16 ++++------------ > >> 2 files changed, 10 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > >> index b05bd008aeab..19d1078e9d34 100644 > >> --- a/drivers/gpu/drm/vkms/vkms_composer.c > >> +++ b/drivers/gpu/drm/vkms/vkms_composer.c > >> @@ -92,8 +92,13 @@ static int get_y_pos(struct vkms_frame_info *frame_info, int y) > >> return -1; > >> return y + frame_info->dst.x1; > >> default: > >> - return y; > >> + break; > >> } > >> + > >> + if (frame_info->rotation & DRM_MODE_REFLECT_Y) > >> + return drm_rect_height(&frame_info->dst) - y - 1; > >> + > >> + return y; > >> } > >> > >> static bool check_limit(struct vkms_frame_info *frame_info, int pos) > >> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > >> index 11662afa9fe4..d08bda869a24 100644 > >> --- a/drivers/gpu/drm/vkms/vkms_plane.c > >> +++ b/drivers/gpu/drm/vkms/vkms_plane.c > >> @@ -121,12 +121,8 @@ static void vkms_plane_atomic_update(struct drm_plane *plane, > >> frame_info->fb = fb; > >> memcpy(&frame_info->map, &shadow_plane_state->data, sizeof(frame_info->map)); > >> drm_framebuffer_get(frame_info->fb); > >> - frame_info->rotation = drm_rotation_simplify(new_state->rotation, > >> - DRM_MODE_ROTATE_0 | > >> - DRM_MODE_ROTATE_90 | > >> - DRM_MODE_ROTATE_180 | > >> - DRM_MODE_ROTATE_270 | > >> - DRM_MODE_REFLECT_X); > >> + frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_MASK | > >> + DRM_MODE_REFLECT_MASK); > > > > What are you trying to achieve with that? > > Yeah, seeing it right now I can see that this is not achieving anything. > I will remove it in the next version. I think using it might still make sense to eg. remove the 180/270 cases from your actual rendering code. I'm also a bit uneasy about all that hand rolled coordinate calculation stuff. Ideally drm_rect_rotate*() should handle all that for you, and make sure the rotate vs. reflect actually get applied in the correct order. -- Ville Syrjälä Intel
