Re: [PATCH xserver 2/2] glamor: Avoid software fallback for planemasked ZPixmap GetImage
On Mon, 2017-03-20 at 10:42 -0700, Eric Anholt wrote:
> > Adam Jackson writes:
> > @@ -128,6 +128,16 @@ glamor_get_image_gl(DrawablePtr drawable, int x, int
> > y, int w, int h,
> > drawable->x + off_x, drawable->y + off_y,
> > -x, -y,
> > (uint8_t *) d, byte_stride);
> > +
> > +if (!glamor_pm_is_solid(drawable->depth, plane_mask)) {
> > +FbStip pm = fbReplicatePixel(plane_mask, drawable->bitsPerPixel);
> > +FbStip *dst = (void *)d;
> > +uint32_t dstStride = byte_stride /= sizeof(FbStip);
>
> I don't think you want the second '=' here. Harmless, but looked weird.
>
> Other than that, r-b.
Good eye. Pretty sure that was a leftover from an earlier iteration of
the patch. Fixed that up and merged:
remote: I: patch #112622 updated using rev
4aa35c46dab72bc945981f6fd29e494133bc2b0a.
remote: E: failed to find patch for rev
1ad230682338a9d2fc6eca6966a5bebb007df32c.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
368f60d..1ad2306 master -> master
- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/2] glamor: Avoid software fallback for planemasked ZPixmap GetImage
Adam Jackson writes:
> Same trick as in fb: just do a normal GetImage and deal with the
> planemask on the CPU if you have to. Since the software fallback hit for
> glamor is pretty brutal, this is a much more impressive win for glamor
> than it was for fb:
>
> 11100.0 87700.0 (7.901) (copy 0x) ShmGetImage 10x10 square
>9840.0 47800.0 (4.858) (copy 0x) ShmGetImage 100x100 square
>1550.0 4240.0 (2.735) (copy 0x) ShmGetImage 500x500 square
>9450.0 78900.0 (8.349) (0x) GetImage 10x10 square
>6910.0 30900.0 (4.472) (0x) GetImage 100x100 square
> 431.0 2020.0 (4.687) (0x) GetImage 500x500 square
>
> Measured with Xephyr -glamor on Skylake GT3e.
>
> Signed-off-by: Adam Jackson
> ---
> glamor/glamor_image.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c
> index 3158749..7e17961 100644
> --- a/glamor/glamor_image.c
> +++ b/glamor/glamor_image.c
> @@ -116,7 +116,7 @@ glamor_get_image_gl(DrawablePtr drawable, int x, int y,
> int w, int h,
> if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
> goto bail;
>
> -if (format != ZPixmap || !glamor_pm_is_solid(drawable->depth,
> plane_mask))
> +if (format != ZPixmap)
> goto bail;
>
> glamor_get_drawable_deltas(drawable, pixmap, &off_x, &off_y);
> @@ -128,6 +128,16 @@ glamor_get_image_gl(DrawablePtr drawable, int x, int y,
> int w, int h,
>drawable->x + off_x, drawable->y + off_y,
>-x, -y,
>(uint8_t *) d, byte_stride);
> +
> +if (!glamor_pm_is_solid(drawable->depth, plane_mask)) {
> +FbStip pm = fbReplicatePixel(plane_mask, drawable->bitsPerPixel);
> +FbStip *dst = (void *)d;
> +uint32_t dstStride = byte_stride /= sizeof(FbStip);
I don't think you want the second '=' here. Harmless, but looked weird.
Other than that, r-b.
> +
> +for (int i = 0; i < dstStride * h; i++)
> +dst[i] &= pm;
> +}
> +
> return TRUE;
> bail:
> return FALSE;
> --
> 2.9.3
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/2] glamor: Avoid software fallback for planemasked ZPixmap GetImage
On Wed, 2016-09-28 at 13:05 -0400, Adam Jackson wrote: > Same trick as in fb: just do a normal GetImage and deal with the > planemask on the CPU if you have to. Since the software fallback hit for > glamor is pretty brutal, this is a much more impressive win for glamor > than it was for fb: > > 11100.0 87700.0 (7.901) (copy 0x) ShmGetImage 10x10 square > 9840.0 47800.0 (4.858) (copy 0x) ShmGetImage 100x100 square > 1550.0 4240.0 (2.735) (copy 0x) ShmGetImage 500x500 square > 9450.0 78900.0 (8.349) (0x) GetImage 10x10 square > 6910.0 30900.0 (4.472) (0x) GetImage 100x100 square > 431.0 2020.0 (4.687) (0x) GetImage 500x500 square > > Measured with Xephyr -glamor on Skylake GT3e. The objection to this series was that they got 24bpp wrong. Since that's no longer a problem, anyone care to review these? - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 2/2] glamor: Avoid software fallback for planemasked ZPixmap GetImage
Same trick as in fb: just do a normal GetImage and deal with the
planemask on the CPU if you have to. Since the software fallback hit for
glamor is pretty brutal, this is a much more impressive win for glamor
than it was for fb:
11100.0 87700.0 (7.901) (copy 0x) ShmGetImage 10x10 square
9840.0 47800.0 (4.858) (copy 0x) ShmGetImage 100x100 square
1550.0 4240.0 (2.735) (copy 0x) ShmGetImage 500x500 square
9450.0 78900.0 (8.349) (0x) GetImage 10x10 square
6910.0 30900.0 (4.472) (0x) GetImage 100x100 square
431.0 2020.0 (4.687) (0x) GetImage 500x500 square
Measured with Xephyr -glamor on Skylake GT3e.
Signed-off-by: Adam Jackson
---
glamor/glamor_image.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c
index 3158749..7e17961 100644
--- a/glamor/glamor_image.c
+++ b/glamor/glamor_image.c
@@ -116,7 +116,7 @@ glamor_get_image_gl(DrawablePtr drawable, int x, int y, int
w, int h,
if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
goto bail;
-if (format != ZPixmap || !glamor_pm_is_solid(drawable->depth, plane_mask))
+if (format != ZPixmap)
goto bail;
glamor_get_drawable_deltas(drawable, pixmap, &off_x, &off_y);
@@ -128,6 +128,16 @@ glamor_get_image_gl(DrawablePtr drawable, int x, int y,
int w, int h,
drawable->x + off_x, drawable->y + off_y,
-x, -y,
(uint8_t *) d, byte_stride);
+
+if (!glamor_pm_is_solid(drawable->depth, plane_mask)) {
+FbStip pm = fbReplicatePixel(plane_mask, drawable->bitsPerPixel);
+FbStip *dst = (void *)d;
+uint32_t dstStride = byte_stride /= sizeof(FbStip);
+
+for (int i = 0; i < dstStride * h; i++)
+dst[i] &= pm;
+}
+
return TRUE;
bail:
return FALSE;
--
2.9.3
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
