Re: [PATCH xserver 1/2] fb: Handle ZPixmap planemask in GetImage the other way around

2017-03-20 Thread Eric Anholt
Adam Jackson  writes:

> Formerly we'd zero the image data and then pull out a plane at a time.
> It's faster to apply the planemask after the fact, since that turns the
> GetImage into a memcpy:
>
>   10.0  101000.0 (1.010) (copy 0x) ShmGetImage 10x10 square
>42400.0   59400.0 (1.401) (copy 0x) ShmGetImage 100x100 square
> 3040.05280.0 (1.737) (copy 0x) ShmGetImage 500x500 square
>96100.0   95200.0 (0.991) (0x) GetImage 10x10 square
>29600.0   36800.0 (1.243) (0x) GetImage 100x100 square
> 1850.02620.0 (1.416) (0x) GetImage 500x500 square
>
> Measured with Xvfb at depth 24 on Skylake i7-6560U.
>
> Signed-off-by: Adam Jackson 

It's a bit surprising that trading a memset and a pass of dword access
for a memcpy and a pass of dword access is a win, but then the MergeRop
isn't just a single & operation.

Reviewed-by: Eric Anholt 


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 1/2] fb: Handle ZPixmap planemask in GetImage the other way around

2016-09-28 Thread Keith Packard
Adam Jackson  writes:

> I have a plan to deal with 24bpp, very harshly indeed.

You know how much time I spent writing that code. You're deleting some
of the "finest" bitblt in the server.

> You do appear to be correct though, xfree86 still has paths by which
> you can ask for a 24bpp pixmap/wire format, so I can't assume we'll
> take the fb24_32GetImage path instead. I'll defer this patch until
> after I've removed 24bpp entirely then.

Sounds like a fine plan :-)

-- 
-keith


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 1/2] fb: Handle ZPixmap planemask in GetImage the other way around

2016-09-28 Thread Adam Jackson
On Wed, 2016-09-28 at 10:19 -0700, Keith Packard wrote:

> You'll need to deal with 24bpp separately...

I have a plan to deal with 24bpp, very harshly indeed.

You do appear to be correct though, xfree86 still has paths by which
you can ask for a 24bpp pixmap/wire format, so I can't assume we'll
take the fb24_32GetImage path instead. I'll defer this patch until
after I've removed 24bpp entirely then.

- 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 1/2] fb: Handle ZPixmap planemask in GetImage the other way around

2016-09-28 Thread Keith Packard
Adam Jackson  writes:

> diff --git a/fb/fbimage.c b/fb/fbimage.c
> index 59daa21..8f5f3dc 100644
> --- a/fb/fbimage.c
> +++ b/fb/fbimage.c
> @@ -250,13 +250,16 @@ fbGetImage(DrawablePtr pDrawable,
>  
>  pm = fbReplicatePixel(planeMask, srcBpp);
>  dstStride = PixmapBytePad(w, pDrawable->depth);
> -if (pm != FB_ALLONES)
> -memset(d, 0, dstStride * h);
>  dstStride /= sizeof(FbStip);
>  fbBltStip((FbStip *) (src + (y + srcYoff) * srcStride),
>FbBitsStrideToStipStride(srcStride),
>(x + srcXoff) * srcBpp,
> -  dst, dstStride, 0, w * srcBpp, h, GXcopy, pm, srcBpp);
> +  dst, dstStride, 0, w * srcBpp, h, GXcopy, FB_ALLONES, 
> srcBpp);
> +
> +if (pm != FB_ALLONES) {
> +for (int i = 0; i < dstStride * h; i++)
> +dst[i] &= pm;
> +}

You'll need to deal with 24bpp separately...

-- 
-keith


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

[PATCH xserver 1/2] fb: Handle ZPixmap planemask in GetImage the other way around

2016-09-28 Thread Adam Jackson
Formerly we'd zero the image data and then pull out a plane at a time.
It's faster to apply the planemask after the fact, since that turns the
GetImage into a memcpy:

  10.0  101000.0 (1.010) (copy 0x) ShmGetImage 10x10 square
   42400.0   59400.0 (1.401) (copy 0x) ShmGetImage 100x100 square
3040.05280.0 (1.737) (copy 0x) ShmGetImage 500x500 square
   96100.0   95200.0 (0.991) (0x) GetImage 10x10 square
   29600.0   36800.0 (1.243) (0x) GetImage 100x100 square
1850.02620.0 (1.416) (0x) GetImage 500x500 square

Measured with Xvfb at depth 24 on Skylake i7-6560U.

Signed-off-by: Adam Jackson 
---
 fb/fbimage.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fb/fbimage.c b/fb/fbimage.c
index 59daa21..8f5f3dc 100644
--- a/fb/fbimage.c
+++ b/fb/fbimage.c
@@ -250,13 +250,16 @@ fbGetImage(DrawablePtr pDrawable,
 
 pm = fbReplicatePixel(planeMask, srcBpp);
 dstStride = PixmapBytePad(w, pDrawable->depth);
-if (pm != FB_ALLONES)
-memset(d, 0, dstStride * h);
 dstStride /= sizeof(FbStip);
 fbBltStip((FbStip *) (src + (y + srcYoff) * srcStride),
   FbBitsStrideToStipStride(srcStride),
   (x + srcXoff) * srcBpp,
-  dst, dstStride, 0, w * srcBpp, h, GXcopy, pm, srcBpp);
+  dst, dstStride, 0, w * srcBpp, h, GXcopy, FB_ALLONES, 
srcBpp);
+
+if (pm != FB_ALLONES) {
+for (int i = 0; i < dstStride * h; i++)
+dst[i] &= pm;
+}
 }
 else {
 dstStride = BitmapBytePad(w) / sizeof(FbStip);
-- 
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