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 <rodrigosiqueiramelo@xxxxxxxxx>
> > Cc: Haneen Mohammed <hamohammed.sa@xxxxxxxxx>
> >
> > Signed-off-by: Sidong Yang <realwakka@xxxxxxxxx>
> > ---
> > 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
