Re: [PATCH] drm/vkms: Use alpha value to blend values.
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
> > Cc: Haneen Mohammed
> >
> > Signed-off-by: Sidong Yang
> > ---
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: Use alpha value to blend values.
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
> Cc: Haneen Mohammed
>
> Signed-off-by: Sidong Yang
> ---
> 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
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vkms: Use alpha value to blend values.
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
Cc: Haneen Mohammed
Signed-off-by: Sidong Yang
---
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: Use alpha value to blend values.
On Wed, Sep 04, 2019 at 08:27:07AM +0100, Sidong Yang wrote:
> On Mon, Sep 02, 2019 at 03:28:58PM +0300, Ville Syrjälä wrote:
> > On Sat, Aug 31, 2019 at 06:25:46PM +0100, Sidong Yang wrote:
> > > Use alpha value to blend source value and destination value Instead of
> > > just overwrite with source value.
> > >
> > > Signed-off-by: Sidong Yang
> > > ---
> > > drivers/gpu/drm/vkms/vkms_composer.c | 13 +++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> > > b/drivers/gpu/drm/vkms/vkms_composer.c
> > > index d5585695c64d..b776185e5cb5 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > > @@ -75,6 +75,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
> > > int y_limit = y_src + h_dst;
> > > int x_limit = x_src + w_dst;
> > >
> > > + u8 *src, *dst;
> > > + u32 alpha, inv_alpha;
> >
> > These could all live in a tighter scope.
>
> Hi, Ville.
>
> Thank you for reviewing my patch.
> I think that's good idea and I'll do that in next version.
> I found some patch in mailing list that is similar with this patch.
> So should I drop this patch and find other thing?
Probably best if you discuss that with whoever sent that other patch.
>
> Sidong.
>
> >
> > Apart from that lgtm
> > Reviewed-by: Ville Syrjälä
> >
> > > +
> > > for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
> > > for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
> > > offset_dst = dest_composer->offset
> > > @@ -84,8 +87,14 @@ 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.20.1
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: Use alpha value to blend values.
On Mon, Sep 02, 2019 at 03:28:58PM +0300, Ville Syrjälä wrote:
> On Sat, Aug 31, 2019 at 06:25:46PM +0100, Sidong Yang wrote:
> > Use alpha value to blend source value and destination value Instead of
> > just overwrite with source value.
> >
> > Signed-off-by: Sidong Yang
> > ---
> > drivers/gpu/drm/vkms/vkms_composer.c | 13 +++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> > b/drivers/gpu/drm/vkms/vkms_composer.c
> > index d5585695c64d..b776185e5cb5 100644
> > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > @@ -75,6 +75,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
> > int y_limit = y_src + h_dst;
> > int x_limit = x_src + w_dst;
> >
> > + u8 *src, *dst;
> > + u32 alpha, inv_alpha;
>
> These could all live in a tighter scope.
Hi, Ville.
Thank you for reviewing my patch.
I think that's good idea and I'll do that in next version.
I found some patch in mailing list that is similar with this patch.
So should I drop this patch and find other thing?
Sidong.
>
> Apart from that lgtm
> Reviewed-by: Ville Syrjälä
>
> > +
> > for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
> > for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
> > offset_dst = dest_composer->offset
> > @@ -84,8 +87,14 @@ 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.20.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH] drm/vkms: Use alpha value to blend values.
On Sat, Aug 31, 2019 at 06:25:46PM +0100, Sidong Yang wrote:
> Use alpha value to blend source value and destination value Instead of
> just overwrite with source value.
>
> Signed-off-by: Sidong Yang
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index d5585695c64d..b776185e5cb5 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -75,6 +75,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
> int y_limit = y_src + h_dst;
> int x_limit = x_src + w_dst;
>
> + u8 *src, *dst;
> + u32 alpha, inv_alpha;
These could all live in a tighter scope.
Apart from that lgtm
Reviewed-by: Ville Syrjälä
> +
> for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
> for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
> offset_dst = dest_composer->offset
> @@ -84,8 +87,14 @@ 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.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vkms: Use alpha value to blend values.
Use alpha value to blend source value and destination value Instead of
just overwrite with source value.
Signed-off-by: Sidong Yang
---
drivers/gpu/drm/vkms/vkms_composer.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index d5585695c64d..b776185e5cb5 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -75,6 +75,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
int y_limit = y_src + h_dst;
int x_limit = x_src + w_dst;
+ u8 *src, *dst;
+ u32 alpha, inv_alpha;
+
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
offset_dst = dest_composer->offset
@@ -84,8 +87,14 @@ 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.20.1
[PATCH] drm/vkms: Use alpha value to blend values.
Use alpha value to blend source value and destination value Instead of
just overwrite with source value.
Signed-off-by: Sidong Yang
---
drivers/gpu/drm/vkms/vkms_composer.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index d5585695c64d..b776185e5cb5 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -75,6 +75,9 @@ static void blend(void *vaddr_dst, void *vaddr_src,
int y_limit = y_src + h_dst;
int x_limit = x_src + w_dst;
+ u8 *src, *dst;
+ u32 alpha, inv_alpha;
+
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
offset_dst = dest_composer->offset
@@ -84,8 +87,14 @@ 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.20.1
