At the moment, video mode parameters like video=540x960,reflect_x, (without rotation set) are not taken into account when applying the mode in drm_client_modeset.c. One of the reasons for this is that the calculation that combines the panel_orientation with cmdline->rotation_reflection does not handle the case when cmdline->rotation_reflection does not have any rotation set. (i.e. cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK == 0) Example: *rotation = DRM_MODE_ROTATE_0 (no panel_orientation) cmdline->rotation_reflection = DRM_MODE_REFLECT_X (video=MODE,reflect_x) The current code does: panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK); cmdline_rot = ilog2(cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK); sum_rot = (panel_rot + cmdline_rot) % 4; and therefore: panel_rot = ilog2(DRM_MODE_ROTATE_0) = ilog2(1) = 0 cmdline_rot = ilog2(0) = -1 sum_rot = (0 + -1) % 4 = -1 % 4 = 3 ... *rotation = DRM_MODE_ROTATE_270 | DRM_MODE_REFLECT_X So we incorrectly generate DRM_MODE_ROTATE_270 in this case. To prevent cmdline_rot from becoming -1, we need to treat the rotation as DRM_MODE_ROTATE_0. On the other hand, there is no need to go through that calculation at all if no rotation is set in rotation_reflection. A simple XOR is enough to combine the reflections. Finally, also allow DRM_MODE_ROTATE_0 in the if statement below. DRM_MODE_ROTATE_0 means "no rotation" and should therefore not require any special handling (e.g. specific tiling format). This makes video parameters with only reflect option work correctly. Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> --- v1: https://lists.freedesktop.org/archives/dri-devel/2019-December/248145.html Changes in v2: - Clarified commit message - parameters are parsed correctly, but not taken into account when applying the mode. --- drivers/gpu/drm/drm_client_modeset.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 895b73f23079..cfebce4f19a5 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -859,19 +859,23 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation) */ cmdline = &connector->cmdline_mode; if (cmdline->specified && cmdline->rotation_reflection) { - unsigned int cmdline_rest, panel_rest; - unsigned int cmdline_rot, panel_rot; - unsigned int sum_rot, sum_rest; + if (cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK) { + unsigned int cmdline_rest, panel_rest; + unsigned int cmdline_rot, panel_rot; + unsigned int sum_rot, sum_rest; - panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK); - cmdline_rot = ilog2(cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK); - sum_rot = (panel_rot + cmdline_rot) % 4; + panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK); + cmdline_rot = ilog2(cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK); + sum_rot = (panel_rot + cmdline_rot) % 4; - panel_rest = *rotation & ~DRM_MODE_ROTATE_MASK; - cmdline_rest = cmdline->rotation_reflection & ~DRM_MODE_ROTATE_MASK; - sum_rest = panel_rest ^ cmdline_rest; + panel_rest = *rotation & ~DRM_MODE_ROTATE_MASK; + cmdline_rest = cmdline->rotation_reflection & ~DRM_MODE_ROTATE_MASK; + sum_rest = panel_rest ^ cmdline_rest; - *rotation = (1 << sum_rot) | sum_rest; + *rotation = (1 << sum_rot) | sum_rest; + } else { + *rotation ^= cmdline->rotation_reflection; + } } /* @@ -879,7 +883,8 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation) * depending on the hardware this may require the framebuffer * to be in a specific tiling format. */ - if ((*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_180 || + if (((*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_0 && + (*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_180) || !plane->rotation_property) return false; -- 2.24.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
