I wrote this patch for TODO list in vkms documentation.
Use alpha value to blend source value and destination value Instead of
just overwrite with source value.
Cc: Rodrigo Siqueira <[email protected]>
Cc: Haneen Mohammed <[email protected]>
Signed-off-by: Sidong Yang <[email protected]>
---
drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 4f3b07a32b60..e3230e2a99af 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -77,6 +77,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
+ u8 *src, *dst;
+ u32 alpha, inv_alpha;
+
offset_dst = dest_composer->offset
+ (i_dst * dest_composer->pitch)
+ (j_dst++ * dest_composer->cpp);
@@ -84,8 +87,15 @@ 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));
+ src = vaddr_src + offset_src;
+ dst = vaddr_dst + offset_dst;
+ alpha = src[3] + 1;
+ inv_alpha = 256 - src[3];
+ dst[0] = (alpha * src[0] + inv_alpha * dst[0]) >> 8;
+ dst[1] = (alpha * src[1] + inv_alpha * dst[1]) >> 8;
+ dst[2] = (alpha * src[2] + inv_alpha * dst[2]) >> 8;
+ dst[3] = 0xff;
+
}
i_dst++;
}
--
2.17.1
Hi Sidong,
Thanks a lot for your patch and effort to improve VKMS.
On 08/18, Sidong Yang wrote:
> I wrote this patch for TODO list in vkms documentation.
>
> Use alpha value to blend source value and destination value Instead of
> just overwrite with source value.
>
> Cc: Rodrigo Siqueira <[email protected]>
> Cc: Haneen Mohammed <[email protected]>
>
> Signed-off-by: Sidong Yang <[email protected]>
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 4f3b07a32b60..e3230e2a99af 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -77,6 +77,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
>
> for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
> for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
> + u8 *src, *dst;
> + u32 alpha, inv_alpha;
> +
> offset_dst = dest_composer->offset
> + (i_dst * dest_composer->pitch)
> + (j_dst++ * dest_composer->cpp);
> @@ -84,8 +87,15 @@ 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));
> + src = vaddr_src + offset_src;
> + dst = vaddr_dst + offset_dst;
> + alpha = src[3] + 1;
> + inv_alpha = 256 - src[3];
> + dst[0] = (alpha * src[0] + inv_alpha * dst[0]) >> 8;
> + dst[1] = (alpha * src[1] + inv_alpha * dst[1]) >> 8;
> + dst[2] = (alpha * src[2] + inv_alpha * dst[2]) >> 8;
Did you test your change with IGT? Maybe I missed something but looks
like that you're applying the alpha value but the value that we get is
already pre-multiplied.
Btw, It looks like that you and Melissa are working in the same feature,
maybe you two could try to sync for avoiding overlapping.
Finally, do you have plans to send your fix for
vkms_get_vblank_timestamp() function? That patch was really good and
removes a lot of warning generated during the IGT test.
Best Regards
> + dst[3] = 0xff;
> +
> }
> i_dst++;
> }
> --
> 2.17.1
>
--
Rodrigo Siqueira
https://siqueira.tech
On Mon, Aug 24, 2020 at 11:15:01PM -0400, Rodrigo Siqueira wrote:
> Hi Sidong,
>
> Thanks a lot for your patch and effort to improve VKMS.
>
> On 08/18, Sidong Yang wrote:
> > I wrote this patch for TODO list in vkms documentation.
> >
> > Use alpha value to blend source value and destination value Instead of
> > just overwrite with source value.
> >
> > Cc: Rodrigo Siqueira <[email protected]>
> > Cc: Haneen Mohammed <[email protected]>
> >
> > Signed-off-by: Sidong Yang <[email protected]>
> > ---
> > drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> > index 4f3b07a32b60..e3230e2a99af 100644
> > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > @@ -77,6 +77,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
> >
> > for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
> > for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
> > + u8 *src, *dst;
> > + u32 alpha, inv_alpha;
> > +
> > offset_dst = dest_composer->offset
> > + (i_dst * dest_composer->pitch)
> > + (j_dst++ * dest_composer->cpp);
> > @@ -84,8 +87,15 @@ 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));
> > + src = vaddr_src + offset_src;
> > + dst = vaddr_dst + offset_dst;
> > + alpha = src[3] + 1;
> > + inv_alpha = 256 - src[3];
> > + dst[0] = (alpha * src[0] + inv_alpha * dst[0]) >> 8;
> > + dst[1] = (alpha * src[1] + inv_alpha * dst[1]) >> 8;
> > + dst[2] = (alpha * src[2] + inv_alpha * dst[2]) >> 8;
>
Hi, Rodrigo!
> Did you test your change with IGT? Maybe I missed something but looks
> like that you're applying the alpha value but the value that we get is
> already pre-multiplied.
>
> Btw, It looks like that you and Melissa are working in the same feature,
> maybe you two could try to sync for avoiding overlapping.
Thanks for review.
Yes, this patch should be dropped and I should watch Melissa's patch.
>
> Finally, do you have plans to send your fix for
> vkms_get_vblank_timestamp() function? That patch was really good and
> removes a lot of warning generated during the IGT test.
Okay, I'll work for improve vkms_get_vblank_timestamp().
Thank you so much.
Sincerely,
-Sidong
>
> Best Regards
>
> > + dst[3] = 0xff;
> > +
> > }
> > i_dst++;
> > }
> > --
> > 2.17.1
> >
>
> --
> Rodrigo Siqueira
> https://siqueira.tech