Combining horizontal and vertical reflections gives us 180 degrees of
rotation.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/gpu/drm/tegra/dc.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index f31bca27cde4..ddd9b88f8fce 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -608,6 +608,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
{
struct tegra_plane_state *plane_state = to_tegra_plane_state(state);
unsigned int rotation = DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_180 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y;
struct tegra_bo_tiling *tiling = &plane_state->tiling;
@@ -659,6 +660,14 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
else
plane_state->reflect_y = false;
+ if (tegra_fb_is_bottom_up(state->fb))
+ plane_state->reflect_y = true;
+
+ if (rotation & DRM_MODE_ROTATE_180) {
+ plane_state->reflect_x = !plane_state->reflect_x;
+ plane_state->reflect_y = !plane_state->reflect_y;
+ }
+
/*
* Tegra doesn't support different strides for U and V planes so we
* error out if the user tries to display a framebuffer with such a
@@ -720,7 +729,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
window.dst.h = drm_rect_height(&plane->state->dst);
window.bits_per_pixel = fb->format->cpp[0] * 8;
window.reflect_x = state->reflect_x;
- window.reflect_y = tegra_fb_is_bottom_up(fb) || state->reflect_y;
+ window.reflect_y = state->reflect_y;
/* copy from state */
window.zpos = plane->state->normalized_zpos;
@@ -806,6 +815,7 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
err = drm_plane_create_rotation_property(&plane->base,
DRM_MODE_ROTATE_0,
DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_180 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y);
if (err < 0)
@@ -1094,6 +1104,7 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
err = drm_plane_create_rotation_property(&plane->base,
DRM_MODE_ROTATE_0,
DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_180 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y);
if (err < 0)
--
2.26.0
On Sun, Jun 14, 2020 at 11:01:21PM +0300, Dmitry Osipenko wrote:
> Combining horizontal and vertical reflections gives us 180 degrees of
> rotation.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/gpu/drm/tegra/dc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f31bca27cde4..ddd9b88f8fce 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -608,6 +608,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
> {
> struct tegra_plane_state *plane_state = to_tegra_plane_state(state);
> unsigned int rotation = DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_180 |
Leave this out ...
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y;
> struct tegra_bo_tiling *tiling = &plane_state->tiling;
> @@ -659,6 +660,14 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
> else
> plane_state->reflect_y = false;
>
> + if (tegra_fb_is_bottom_up(state->fb))
> + plane_state->reflect_y = true;
> +
> + if (rotation & DRM_MODE_ROTATE_180) {
> + plane_state->reflect_x = !plane_state->reflect_x;
> + plane_state->reflect_y = !plane_state->reflect_y;
> + }
... and drm_rotation_simplify() will do this for you.
Though the bottim_up() thing will need a slightly different tweak I
guess.
I'd write this as somehting like:
rotation = state->rotation;
if (bottom_up())
rotation ^= DRM_MODE_REFLECT_Y;
rotation = drm_rotation_simplify(rotation,
DRM_MODE_ROTATE_0 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y;
Also note my use of XOR for the bottom_up() handling. I suspect
the current code is already broken if you combine bottom_up()
and REFLECT_Y since it just uses an OR instead of an XOR. That's
assuming my hucnh what bottom_up() is supposed to do is correct.
> +
> /*
> * Tegra doesn't support different strides for U and V planes so we
> * error out if the user tries to display a framebuffer with such a
> @@ -720,7 +729,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
> window.dst.h = drm_rect_height(&plane->state->dst);
> window.bits_per_pixel = fb->format->cpp[0] * 8;
> window.reflect_x = state->reflect_x;
> - window.reflect_y = tegra_fb_is_bottom_up(fb) || state->reflect_y;
> + window.reflect_y = state->reflect_y;
>
> /* copy from state */
> window.zpos = plane->state->normalized_zpos;
> @@ -806,6 +815,7 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
> err = drm_plane_create_rotation_property(&plane->base,
> DRM_MODE_ROTATE_0,
> DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_180 |
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y);
> if (err < 0)
> @@ -1094,6 +1104,7 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
> err = drm_plane_create_rotation_property(&plane->base,
> DRM_MODE_ROTATE_0,
> DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_180 |
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y);
> if (err < 0)
> --
> 2.26.0
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrj?l?
Intel
Hi all,
Perhaps a silly question:
On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko <[email protected]> wrote:
>
> Combining horizontal and vertical reflections gives us 180 degrees of
> rotation.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/gpu/drm/tegra/dc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f31bca27cde4..ddd9b88f8fce 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> + if (rotation & DRM_MODE_ROTATE_180) {
> + plane_state->reflect_x = !plane_state->reflect_x;
> + plane_state->reflect_y = !plane_state->reflect_y;
> + }
> +
As mentioned by Ville the above is already handled by
drm_rotation_simplify() ... although it makes me wonder:
> err = drm_plane_create_rotation_property(&plane->base,
> DRM_MODE_ROTATE_0,
> DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_180 |
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y);
Would it make sense for drm_plane_create_rotation_property() itself,
to add DRM_MODE_ROTATE_180, when both reflections are supported?
-Emil
16.06.2020 00:47, Emil Velikov пишет:
> Hi all,
>
> Perhaps a silly question:
>
> On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko <[email protected]> wrote:
>>
>> Combining horizontal and vertical reflections gives us 180 degrees of
>> rotation.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/gpu/drm/tegra/dc.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index f31bca27cde4..ddd9b88f8fce 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>
>> + if (rotation & DRM_MODE_ROTATE_180) {
>> + plane_state->reflect_x = !plane_state->reflect_x;
>> + plane_state->reflect_y = !plane_state->reflect_y;
>> + }
>> +
> As mentioned by Ville the above is already handled by
> drm_rotation_simplify() ... although it makes me wonder:
>
>
>> err = drm_plane_create_rotation_property(&plane->base,
>> DRM_MODE_ROTATE_0,
>> DRM_MODE_ROTATE_0 |
>> + DRM_MODE_ROTATE_180 |
>> DRM_MODE_REFLECT_X |
>> DRM_MODE_REFLECT_Y);
>
> Would it make sense for drm_plane_create_rotation_property() itself,
> to add DRM_MODE_ROTATE_180, when both reflections are supported?
Hello Emil,
That's a good point! All DRM_MODE_ROTATE_180 should be removed because
Tegra can't do 180° + reflected-x. The DRM core takes care of 180°
rotation when both x/y reflections are supported.
16.06.2020 14:25, Dmitry Osipenko пишет:
> 16.06.2020 00:47, Emil Velikov пишет:
>> Hi all,
>>
>> Perhaps a silly question:
>>
>> On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko <[email protected]> wrote:
>>>
>>> Combining horizontal and vertical reflections gives us 180 degrees of
>>> rotation.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> drivers/gpu/drm/tegra/dc.c | 13 ++++++++++++-
>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>> index f31bca27cde4..ddd9b88f8fce 100644
>>> --- a/drivers/gpu/drm/tegra/dc.c
>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>
>>> + if (rotation & DRM_MODE_ROTATE_180) {
>>> + plane_state->reflect_x = !plane_state->reflect_x;
>>> + plane_state->reflect_y = !plane_state->reflect_y;
>>> + }
>>> +
>> As mentioned by Ville the above is already handled by
>> drm_rotation_simplify() ... although it makes me wonder:
>>
>>
>>> err = drm_plane_create_rotation_property(&plane->base,
>>> DRM_MODE_ROTATE_0,
>>> DRM_MODE_ROTATE_0 |
>>> + DRM_MODE_ROTATE_180 |
>>> DRM_MODE_REFLECT_X |
>>> DRM_MODE_REFLECT_Y);
>>
>> Would it make sense for drm_plane_create_rotation_property() itself,
>> to add DRM_MODE_ROTATE_180, when both reflections are supported?
>
> Hello Emil,
>
> That's a good point! All DRM_MODE_ROTATE_180 should be removed because
> Tegra can't do 180° + reflected-x. The DRM core takes care of 180°
> rotation when both x/y reflections are supported.
>
I just found out that I forgot to drop the WIP patches which added
transparent rotation support while was checking whether these plane
DRM_MODE_ROTATE_180 could be dropped and it's actually need to be set
for the planes, otherwise 180 rotation support is filtered out by the
atomic check.