From: Melissa Wen <melissa.srw@gmail.com>
To: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>,
Haneen Mohammed <hamohammed.sa@gmail.com>,
Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
kernel-usp@googlegroups.com
Subject: [PATCH] drm/vkms: add alpha-premultiplied color blending
Date: Wed, 19 Aug 2020 17:53:36 -0300 [thread overview]
Message-ID: <20200819205336.fce24lioz34vbcd2@smtp.gmail.com> (raw)
The current VKMS blend function ignores alpha channel and just overwrites
vaddr_src with vaddr_dst. This XRGB approach triggers a warning when
running the kms_cursor_crc/cursor-alpha-transparent test case. In IGT
tests, cairo_format_argb32 uses premultiplied alpha (according to
documentation), so this patch considers premultiplied alpha colors to
compose vaddr_src with vaddr_dst.
This change removes the following cursor-alpha-transparent warning:
Suspicious CRC: All values are 0.
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 43 +++++++++++++++++++++-------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 4f3b07a32b60..6aac962d3e2e 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -32,8 +32,6 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
src_offset = composer->offset
+ (i * composer->pitch)
+ (j * composer->cpp);
- /* XRGB format ignores Alpha channel */
- bitmap_clear(vaddr_out + src_offset, 24, 8);
crc = crc32_le(crc, vaddr_out + src_offset,
sizeof(u32));
}
@@ -42,6 +40,32 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
return crc;
}
+u8 blend_channel(u8 c_src, u8 c_dst, u8 a_src)
+{
+ u32 pre_blend;
+ u8 new_color;
+
+ /* Premultiplied alpha blending - IGT + cairo context */
+ pre_blend = (c_src * 255 + c_dst * (255 - a_src));
+
+ /* Faster div by 255 */
+ new_color = ((pre_blend + ((pre_blend + 257) >> 8)) >> 8);
+
+ return new_color;
+}
+
+void alpha_blending(u8 *argb_src, u8 *argb_dst)
+{
+ u8 a_src;
+
+ a_src = argb_src[3];
+ argb_dst[0] = blend_channel(argb_src[0], argb_dst[0], a_src);
+ argb_dst[1] = blend_channel(argb_src[1], argb_dst[1], a_src);
+ argb_dst[2] = blend_channel(argb_src[2], argb_dst[2], a_src);
+ /* Opaque primary */
+ argb_dst[3] = 0xFF;
+}
+
/**
* blend - blend value at vaddr_src with value at vaddr_dst
* @vaddr_dst: destination address
@@ -50,12 +74,9 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
* @src_composer: source framebuffer's metadata
*
* Blend value at vaddr_src with value at vaddr_dst.
- * Currently, this function write value of vaddr_src on value
- * at vaddr_dst using buffer's metadata to locate the new values
- * from vaddr_src and their destination at vaddr_dst.
- *
- * TODO: Use the alpha value to blend vaddr_src with vaddr_dst
- * instead of overwriting it.
+ * Currently, this function considers premultiplied alpha for blending, as used
+ * by Cairo. It uses buffer's metadata to locate the new composite values at
+ * vaddr_dst.
*/
static void blend(void *vaddr_dst, void *vaddr_src,
struct vkms_composer *dest_composer,
@@ -63,6 +84,7 @@ static void blend(void *vaddr_dst, void *vaddr_src,
{
int i, j, j_dst, i_dst;
int offset_src, offset_dst;
+ u8 *p_dst, *p_src;
int x_src = src_composer->src.x1 >> 16;
int y_src = src_composer->src.y1 >> 16;
@@ -84,8 +106,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
+ (i * src_composer->pitch)
+ (j * src_composer->cpp);
- memcpy(vaddr_dst + offset_dst,
- vaddr_src + offset_src, sizeof(u32));
+ p_src = (u8 *)(vaddr_src + offset_src);
+ p_dst = (u8 *)(vaddr_dst + offset_dst);
+ alpha_blending(p_src, p_dst);
}
i_dst++;
}
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Melissa Wen <melissa.srw@gmail.com>
To: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>,
Haneen Mohammed <hamohammed.sa@gmail.com>,
Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
kernel-usp@googlegroups.com, twoerner@gmail.com
Subject: [PATCH] drm/vkms: add alpha-premultiplied color blending
Date: Wed, 19 Aug 2020 17:53:36 -0300 [thread overview]
Message-ID: <20200819205336.fce24lioz34vbcd2@smtp.gmail.com> (raw)
The current VKMS blend function ignores alpha channel and just overwrites
vaddr_src with vaddr_dst. This XRGB approach triggers a warning when
running the kms_cursor_crc/cursor-alpha-transparent test case. In IGT
tests, cairo_format_argb32 uses premultiplied alpha (according to
documentation), so this patch considers premultiplied alpha colors to
compose vaddr_src with vaddr_dst.
This change removes the following cursor-alpha-transparent warning:
Suspicious CRC: All values are 0.
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 43 +++++++++++++++++++++-------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 4f3b07a32b60..6aac962d3e2e 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -32,8 +32,6 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
src_offset = composer->offset
+ (i * composer->pitch)
+ (j * composer->cpp);
- /* XRGB format ignores Alpha channel */
- bitmap_clear(vaddr_out + src_offset, 24, 8);
crc = crc32_le(crc, vaddr_out + src_offset,
sizeof(u32));
}
@@ -42,6 +40,32 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
return crc;
}
+u8 blend_channel(u8 c_src, u8 c_dst, u8 a_src)
+{
+ u32 pre_blend;
+ u8 new_color;
+
+ /* Premultiplied alpha blending - IGT + cairo context */
+ pre_blend = (c_src * 255 + c_dst * (255 - a_src));
+
+ /* Faster div by 255 */
+ new_color = ((pre_blend + ((pre_blend + 257) >> 8)) >> 8);
+
+ return new_color;
+}
+
+void alpha_blending(u8 *argb_src, u8 *argb_dst)
+{
+ u8 a_src;
+
+ a_src = argb_src[3];
+ argb_dst[0] = blend_channel(argb_src[0], argb_dst[0], a_src);
+ argb_dst[1] = blend_channel(argb_src[1], argb_dst[1], a_src);
+ argb_dst[2] = blend_channel(argb_src[2], argb_dst[2], a_src);
+ /* Opaque primary */
+ argb_dst[3] = 0xFF;
+}
+
/**
* blend - blend value at vaddr_src with value at vaddr_dst
* @vaddr_dst: destination address
@@ -50,12 +74,9 @@ static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
* @src_composer: source framebuffer's metadata
*
* Blend value at vaddr_src with value at vaddr_dst.
- * Currently, this function write value of vaddr_src on value
- * at vaddr_dst using buffer's metadata to locate the new values
- * from vaddr_src and their destination at vaddr_dst.
- *
- * TODO: Use the alpha value to blend vaddr_src with vaddr_dst
- * instead of overwriting it.
+ * Currently, this function considers premultiplied alpha for blending, as used
+ * by Cairo. It uses buffer's metadata to locate the new composite values at
+ * vaddr_dst.
*/
static void blend(void *vaddr_dst, void *vaddr_src,
struct vkms_composer *dest_composer,
@@ -63,6 +84,7 @@ static void blend(void *vaddr_dst, void *vaddr_src,
{
int i, j, j_dst, i_dst;
int offset_src, offset_dst;
+ u8 *p_dst, *p_src;
int x_src = src_composer->src.x1 >> 16;
int y_src = src_composer->src.y1 >> 16;
@@ -84,8 +106,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
+ (i * src_composer->pitch)
+ (j * src_composer->cpp);
- memcpy(vaddr_dst + offset_dst,
- vaddr_src + offset_src, sizeof(u32));
+ p_src = (u8 *)(vaddr_src + offset_src);
+ p_dst = (u8 *)(vaddr_dst + offset_dst);
+ alpha_blending(p_src, p_dst);
}
i_dst++;
}
--
2.28.0
next reply other threads:[~2020-08-19 20:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-19 20:53 Melissa Wen [this message]
2020-08-19 20:53 ` [PATCH] drm/vkms: add alpha-premultiplied color blending Melissa Wen
2020-08-20 7:27 ` Pekka Paalanen
2020-08-20 7:27 ` Pekka Paalanen
2020-08-25 3:04 ` Rodrigo Siqueira
2020-08-25 3:04 ` Rodrigo Siqueira
2020-08-25 11:47 ` Melissa Wen
2020-08-25 11:47 ` Melissa Wen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200819205336.fce24lioz34vbcd2@smtp.gmail.com \
--to=melissa.srw@gmail.com \
--cc=Rodrigo.Siqueira@amd.com \
--cc=airlied@linux.ie \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hamohammed.sa@gmail.com \
--cc=kernel-usp@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rodrigosiqueiramelo@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.