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 <[email protected]>
---
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.
+ *
* 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.
+ *
+ * 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,
--
2.43.0
On Tue, 09 Apr 2024 12:04:06 +0200
Louis Chauvet <[email protected]> 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 <[email protected]>
> ---
> 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
On Tue, Apr 09, 2024 at 12:04:06PM +0200, Louis Chauvet wrote:
> @@ -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.
> + *
> + * 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).
Sphinx reports htmldocs warnings on these transformation diagrams:
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:265: ERROR: Undefined substitution referenced: "o +".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:265: ERROR: Undefined substitution referenced: "+ o".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:273: ERROR: Undefined substitution referenced: "o +".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:277: ERROR: Undefined substitution referenced: "o | ROTATE_180 | +".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:277: ERROR: Undefined substitution referenced: "+ | | o".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:281: ERROR: Undefined substitution referenced: "o | ROTATE_270 |+ o".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:290: ERROR: Undefined substitution referenced: "o +".
Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:290: ERROR: Undefined substitution referenced: "+ o".
I have to wrap them in literal blocks:
---- >8 ----
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 6fbb8730d8b022..f2cbf8d8efbbbc 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -259,36 +259,36 @@ EXPORT_SYMBOL(drm_plane_create_alpha_property);
* the X and Y axis are within the source rectangle, i.e. the X/Y axis before
* rotation.
*
- * Here are some examples of rotation and reflections:
+ * Here are some examples of rotation and reflections::
*
- * |o +| REFLECT_X |+ o|
- * | | ========> | |
- * | | | |
+ * |o +| REFLECT_X |+ o|
+ * | | ========> | |
+ * | | | |
*
- * |o | REFLECT_Y |+ |
- * | | ========> | |
- * |+ | |o |
+ * |o | REFLECT_Y |+ |
+ * | | ========> | |
+ * |+ | |o |
*
- * |o +| ROTATE_90 |+ |
- * | | ========> | |
- * | | |o |
+ * |o +| ROTATE_90 |+ |
+ * | | ========> | |
+ * | | |o |
*
- * |o | ROTATE_180 | +|
- * | | ========> | |
- * |+ | | o|
+ * |o | ROTATE_180 | +|
+ * | | ========> | |
+ * |+ | | o|
*
- * |o | ROTATE_270 |+ 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.
*
- * For example the expected result for DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X is:
+ * For example the expected result for DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X is::
*
- * |o +| REFLECT_X |+ o| ROTATE_90 |o |
- * | | ========> | | ========> | |
- * | | | | |+ |
+ * |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).
Thanks.
--
An old man doll... just what I always wanted! - Clara
Le 15/04/24 - 21:28, Bagas Sanjaya a ?crit :
> On Tue, Apr 09, 2024 at 12:04:06PM +0200, Louis Chauvet wrote:
> > @@ -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.
> > + *
> > + * 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).
>
> Sphinx reports htmldocs warnings on these transformation diagrams:
>
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:265: ERROR: Undefined substitution referenced: "o +".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:265: ERROR: Undefined substitution referenced: "+ o".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:273: ERROR: Undefined substitution referenced: "o +".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:277: ERROR: Undefined substitution referenced: "o | ROTATE_180 | +".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:277: ERROR: Undefined substitution referenced: "+ | | o".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:281: ERROR: Undefined substitution referenced: "o | ROTATE_270 |+ o".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:290: ERROR: Undefined substitution referenced: "o +".
> Documentation/gpu/drm-kms:389: ./drivers/gpu/drm/drm_blend.c:290: ERROR: Undefined substitution referenced: "+ o".
>
> I have to wrap them in literal blocks:
>
> ---- >8 ----
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 6fbb8730d8b022..f2cbf8d8efbbbc 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -259,36 +259,36 @@ EXPORT_SYMBOL(drm_plane_create_alpha_property);
> * the X and Y axis are within the source rectangle, i.e. the X/Y axis before
> * rotation.
> *
> - * Here are some examples of rotation and reflections:
> + * Here are some examples of rotation and reflections::
> *
> - * |o +| REFLECT_X |+ o|
> - * | | ========> | |
> - * | | | |
> + * |o +| REFLECT_X |+ o|
> + * | | ========> | |
> + * | | | |
> *
> - * |o | REFLECT_Y |+ |
> - * | | ========> | |
> - * |+ | |o |
> + * |o | REFLECT_Y |+ |
> + * | | ========> | |
> + * |+ | |o |
> *
> - * |o +| ROTATE_90 |+ |
> - * | | ========> | |
> - * | | |o |
> + * |o +| ROTATE_90 |+ |
> + * | | ========> | |
> + * | | |o |
> *
> - * |o | ROTATE_180 | +|
> - * | | ========> | |
> - * |+ | | o|
> + * |o | ROTATE_180 | +|
> + * | | ========> | |
> + * |+ | | o|
> *
> - * |o | ROTATE_270 |+ 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.
> *
> - * For example the expected result for DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X is:
> + * For example the expected result for DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X is::
> *
> - * |o +| REFLECT_X |+ o| ROTATE_90 |o |
> - * | | ========> | | ========> | |
> - * | | | | |+ |
> + * |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).
>
> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara
It will be fixed in the next version, thanks!
Louis Chauvet
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Le 15/04/24 - 14:36, Pekka Paalanen a écrit :
> On Tue, 09 Apr 2024 12:04:06 +0200
> Louis Chauvet <[email protected]> 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 <[email protected]>
> > ---
> > 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?
The first is the FB source, the second is the expected result on the CRTC
output.
I will add a sentence before the schemas:
* Here are some examples of rotation and reflections, on the left it is
* the content of the source frame buffer, on the right is the expected
* result on the CRTC output.
>
> > + *
> > + * 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.
So, somewhere in [1]? It feel strange because this is in the `GPU Driver
Developer’s Guide` section, not a `UAPI interfaces`.
[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html
Thanks,
Louis Chauvet
> Disclaimer again to everyone else: I cannot tell if this is the correct
> documentation or its inverse.
>
>
> Thanks,
> pq
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com