On 2019-01-10 18:48, Boris Brezillon wrote:
On Thu, 10 Jan 2019 15:10:39 +0000 Peter Rosin peda@axentia.se wrote:
The destination crtc rectangle is independent of source plane rotation.
Signed-off-by: Peter Rosin peda@axentia.se
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index ea8fc0deb814..d6f93f029020 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -642,9 +642,6 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p, * Swap width and size in case of 90 or 270 degrees rotation */ if (drm_rotation_90_or_270(state->base.rotation)) {
tmp = state->crtc_w;state->crtc_w = state->crtc_h;state->crtc_h = tmp;Again, I guess I assumed ->crtc_h/w were the width and height before rotation when I initially added rotation support.
And I thought so too, possibly since I have only been doing drm-stuff with this driver, but I also suspect that the incompleteness of the libdrm modetest program is to blame. I don't think it's possible to specify individual src and dst rectangles with it, and that seems rather limiting when dealing with rotated planes. I can easily see why someone using modetest thinks the crtc rect should be rotated by the driver...
This change might break users too.
Right you are, and the same impossible scenario. Fix things to do the right thing and risk breaking users, or don't and preserve the buggy non-portable issues of the driver making it unusable for others.
I don't care either way, because rotating planes with this stride- method is practically useless here. It simply requires to much memory bandwidth. I might work ok for smaller panels with lower pixel clock frequencies though? I think the LCDC might read the same data more than once when data is not in the "natural" order? (no, I do not need an answer to this question, and I do not have time to dig in this area at the moment...)
However, if you can't do both patch 1 and 2 (because users regress), then patch 3 is no good either. The reason is that drm_atomic_helper_check_plane_state assumes the rotational properties fixed by patch 1 and 2, and the behavior is "odd" if you have that wrong.
Cheers, Peter
tmp = state->src_w; state->src_w = state->src_h; state->src_h = tmp;