* [PATCH v4 1/6] drm/vkms: isolate pixel conversion functionality
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-04-24 21:22 ` Maíra Canal
2023-04-18 13:05 ` [PATCH v4 2/6] drm/vkms: add rotate-0 and reflect-x property Maíra Canal
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Currently, the pixel conversion functions repeat the same loop to
iterate the rows. Instead of repeating the same code for each pixel
format, create a function to wrap the loop and isolate the pixel
conversion functionality.
Suggested-by: Arthur Grillo <arthurgrillo@riseup.net>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>
---
drivers/gpu/drm/vkms/vkms_composer.c | 4 +-
drivers/gpu/drm/vkms/vkms_drv.h | 4 +-
drivers/gpu/drm/vkms/vkms_formats.c | 125 +++++++++++----------------
drivers/gpu/drm/vkms/vkms_formats.h | 2 +-
drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
5 files changed, 56 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 8e53fa80742b..80164e79af00 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -99,7 +99,7 @@ static void blend(struct vkms_writeback_job *wb,
if (!check_y_limit(plane[i]->frame_info, y))
continue;
- plane[i]->plane_read(stage_buffer, plane[i]->frame_info, y);
+ vkms_compose_row(stage_buffer, plane[i], y);
pre_mul_alpha_blend(plane[i]->frame_info, stage_buffer,
output_buffer);
}
@@ -118,7 +118,7 @@ static int check_format_funcs(struct vkms_crtc_state *crtc_state,
u32 n_active_planes = crtc_state->num_active_planes;
for (size_t i = 0; i < n_active_planes; i++)
- if (!planes[i]->plane_read)
+ if (!planes[i]->pixel_read)
return -1;
if (active_wb && !active_wb->wb_write)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 4a248567efb2..f152d54baf76 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -56,8 +56,7 @@ struct vkms_writeback_job {
struct vkms_plane_state {
struct drm_shadow_plane_state base;
struct vkms_frame_info *frame_info;
- void (*plane_read)(struct line_buffer *buffer,
- const struct vkms_frame_info *frame_info, int y);
+ void (*pixel_read)(u8 *src_buffer, struct pixel_argb_u16 *out_pixel);
};
struct vkms_plane {
@@ -155,6 +154,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
/* Composer Support */
void vkms_composer_worker(struct work_struct *work);
void vkms_set_composer(struct vkms_output *out, bool enabled);
+void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state *plane, int y);
/* Writeback */
int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index d4950688b3f1..8d948c73741e 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -42,100 +42,75 @@ static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y
return packed_pixels_addr(frame_info, x_src, y_src);
}
-static void ARGB8888_to_argb_u16(struct line_buffer *stage_buffer,
- const struct vkms_frame_info *frame_info, int y)
+static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
- struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
- u8 *src_pixels = get_packed_src_addr(frame_info, y);
- int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
- stage_buffer->n_pixels);
-
- for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
- /*
- * The 257 is the "conversion ratio". This number is obtained by the
- * (2^16 - 1) / (2^8 - 1) division. Which, in this case, tries to get
- * the best color value in a pixel format with more possibilities.
- * A similar idea applies to others RGB color conversions.
- */
- out_pixels[x].a = (u16)src_pixels[3] * 257;
- out_pixels[x].r = (u16)src_pixels[2] * 257;
- out_pixels[x].g = (u16)src_pixels[1] * 257;
- out_pixels[x].b = (u16)src_pixels[0] * 257;
- }
+ /*
+ * The 257 is the "conversion ratio". This number is obtained by the
+ * (2^16 - 1) / (2^8 - 1) division. Which, in this case, tries to get
+ * the best color value in a pixel format with more possibilities.
+ * A similar idea applies to others RGB color conversions.
+ */
+ out_pixel->a = (u16)src_pixels[3] * 257;
+ out_pixel->r = (u16)src_pixels[2] * 257;
+ out_pixel->g = (u16)src_pixels[1] * 257;
+ out_pixel->b = (u16)src_pixels[0] * 257;
}
-static void XRGB8888_to_argb_u16(struct line_buffer *stage_buffer,
- const struct vkms_frame_info *frame_info, int y)
+static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
- struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
- u8 *src_pixels = get_packed_src_addr(frame_info, y);
- int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
- stage_buffer->n_pixels);
-
- for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
- out_pixels[x].a = (u16)0xffff;
- out_pixels[x].r = (u16)src_pixels[2] * 257;
- out_pixels[x].g = (u16)src_pixels[1] * 257;
- out_pixels[x].b = (u16)src_pixels[0] * 257;
- }
+ out_pixel->a = (u16)0xffff;
+ out_pixel->r = (u16)src_pixels[2] * 257;
+ out_pixel->g = (u16)src_pixels[1] * 257;
+ out_pixel->b = (u16)src_pixels[0] * 257;
}
-static void ARGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
- const struct vkms_frame_info *frame_info,
- int y)
+static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
- struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
- u16 *src_pixels = get_packed_src_addr(frame_info, y);
- int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
- stage_buffer->n_pixels);
+ u16 *pixels = (u16 *)src_pixels;
- for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
- out_pixels[x].a = le16_to_cpu(src_pixels[3]);
- out_pixels[x].r = le16_to_cpu(src_pixels[2]);
- out_pixels[x].g = le16_to_cpu(src_pixels[1]);
- out_pixels[x].b = le16_to_cpu(src_pixels[0]);
- }
+ out_pixel->a = le16_to_cpu(pixels[3]);
+ out_pixel->r = le16_to_cpu(pixels[2]);
+ out_pixel->g = le16_to_cpu(pixels[1]);
+ out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void XRGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
- const struct vkms_frame_info *frame_info,
- int y)
+static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
- struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
- u16 *src_pixels = get_packed_src_addr(frame_info, y);
- int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
- stage_buffer->n_pixels);
+ u16 *pixels = (u16 *)src_pixels;
- for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
- out_pixels[x].a = (u16)0xffff;
- out_pixels[x].r = le16_to_cpu(src_pixels[2]);
- out_pixels[x].g = le16_to_cpu(src_pixels[1]);
- out_pixels[x].b = le16_to_cpu(src_pixels[0]);
- }
+ out_pixel->a = (u16)0xffff;
+ out_pixel->r = le16_to_cpu(pixels[2]);
+ out_pixel->g = le16_to_cpu(pixels[1]);
+ out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void RGB565_to_argb_u16(struct line_buffer *stage_buffer,
- const struct vkms_frame_info *frame_info, int y)
+static void RGB565_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
- struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
- u16 *src_pixels = get_packed_src_addr(frame_info, y);
- int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
- stage_buffer->n_pixels);
+ u16 *pixels = (u16 *)src_pixels;
s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31));
s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63));
- for (size_t x = 0; x < x_limit; x++, src_pixels++) {
- u16 rgb_565 = le16_to_cpu(*src_pixels);
- s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
- s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
- s64 fp_b = drm_int2fixp(rgb_565 & 0x1f);
+ u16 rgb_565 = le16_to_cpu(*pixels);
+ s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
+ s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
+ s64 fp_b = drm_int2fixp(rgb_565 & 0x1f);
- out_pixels[x].a = (u16)0xffff;
- out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
- out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
- out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
- }
+ out_pixel->a = (u16)0xffff;
+ out_pixel->r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
+ out_pixel->g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
+ out_pixel->b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
+}
+
+void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state *plane, int y)
+{
+ struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
+ struct vkms_frame_info *frame_info = plane->frame_info;
+ u8 *src_pixels = get_packed_src_addr(frame_info, y);
+ int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
+
+ for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp)
+ plane->pixel_read(src_pixels, &out_pixels[x]);
}
/*
@@ -249,7 +224,7 @@ static void argb_u16_to_RGB565(struct vkms_frame_info *frame_info,
}
}
-void *get_frame_to_line_function(u32 format)
+void *get_pixel_conversion_function(u32 format)
{
switch (format) {
case DRM_FORMAT_ARGB8888:
diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
index 43b7c1979018..c5b113495d0c 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.h
+++ b/drivers/gpu/drm/vkms/vkms_formats.h
@@ -5,7 +5,7 @@
#include "vkms_drv.h"
-void *get_frame_to_line_function(u32 format);
+void *get_pixel_conversion_function(u32 format);
void *get_line_to_frame_function(u32 format);
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index c41cec7dcb70..0a23875900ec 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -123,7 +123,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
frame_info->offset = fb->offsets[0];
frame_info->pitch = fb->pitches[0];
frame_info->cpp = fb->format->cpp[0];
- vkms_plane_state->plane_read = get_frame_to_line_function(fmt);
+ vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt);
}
static int vkms_plane_atomic_check(struct drm_plane *plane,
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 1/6] drm/vkms: isolate pixel conversion functionality
2023-04-18 13:05 ` [PATCH v4 1/6] drm/vkms: isolate pixel conversion functionality Maíra Canal
@ 2023-04-24 21:22 ` Maíra Canal
0 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-04-24 21:22 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: dri-devel
On 4/18/23 10:05, Maíra Canal wrote:
> Currently, the pixel conversion functions repeat the same loop to
> iterate the rows. Instead of repeating the same code for each pixel
> format, create a function to wrap the loop and isolate the pixel
> conversion functionality.
>
> Suggested-by: Arthur Grillo <arthurgrillo@riseup.net>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>
Applied to drm/drm-misc (drm-misc-next).
Best Regards,
- Maíra Canal
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 4 +-
> drivers/gpu/drm/vkms/vkms_drv.h | 4 +-
> drivers/gpu/drm/vkms/vkms_formats.c | 125 +++++++++++----------------
> drivers/gpu/drm/vkms/vkms_formats.h | 2 +-
> drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
> 5 files changed, 56 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 8e53fa80742b..80164e79af00 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -99,7 +99,7 @@ static void blend(struct vkms_writeback_job *wb,
> if (!check_y_limit(plane[i]->frame_info, y))
> continue;
>
> - plane[i]->plane_read(stage_buffer, plane[i]->frame_info, y);
> + vkms_compose_row(stage_buffer, plane[i], y);
> pre_mul_alpha_blend(plane[i]->frame_info, stage_buffer,
> output_buffer);
> }
> @@ -118,7 +118,7 @@ static int check_format_funcs(struct vkms_crtc_state *crtc_state,
> u32 n_active_planes = crtc_state->num_active_planes;
>
> for (size_t i = 0; i < n_active_planes; i++)
> - if (!planes[i]->plane_read)
> + if (!planes[i]->pixel_read)
> return -1;
>
> if (active_wb && !active_wb->wb_write)
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 4a248567efb2..f152d54baf76 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -56,8 +56,7 @@ struct vkms_writeback_job {
> struct vkms_plane_state {
> struct drm_shadow_plane_state base;
> struct vkms_frame_info *frame_info;
> - void (*plane_read)(struct line_buffer *buffer,
> - const struct vkms_frame_info *frame_info, int y);
> + void (*pixel_read)(u8 *src_buffer, struct pixel_argb_u16 *out_pixel);
> };
>
> struct vkms_plane {
> @@ -155,6 +154,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> /* Composer Support */
> void vkms_composer_worker(struct work_struct *work);
> void vkms_set_composer(struct vkms_output *out, bool enabled);
> +void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state *plane, int y);
>
> /* Writeback */
> int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index d4950688b3f1..8d948c73741e 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -42,100 +42,75 @@ static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y
> return packed_pixels_addr(frame_info, x_src, y_src);
> }
>
> -static void ARGB8888_to_argb_u16(struct line_buffer *stage_buffer,
> - const struct vkms_frame_info *frame_info, int y)
> +static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
> {
> - struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> - u8 *src_pixels = get_packed_src_addr(frame_info, y);
> - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> - stage_buffer->n_pixels);
> -
> - for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
> - /*
> - * The 257 is the "conversion ratio". This number is obtained by the
> - * (2^16 - 1) / (2^8 - 1) division. Which, in this case, tries to get
> - * the best color value in a pixel format with more possibilities.
> - * A similar idea applies to others RGB color conversions.
> - */
> - out_pixels[x].a = (u16)src_pixels[3] * 257;
> - out_pixels[x].r = (u16)src_pixels[2] * 257;
> - out_pixels[x].g = (u16)src_pixels[1] * 257;
> - out_pixels[x].b = (u16)src_pixels[0] * 257;
> - }
> + /*
> + * The 257 is the "conversion ratio". This number is obtained by the
> + * (2^16 - 1) / (2^8 - 1) division. Which, in this case, tries to get
> + * the best color value in a pixel format with more possibilities.
> + * A similar idea applies to others RGB color conversions.
> + */
> + out_pixel->a = (u16)src_pixels[3] * 257;
> + out_pixel->r = (u16)src_pixels[2] * 257;
> + out_pixel->g = (u16)src_pixels[1] * 257;
> + out_pixel->b = (u16)src_pixels[0] * 257;
> }
>
> -static void XRGB8888_to_argb_u16(struct line_buffer *stage_buffer,
> - const struct vkms_frame_info *frame_info, int y)
> +static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
> {
> - struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> - u8 *src_pixels = get_packed_src_addr(frame_info, y);
> - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> - stage_buffer->n_pixels);
> -
> - for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
> - out_pixels[x].a = (u16)0xffff;
> - out_pixels[x].r = (u16)src_pixels[2] * 257;
> - out_pixels[x].g = (u16)src_pixels[1] * 257;
> - out_pixels[x].b = (u16)src_pixels[0] * 257;
> - }
> + out_pixel->a = (u16)0xffff;
> + out_pixel->r = (u16)src_pixels[2] * 257;
> + out_pixel->g = (u16)src_pixels[1] * 257;
> + out_pixel->b = (u16)src_pixels[0] * 257;
> }
>
> -static void ARGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
> - const struct vkms_frame_info *frame_info,
> - int y)
> +static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
> {
> - struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> - u16 *src_pixels = get_packed_src_addr(frame_info, y);
> - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> - stage_buffer->n_pixels);
> + u16 *pixels = (u16 *)src_pixels;
>
> - for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
> - out_pixels[x].a = le16_to_cpu(src_pixels[3]);
> - out_pixels[x].r = le16_to_cpu(src_pixels[2]);
> - out_pixels[x].g = le16_to_cpu(src_pixels[1]);
> - out_pixels[x].b = le16_to_cpu(src_pixels[0]);
> - }
> + out_pixel->a = le16_to_cpu(pixels[3]);
> + out_pixel->r = le16_to_cpu(pixels[2]);
> + out_pixel->g = le16_to_cpu(pixels[1]);
> + out_pixel->b = le16_to_cpu(pixels[0]);
> }
>
> -static void XRGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
> - const struct vkms_frame_info *frame_info,
> - int y)
> +static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
> {
> - struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> - u16 *src_pixels = get_packed_src_addr(frame_info, y);
> - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> - stage_buffer->n_pixels);
> + u16 *pixels = (u16 *)src_pixels;
>
> - for (size_t x = 0; x < x_limit; x++, src_pixels += 4) {
> - out_pixels[x].a = (u16)0xffff;
> - out_pixels[x].r = le16_to_cpu(src_pixels[2]);
> - out_pixels[x].g = le16_to_cpu(src_pixels[1]);
> - out_pixels[x].b = le16_to_cpu(src_pixels[0]);
> - }
> + out_pixel->a = (u16)0xffff;
> + out_pixel->r = le16_to_cpu(pixels[2]);
> + out_pixel->g = le16_to_cpu(pixels[1]);
> + out_pixel->b = le16_to_cpu(pixels[0]);
> }
>
> -static void RGB565_to_argb_u16(struct line_buffer *stage_buffer,
> - const struct vkms_frame_info *frame_info, int y)
> +static void RGB565_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
> {
> - struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> - u16 *src_pixels = get_packed_src_addr(frame_info, y);
> - int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
> - stage_buffer->n_pixels);
> + u16 *pixels = (u16 *)src_pixels;
>
> s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31));
> s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63));
>
> - for (size_t x = 0; x < x_limit; x++, src_pixels++) {
> - u16 rgb_565 = le16_to_cpu(*src_pixels);
> - s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
> - s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
> - s64 fp_b = drm_int2fixp(rgb_565 & 0x1f);
> + u16 rgb_565 = le16_to_cpu(*pixels);
> + s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
> + s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
> + s64 fp_b = drm_int2fixp(rgb_565 & 0x1f);
>
> - out_pixels[x].a = (u16)0xffff;
> - out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
> - out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
> - out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
> - }
> + out_pixel->a = (u16)0xffff;
> + out_pixel->r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
> + out_pixel->g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
> + out_pixel->b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
> +}
> +
> +void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state *plane, int y)
> +{
> + struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
> + struct vkms_frame_info *frame_info = plane->frame_info;
> + u8 *src_pixels = get_packed_src_addr(frame_info, y);
> + int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
> +
> + for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp)
> + plane->pixel_read(src_pixels, &out_pixels[x]);
> }
>
> /*
> @@ -249,7 +224,7 @@ static void argb_u16_to_RGB565(struct vkms_frame_info *frame_info,
> }
> }
>
> -void *get_frame_to_line_function(u32 format)
> +void *get_pixel_conversion_function(u32 format)
> {
> switch (format) {
> case DRM_FORMAT_ARGB8888:
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
> index 43b7c1979018..c5b113495d0c 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.h
> +++ b/drivers/gpu/drm/vkms/vkms_formats.h
> @@ -5,7 +5,7 @@
>
> #include "vkms_drv.h"
>
> -void *get_frame_to_line_function(u32 format);
> +void *get_pixel_conversion_function(u32 format);
>
> void *get_line_to_frame_function(u32 format);
>
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index c41cec7dcb70..0a23875900ec 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -123,7 +123,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
> frame_info->offset = fb->offsets[0];
> frame_info->pitch = fb->pitches[0];
> frame_info->cpp = fb->format->cpp[0];
> - vkms_plane_state->plane_read = get_frame_to_line_function(fmt);
> + vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt);
> }
>
> static int vkms_plane_atomic_check(struct drm_plane *plane,
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/6] drm/vkms: add rotate-0 and reflect-x property
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
2023-04-18 13:05 ` [PATCH v4 1/6] drm/vkms: isolate pixel conversion functionality Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-04-18 13:05 ` [PATCH v4 3/6] drm/vkms: add reflect-y and rotate-180 property Maíra Canal
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Currently, vkms doesn't support any reflection property. Therefore, add
the reflect-x property to vkms through a software implementation of the
operation. This is possible by reverse reading the x axis during the
blending.
Tested with igt@kms_rotation_crc@primary-reflect-x and
igt@kms_rotation_crc@sprite-reflect-x [1].
[1] https://patchwork.freedesktop.org/series/116025/
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
drivers/gpu/drm/vkms/vkms_formats.c | 16 +++++++++++++---
drivers/gpu/drm/vkms/vkms_plane.c | 12 ++++++++++++
4 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 80164e79af00..a4436981cbcd 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -55,7 +55,7 @@ static void pre_mul_alpha_blend(struct vkms_frame_info *frame_info,
static bool check_y_limit(struct vkms_frame_info *frame_info, int y)
{
- if (y >= frame_info->dst.y1 && y < frame_info->dst.y2)
+ if (y >= frame_info->rotated.y1 && y < frame_info->rotated.y2)
return true;
return false;
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f152d54baf76..5f1a0a44a78c 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -26,7 +26,9 @@
struct vkms_frame_info {
struct drm_framebuffer *fb;
struct drm_rect src, dst;
+ struct drm_rect rotated;
struct iosys_map map[DRM_FORMAT_MAX_PLANES];
+ unsigned int rotation;
unsigned int offset;
unsigned int pitch;
unsigned int cpp;
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 8d948c73741e..f59b1c48a563 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -37,11 +37,18 @@ static void *packed_pixels_addr(const struct vkms_frame_info *frame_info,
static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y)
{
int x_src = frame_info->src.x1 >> 16;
- int y_src = y - frame_info->dst.y1 + (frame_info->src.y1 >> 16);
+ int y_src = y - frame_info->rotated.y1 + (frame_info->src.y1 >> 16);
return packed_pixels_addr(frame_info, x_src, y_src);
}
+static int get_x_position(const struct vkms_frame_info *frame_info, int limit, int x)
+{
+ if (frame_info->rotation & DRM_MODE_REFLECT_X)
+ return limit - x - 1;
+ return x;
+}
+
static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
{
/*
@@ -109,8 +116,11 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
u8 *src_pixels = get_packed_src_addr(frame_info, y);
int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
- for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp)
- plane->pixel_read(src_pixels, &out_pixels[x]);
+ for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
+ int x_pos = get_x_position(frame_info, limit, x);
+
+ plane->pixel_read(src_pixels, &out_pixels[x_pos]);
+ }
}
/*
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 0a23875900ec..3639afd7da47 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -4,6 +4,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
@@ -117,9 +118,16 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
frame_info = vkms_plane_state->frame_info;
memcpy(&frame_info->src, &new_state->src, sizeof(struct drm_rect));
memcpy(&frame_info->dst, &new_state->dst, sizeof(struct drm_rect));
+ memcpy(&frame_info->rotated, &new_state->dst, sizeof(struct drm_rect));
frame_info->fb = fb;
memcpy(&frame_info->map, &shadow_plane_state->data, sizeof(frame_info->map));
drm_framebuffer_get(frame_info->fb);
+ frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_0 |
+ DRM_MODE_REFLECT_X);
+
+ drm_rect_rotate(&frame_info->rotated, drm_rect_width(&frame_info->rotated),
+ drm_rect_height(&frame_info->rotated), frame_info->rotation);
+
frame_info->offset = fb->offsets[0];
frame_info->pitch = fb->pitches[0];
frame_info->cpp = fb->format->cpp[0];
@@ -229,5 +237,9 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
drm_plane_helper_add(&plane->base, funcs);
+ drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
+ DRM_MODE_ROTATE_0 |
+ DRM_MODE_REFLECT_X);
+
return plane;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 3/6] drm/vkms: add reflect-y and rotate-180 property
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
2023-04-18 13:05 ` [PATCH v4 1/6] drm/vkms: isolate pixel conversion functionality Maíra Canal
2023-04-18 13:05 ` [PATCH v4 2/6] drm/vkms: add rotate-0 and reflect-x property Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-04-18 13:05 ` [PATCH v4 4/6] drm/vkms: add rotate-90 property Maíra Canal
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Currently, vkms only supports the reflect-x property. Therefore, add the
reflect-y property to vkms through a software implementation of the
operation. This is possible by reverse reading the y axis during the
blending.
Note that, by implementing the reflect-x and reflect-y properties, it is
also possible to add the rotate-180 property, as it is a combination
of those two properties.
Tested with igt@kms_rotation_crc@primary-reflect-y [1],
igt@kms_rotation_crc@sprite-reflect-y [1],
igt@kms_rotation_crc@primary-rotation-180,
igt@kms_rotation_crc@sprite-rotation-180,
and igt@kms_rotation_crc@cursor-rotation-180.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 16 +++++++++++++---
drivers/gpu/drm/vkms/vkms_plane.c | 7 +++++--
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index a4436981cbcd..6c5ef11b3943 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -53,6 +53,13 @@ static void pre_mul_alpha_blend(struct vkms_frame_info *frame_info,
}
}
+static int get_y_pos(struct vkms_frame_info *frame_info, int y)
+{
+ if (frame_info->rotation & DRM_MODE_REFLECT_Y)
+ return drm_rect_height(&frame_info->rotated) - y - 1;
+ return y;
+}
+
static bool check_y_limit(struct vkms_frame_info *frame_info, int y)
{
if (y >= frame_info->rotated.y1 && y < frame_info->rotated.y2)
@@ -86,6 +93,7 @@ static void blend(struct vkms_writeback_job *wb,
{
struct vkms_plane_state **plane = crtc_state->active_planes;
u32 n_active_planes = crtc_state->num_active_planes;
+ int y_pos;
const struct pixel_argb_u16 background_color = { .a = 0xffff };
@@ -96,10 +104,12 @@ static void blend(struct vkms_writeback_job *wb,
/* The active planes are composed associatively in z-order. */
for (size_t i = 0; i < n_active_planes; i++) {
- if (!check_y_limit(plane[i]->frame_info, y))
+ y_pos = get_y_pos(plane[i]->frame_info, y);
+
+ if (!check_y_limit(plane[i]->frame_info, y_pos))
continue;
- vkms_compose_row(stage_buffer, plane[i], y);
+ vkms_compose_row(stage_buffer, plane[i], y_pos);
pre_mul_alpha_blend(plane[i]->frame_info, stage_buffer,
output_buffer);
}
@@ -107,7 +117,7 @@ static void blend(struct vkms_writeback_job *wb,
*crc32 = crc32_le(*crc32, (void *)output_buffer->pixels, row_size);
if (wb)
- wb->wb_write(&wb->wb_frame_info, output_buffer, y);
+ wb->wb_write(&wb->wb_frame_info, output_buffer, y_pos);
}
}
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 3639afd7da47..904a278e07d7 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -123,7 +123,8 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
memcpy(&frame_info->map, &shadow_plane_state->data, sizeof(frame_info->map));
drm_framebuffer_get(frame_info->fb);
frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_0 |
- DRM_MODE_REFLECT_X);
+ DRM_MODE_REFLECT_X |
+ DRM_MODE_REFLECT_Y);
drm_rect_rotate(&frame_info->rotated, drm_rect_width(&frame_info->rotated),
drm_rect_height(&frame_info->rotated), frame_info->rotation);
@@ -239,7 +240,9 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
DRM_MODE_ROTATE_0 |
- DRM_MODE_REFLECT_X);
+ DRM_MODE_ROTATE_180 |
+ DRM_MODE_REFLECT_X |
+ DRM_MODE_REFLECT_Y);
return plane;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 4/6] drm/vkms: add rotate-90 property
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
` (2 preceding siblings ...)
2023-04-18 13:05 ` [PATCH v4 3/6] drm/vkms: add reflect-y and rotate-180 property Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-05-07 20:48 ` Melissa Wen
2023-04-18 13:05 ` [PATCH v4 5/6] drm/vkms: add rotate-270 property Maíra Canal
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Currently, vkms only supports the rotate-180, reflect-x and reflect-y
properties. Therefore, improve the vkms IGT test coverage by adding the
rotate-90 property to vkms. The rotation was implement by software: rotate
the way the blending occurs by making the source x axis be the destination
y axis and the source y axis be the destination x axis.
Tested with igt@kms_rotation_crc@primary-rotation-90,
igt@kms_rotation_crc@sprite-rotation-90, and
igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 21 ++++++++++++++++-----
drivers/gpu/drm/vkms/vkms_formats.c | 4 ++++
drivers/gpu/drm/vkms/vkms_plane.c | 2 ++
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 6c5ef11b3943..491850ffeac9 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -57,13 +57,24 @@ static int get_y_pos(struct vkms_frame_info *frame_info, int y)
{
if (frame_info->rotation & DRM_MODE_REFLECT_Y)
return drm_rect_height(&frame_info->rotated) - y - 1;
- return y;
+
+ switch (frame_info->rotation & DRM_MODE_ROTATE_MASK) {
+ case DRM_MODE_ROTATE_90:
+ return frame_info->rotated.x2 - y - 1;
+ default:
+ return y;
+ }
}
-static bool check_y_limit(struct vkms_frame_info *frame_info, int y)
+static bool check_limit(struct vkms_frame_info *frame_info, int pos)
{
- if (y >= frame_info->rotated.y1 && y < frame_info->rotated.y2)
- return true;
+ if (frame_info->rotation & DRM_MODE_ROTATE_90) {
+ if (pos >= 0 && pos < drm_rect_width(&frame_info->rotated))
+ return true;
+ } else {
+ if (pos >= frame_info->rotated.y1 && pos < frame_info->rotated.y2)
+ return true;
+ }
return false;
}
@@ -106,7 +117,7 @@ static void blend(struct vkms_writeback_job *wb,
for (size_t i = 0; i < n_active_planes; i++) {
y_pos = get_y_pos(plane[i]->frame_info, y);
- if (!check_y_limit(plane[i]->frame_info, y_pos))
+ if (!check_limit(plane[i]->frame_info, y_pos))
continue;
vkms_compose_row(stage_buffer, plane[i], y_pos);
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index f59b1c48a563..f56f359e0d2d 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -119,6 +119,10 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
int x_pos = get_x_position(frame_info, limit, x);
+ if (frame_info->rotation & DRM_MODE_ROTATE_90)
+ src_pixels = get_packed_src_addr(frame_info, x + frame_info->rotated.y1)
+ + frame_info->cpp * y;
+
plane->pixel_read(src_pixels, &out_pixels[x_pos]);
}
}
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 904a278e07d7..9e451ce97099 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -123,6 +123,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
memcpy(&frame_info->map, &shadow_plane_state->data, sizeof(frame_info->map));
drm_framebuffer_get(frame_info->fb);
frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_90 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y);
@@ -240,6 +241,7 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_90 |
DRM_MODE_ROTATE_180 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y);
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 4/6] drm/vkms: add rotate-90 property
2023-04-18 13:05 ` [PATCH v4 4/6] drm/vkms: add rotate-90 property Maíra Canal
@ 2023-05-07 20:48 ` Melissa Wen
0 siblings, 0 replies; 11+ messages in thread
From: Melissa Wen @ 2023-05-07 20:48 UTC (permalink / raw)
To: Maíra Canal
Cc: Haneen Mohammed, Rodrigo Siqueira, dri-devel, Arthur Grillo,
Igor Matheus Andrade Torrente
On 04/18, Maíra Canal wrote:
> Currently, vkms only supports the rotate-180, reflect-x and reflect-y
> properties. Therefore, improve the vkms IGT test coverage by adding the
> rotate-90 property to vkms. The rotation was implement by software: rotate
> the way the blending occurs by making the source x axis be the destination
> y axis and the source y axis be the destination x axis.
>
> Tested with igt@kms_rotation_crc@primary-rotation-90,
> igt@kms_rotation_crc@sprite-rotation-90, and
> igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.
In the same way you did for reflect-x, could you add the link for your
IGT patchset here to avoid misunderstanding between VKMS and IGT tests
status? Double-check if it's necessary for other commits in this series.
Thanks,
Melissa
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 21 ++++++++++++++++-----
> drivers/gpu/drm/vkms/vkms_formats.c | 4 ++++
> drivers/gpu/drm/vkms/vkms_plane.c | 2 ++
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 6c5ef11b3943..491850ffeac9 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -57,13 +57,24 @@ static int get_y_pos(struct vkms_frame_info *frame_info, int y)
> {
> if (frame_info->rotation & DRM_MODE_REFLECT_Y)
> return drm_rect_height(&frame_info->rotated) - y - 1;
> - return y;
> +
> + switch (frame_info->rotation & DRM_MODE_ROTATE_MASK) {
> + case DRM_MODE_ROTATE_90:
> + return frame_info->rotated.x2 - y - 1;
> + default:
> + return y;
> + }
> }
>
> -static bool check_y_limit(struct vkms_frame_info *frame_info, int y)
> +static bool check_limit(struct vkms_frame_info *frame_info, int pos)
> {
> - if (y >= frame_info->rotated.y1 && y < frame_info->rotated.y2)
> - return true;
> + if (frame_info->rotation & DRM_MODE_ROTATE_90) {
> + if (pos >= 0 && pos < drm_rect_width(&frame_info->rotated))
> + return true;
> + } else {
> + if (pos >= frame_info->rotated.y1 && pos < frame_info->rotated.y2)
> + return true;
> + }
>
> return false;
> }
> @@ -106,7 +117,7 @@ static void blend(struct vkms_writeback_job *wb,
> for (size_t i = 0; i < n_active_planes; i++) {
> y_pos = get_y_pos(plane[i]->frame_info, y);
>
> - if (!check_y_limit(plane[i]->frame_info, y_pos))
> + if (!check_limit(plane[i]->frame_info, y_pos))
> continue;
>
> vkms_compose_row(stage_buffer, plane[i], y_pos);
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index f59b1c48a563..f56f359e0d2d 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -119,6 +119,10 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
> for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
> int x_pos = get_x_position(frame_info, limit, x);
>
> + if (frame_info->rotation & DRM_MODE_ROTATE_90)
> + src_pixels = get_packed_src_addr(frame_info, x + frame_info->rotated.y1)
> + + frame_info->cpp * y;
> +
> plane->pixel_read(src_pixels, &out_pixels[x_pos]);
> }
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 904a278e07d7..9e451ce97099 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -123,6 +123,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
> memcpy(&frame_info->map, &shadow_plane_state->data, sizeof(frame_info->map));
> drm_framebuffer_get(frame_info->fb);
> frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_90 |
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y);
>
> @@ -240,6 +241,7 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
>
> drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
> DRM_MODE_ROTATE_0 |
> + DRM_MODE_ROTATE_90 |
> DRM_MODE_ROTATE_180 |
> DRM_MODE_REFLECT_X |
> DRM_MODE_REFLECT_Y);
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 5/6] drm/vkms: add rotate-270 property
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
` (3 preceding siblings ...)
2023-04-18 13:05 ` [PATCH v4 4/6] drm/vkms: add rotate-90 property Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-04-18 13:05 ` [PATCH v4 6/6] drm/vkms: drop "Rotation" TODO Maíra Canal
2023-05-07 20:54 ` [PATCH v4 0/6] drm/vkms: introduce plane rotation property Melissa Wen
6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Currently, vkms supports the rotate-90, rotate-180, reflect-x and
reflect-y properties. Therefore, improve the vkms IGT test coverage by
adding the rotate-270 property to vkms. The rotation was implement by
software: rotate the way the blending occurs by making the source x axis
be the destination y axis and the source y axis be the destination x
axis and reverse-read the axis.
Now, vkms supports all possible rotation values.
Tested with igt@kms_rotation_crc@primary-rotation-270,
and igt@kms_rotation_crc@sprite-rotation-270.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 5 ++++-
drivers/gpu/drm/vkms/vkms_formats.c | 6 ++++--
drivers/gpu/drm/vkms/vkms_plane.c | 7 ++-----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 491850ffeac9..906d3df40cdb 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -4,6 +4,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_vblank.h>
@@ -61,6 +62,8 @@ static int get_y_pos(struct vkms_frame_info *frame_info, int y)
switch (frame_info->rotation & DRM_MODE_ROTATE_MASK) {
case DRM_MODE_ROTATE_90:
return frame_info->rotated.x2 - y - 1;
+ case DRM_MODE_ROTATE_270:
+ return y + frame_info->rotated.x1;
default:
return y;
}
@@ -68,7 +71,7 @@ static int get_y_pos(struct vkms_frame_info *frame_info, int y)
static bool check_limit(struct vkms_frame_info *frame_info, int pos)
{
- if (frame_info->rotation & DRM_MODE_ROTATE_90) {
+ if (drm_rotation_90_or_270(frame_info->rotation)) {
if (pos >= 0 && pos < drm_rect_width(&frame_info->rotated))
return true;
} else {
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index f56f359e0d2d..ebacb8efa055 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -2,6 +2,8 @@
#include <linux/kernel.h>
#include <linux/minmax.h>
+
+#include <drm/drm_blend.h>
#include <drm/drm_rect.h>
#include <drm/drm_fixed.h>
@@ -44,7 +46,7 @@ static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y
static int get_x_position(const struct vkms_frame_info *frame_info, int limit, int x)
{
- if (frame_info->rotation & DRM_MODE_REFLECT_X)
+ if (frame_info->rotation & (DRM_MODE_REFLECT_X | DRM_MODE_ROTATE_270))
return limit - x - 1;
return x;
}
@@ -119,7 +121,7 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
int x_pos = get_x_position(frame_info, limit, x);
- if (frame_info->rotation & DRM_MODE_ROTATE_90)
+ if (drm_rotation_90_or_270(frame_info->rotation))
src_pixels = get_packed_src_addr(frame_info, x + frame_info->rotated.y1)
+ frame_info->cpp * y;
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 9e451ce97099..66263081639d 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -124,6 +124,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane,
drm_framebuffer_get(frame_info->fb);
frame_info->rotation = drm_rotation_simplify(new_state->rotation, DRM_MODE_ROTATE_0 |
DRM_MODE_ROTATE_90 |
+ DRM_MODE_ROTATE_270 |
DRM_MODE_REFLECT_X |
DRM_MODE_REFLECT_Y);
@@ -240,11 +241,7 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
drm_plane_helper_add(&plane->base, funcs);
drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
- DRM_MODE_ROTATE_0 |
- DRM_MODE_ROTATE_90 |
- DRM_MODE_ROTATE_180 |
- DRM_MODE_REFLECT_X |
- DRM_MODE_REFLECT_Y);
+ DRM_MODE_ROTATE_MASK | DRM_MODE_REFLECT_MASK);
return plane;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 6/6] drm/vkms: drop "Rotation" TODO
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
` (4 preceding siblings ...)
2023-04-18 13:05 ` [PATCH v4 5/6] drm/vkms: add rotate-270 property Maíra Canal
@ 2023-04-18 13:05 ` Maíra Canal
2023-05-07 20:54 ` [PATCH v4 0/6] drm/vkms: introduce plane rotation property Melissa Wen
6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-04-18 13:05 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rodrigo Siqueira, Melissa Wen,
Haneen Mohammed, Igor Matheus Andrade Torrente, Arthur Grillo,
Ville Syrjälä
Cc: Maíra Canal, dri-devel
Now that VKMS supports all values of rotation and reflection, drop the
"Rotation" task from the TODO list.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
Documentation/gpu/vkms.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 49db221c0f52..413e6815b9bc 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -125,7 +125,7 @@ There's lots of plane features we could add support for:
- Full alpha blending on all planes.
-- Rotation, scaling.
+- Scaling.
- Additional buffer formats, especially YUV formats for video like NV12.
Low/high bpp RGB formats would also be interesting.
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 0/6] drm/vkms: introduce plane rotation property
2023-04-18 13:05 [PATCH v4 0/6] drm/vkms: introduce plane rotation property Maíra Canal
` (5 preceding siblings ...)
2023-04-18 13:05 ` [PATCH v4 6/6] drm/vkms: drop "Rotation" TODO Maíra Canal
@ 2023-05-07 20:54 ` Melissa Wen
2023-05-08 13:02 ` Maíra Canal
6 siblings, 1 reply; 11+ messages in thread
From: Melissa Wen @ 2023-05-07 20:54 UTC (permalink / raw)
To: Maíra Canal
Cc: Haneen Mohammed, Rodrigo Siqueira, dri-devel, Arthur Grillo,
Igor Matheus Andrade Torrente
On 04/18, Maíra Canal wrote:
> This patchset implements all possible rotation value in vkms. All operations
> were implemented by software by changing the way the pixels are read. The way
> the blending is performed can be depicted as:
>
> - rotate-0:
> (x) ---->
> ----------------------
> (y) | |
> | | |
> | | |
> ˇ | |
> ----------------------
>
> - rotate-90:
> <---- (y)
> ----------------------
> (x) | |
> | | |
> | | |
> ˇ | |
> ----------------------
>
> - rotate-180:
> <---- (x)
> ----------------------
> (y) | |
> ^ | |
> | | |
> | | |
> ----------------------
>
> - rotate-270:
> (y) ---->
> ----------------------
> (x) | |
> ^ | |
> | | |
> | | |
> ----------------------
>
> - reflect-x:
> <---- (x)
> ----------------------
> (y) | |
> | | |
> | | |
> ˇ | |
> ----------------------
>
> - reflect-y:
> (x) ---->
> ----------------------
> (y) | |
> ^ | |
> | | |
> | | |
> ----------------------
>
> The patchset was tested with IGT's kms_rotation_crc tests and also with some
> additional tests [1] for the reflection operations.
>
> In order to avoid code duplication, I introduced a patch that isolates the
> pixel format convertion and wraps it in a single loop.
>
> I tried to apply Ville's suggestion to avoid hand rolled coordinate
> calculation stuff. Although I couldn't apply all the code suggested by
> Ville, I was able to remove all the hardcoded code related to the x-offset.
> As VKMS' composition is performed by line, I still need to indicate the
> right pixel, which means that I still have some hardcoded code. Thanks for
> the suggestion, Ville! It really reduced the code complexity.
>
> v1 -> v2: https://lore.kernel.org/dri-devel/20230406130138.70752-1-mcanal@igalia.com/T/
>
> * Add patch to isolate pixel format conversion (Arthur Grillo).
>
> v2 -> v3: https://lore.kernel.org/dri-devel/20230414135151.75975-1-mcanal@igalia.com/T/
>
> * Use cpp to calculate pixel size instead of hard-coding (Arthur Grillo).
> * Don't use the x coordinate in the pixel_read functions (Arthur Grillo).
> * Use drm_rotate_simplify() to avoid hard-coding rotate-180 (Ville Syrjälä).
> * Use u8* to input the src_pixels instead of using u16*.
>
> v3 -> v4: https://lore.kernel.org/dri-devel/20230417121056.63917-1-mcanal@igalia.com/T/
>
> * Create a original rectangle and a rotated rectangle and use the original
> rectangle to offset the x-axis (Ville Syrjälä).
>
> [1] https://patchwork.freedesktop.org/series/116025/
Hi Maíra,
Thanks for adding rotation properties to VKMS!
Overall, LGTM and this series is:
Reviewed-by: Melissa Wen <mwen@igalia.com>
As you already applied the first patch, not a big issue, but I see new
function documentation is missing. Could you send a follow-up patch
documenting "vkms_compose_row()"? Also, would be good to apply the same
improvement for the remaining conversion functions too.
Thanks,
Melissa
>
> Best Regards,
> - Maíra Canal
>
> Maíra Canal (6):
> drm/vkms: isolate pixel conversion functionality
> drm/vkms: add rotate-0 and reflect-x property
> drm/vkms: add reflect-y and rotate-180 property
> drm/vkms: add rotate-90 property
> drm/vkms: add rotate-270 property
> drm/vkms: drop "Rotation" TODO
>
> Documentation/gpu/vkms.rst | 2 +-
> drivers/gpu/drm/vkms/vkms_composer.c | 38 ++++++--
> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
> drivers/gpu/drm/vkms/vkms_formats.c | 139 +++++++++++++--------------
> drivers/gpu/drm/vkms/vkms_formats.h | 2 +-
> drivers/gpu/drm/vkms/vkms_plane.c | 16 ++-
> 6 files changed, 117 insertions(+), 86 deletions(-)
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 0/6] drm/vkms: introduce plane rotation property
2023-05-07 20:54 ` [PATCH v4 0/6] drm/vkms: introduce plane rotation property Melissa Wen
@ 2023-05-08 13:02 ` Maíra Canal
0 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2023-05-08 13:02 UTC (permalink / raw)
To: Melissa Wen
Cc: Haneen Mohammed, Rodrigo Siqueira, dri-devel, Arthur Grillo,
Igor Matheus Andrade Torrente
On 5/7/23 17:54, Melissa Wen wrote:
> On 04/18, Maíra Canal wrote:
>> This patchset implements all possible rotation value in vkms. All operations
>> were implemented by software by changing the way the pixels are read. The way
>> the blending is performed can be depicted as:
>>
>> - rotate-0:
>> (x) ---->
>> ----------------------
>> (y) | |
>> | | |
>> | | |
>> ˇ | |
>> ----------------------
>>
>> - rotate-90:
>> <---- (y)
>> ----------------------
>> (x) | |
>> | | |
>> | | |
>> ˇ | |
>> ----------------------
>>
>> - rotate-180:
>> <---- (x)
>> ----------------------
>> (y) | |
>> ^ | |
>> | | |
>> | | |
>> ----------------------
>>
>> - rotate-270:
>> (y) ---->
>> ----------------------
>> (x) | |
>> ^ | |
>> | | |
>> | | |
>> ----------------------
>>
>> - reflect-x:
>> <---- (x)
>> ----------------------
>> (y) | |
>> | | |
>> | | |
>> ˇ | |
>> ----------------------
>>
>> - reflect-y:
>> (x) ---->
>> ----------------------
>> (y) | |
>> ^ | |
>> | | |
>> | | |
>> ----------------------
>>
>> The patchset was tested with IGT's kms_rotation_crc tests and also with some
>> additional tests [1] for the reflection operations.
>>
>> In order to avoid code duplication, I introduced a patch that isolates the
>> pixel format convertion and wraps it in a single loop.
>>
>> I tried to apply Ville's suggestion to avoid hand rolled coordinate
>> calculation stuff. Although I couldn't apply all the code suggested by
>> Ville, I was able to remove all the hardcoded code related to the x-offset.
>> As VKMS' composition is performed by line, I still need to indicate the
>> right pixel, which means that I still have some hardcoded code. Thanks for
>> the suggestion, Ville! It really reduced the code complexity.
>> v1 -> v2: https://lore.kernel.org/dri-devel/20230406130138.70752-1-mcanal@igalia.com/T/
>>
>> * Add patch to isolate pixel format conversion (Arthur Grillo).
>>
>> v2 -> v3: https://lore.kernel.org/dri-devel/20230414135151.75975-1-mcanal@igalia.com/T/
>>
>> * Use cpp to calculate pixel size instead of hard-coding (Arthur Grillo).
>> * Don't use the x coordinate in the pixel_read functions (Arthur Grillo).
>> * Use drm_rotate_simplify() to avoid hard-coding rotate-180 (Ville Syrjälä).
>> * Use u8* to input the src_pixels instead of using u16*.
>>
>> v3 -> v4: https://lore.kernel.org/dri-devel/20230417121056.63917-1-mcanal@igalia.com/T/
>>
>> * Create a original rectangle and a rotated rectangle and use the original
>> rectangle to offset the x-axis (Ville Syrjälä).
>>
>> [1] https://patchwork.freedesktop.org/series/116025/
>
> Hi Maíra,
>
> Thanks for adding rotation properties to VKMS!
> Overall, LGTM and this series is:
>
> Reviewed-by: Melissa Wen <mwen@igalia.com>
Thanks for the review, Melissa! Applied to drm/drm-misc (drm-misc-next).
>
> As you already applied the first patch, not a big issue, but I see new
> function documentation is missing. Could you send a follow-up patch
> documenting "vkms_compose_row()"? Also, would be good to apply the same
> improvement for the remaining conversion functions too.
I'll try to work on some follow-up patches ASAP.
Best Regards,
- Maíra Canal
>
> Thanks,
>
> Melissa
>
>>
>> Best Regards,
>> - Maíra Canal
>>
>> Maíra Canal (6):
>> drm/vkms: isolate pixel conversion functionality
>> drm/vkms: add rotate-0 and reflect-x property
>> drm/vkms: add reflect-y and rotate-180 property
>> drm/vkms: add rotate-90 property
>> drm/vkms: add rotate-270 property
>> drm/vkms: drop "Rotation" TODO
>>
>> Documentation/gpu/vkms.rst | 2 +-
>> drivers/gpu/drm/vkms/vkms_composer.c | 38 ++++++--
>> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
>> drivers/gpu/drm/vkms/vkms_formats.c | 139 +++++++++++++--------------
>> drivers/gpu/drm/vkms/vkms_formats.h | 2 +-
>> drivers/gpu/drm/vkms/vkms_plane.c | 16 ++-
>> 6 files changed, 117 insertions(+), 86 deletions(-)
>>
>> --
>> 2.39.2
>>
^ permalink raw reply [flat|nested] 11+ messages in thread