linux-kernel - Re: [PATCH 2/3] drm: drm_blend.c: Improve drm_plane_create_rotation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240415143622.7e600508.pekka.paalanen@collabora.com>
Date: Mon, 15 Apr 2024 14:36:22 +0300
From: Pekka Paalanen <pekka.paalanen@...labora.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
 <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 seanpaul@...gle.com, marcheu@...gle.com, nicolejadeyee@...gle.com,
 thomas.petazzoni@...tlin.com
Subject: Re: [PATCH 2/3] drm: drm_blend.c: Improve
 drm_plane_create_rotation_property kernel doc

On Tue, 09 Apr 2024 12:04:06 +0200
Louis Chauvet <louis.chauvet@...tlin.com> wrote:

> The expected behavior of the rotation property was not very clear. Add
> more examples to explain what is the expected result.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
>  drivers/gpu/drm/drm_blend.c | 52 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 8d4b317eb9d7..6fbb8730d8b0 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -104,6 +104,9 @@
>   *	Without this property the rectangle is only scaled, but not rotated or
>   *	reflected.
>   *
> + *	See drm_plane_create_rotation_property() for details about the expected rotation and
> + *	reflection behavior.

I think internal function docs should be referring to UAPI docs, and
not vice versa. Internal functions can change, but UAPI cannot.

> + *
>   *	Possbile values:
>   *
>   *	"rotate-<degrees>":
> @@ -114,18 +117,6 @@
>   *		Signals that the contents of a drm plane is reflected along the
>   *		<axis> axis, in the same way as mirroring.
>   *
> - *	reflect-x::
> - *
> - *			|o |    | o|
> - *			|  | -> |  |
> - *			| v|    |v |
> - *
> - *	reflect-y::
> - *
> - *			|o |    | ^|
> - *			|  | -> |  |
> - *			| v|    |o |
> - *
>   * zpos:
>   *	Z position is set up with drm_plane_create_zpos_immutable_property() and
>   *	drm_plane_create_zpos_property(). It controls the visibility of overlapping
> @@ -266,8 +257,41 @@ EXPORT_SYMBOL(drm_plane_create_alpha_property);
>   *
>   * Rotation is the specified amount in degrees in counter clockwise direction,
>   * the X and Y axis are within the source rectangle, i.e.  the X/Y axis before
> - * rotation. After reflection, the rotation is applied to the image sampled from
> - * the source rectangle, before scaling it to fit the destination rectangle.
> + * rotation.
> + *
> + * Here are some examples of rotation and reflections:
> + *
> + * |o  +|  REFLECT_X  |+  o|
> + * |    |  ========>  |    |
> + * |    |             |    |
> + *
> + * |o   |  REFLECT_Y  |+   |
> + * |    |  ========>  |    |
> + * |+   |             |o   |
> + *
> + * |o  +|  ROTATE_90  |+   |
> + * |    |  ========>  |    |
> + * |    |             |o   |
> + *
> + * |o   |  ROTATE_180 |   +|
> + * |    |  ========>  |    |
> + * |+   |             |   o|
> + *
> + * |o   |  ROTATE_270 |+  o|
> + * |    |  ========>  |    |
> + * |+   |             |    |
> + *
> + * Rotation and reflection can be combined to handle more situations. In this condition, the
> + * reflection is applied first and the rotation in second.

When going in which direction? Is the first image the FB source
rectangle contents, and the second image what the plane looks like in
CRTC frame of reference?

> + *
> + * For example the expected result for DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X is:
> + *
> + * |o  +|  REFLECT_X  |+  o|  ROTATE_90  |o   |
> + * |    |  ========>  |    |  ========>  |    |
> + * |    |             |    |             |+   |
> + *
> + * It is not possible to pass multiple rotation at the same time. (i.e ROTATE_90 | ROTATE_180 is
> + * not the same as ROTATE_270 and is not accepted).
>   */
>  int drm_plane_create_rotation_property(struct drm_plane *plane,
>  				       unsigned int rotation,
> 

These are definitely improvements. I think they should just be in the
UAPI section rather than implementation details.

Disclaimer again to everyone else: I cannot tell if this is the correct
documentation or its inverse.


Thanks,
pq

Content of type "application/pgp-signature" skipped