Re: [PATCH] [RFC] glamor: implement write-only prepares

[Eric Anholt]

Dave Airlie  writes:

> From: Dave Airlie 
>
> For some putimages we know we won't ever care about the data we readback,
> we are going to trash it with the putimage contents. So implement
> GLAMOR_ACCESS_WO mode.
>
> This will avoid doing the readbacks. Use it for putimages that are copy
> with a solid planemask.
>
> inspired by Ilia and xlock -mode wator which does loads of XYBitmap
> putimages.
>
> Signed-off-by: Dave Airlie 
> ---
>  glamor/glamor_image.c   | 6 +-
>  glamor/glamor_prepare.c | 9 +
>  glamor/glamor_priv.h| 1 +
>  3 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c
> index 3158749..87fdcf6 100644
> --- a/glamor/glamor_image.c
> +++ b/glamor/glamor_image.c
> @@ -88,7 +88,11 @@ static void
>  glamor_put_image_bail(DrawablePtr drawable, GCPtr gc, int depth, int x, int 
> y,
>int w, int h, int leftPad, int format, char *bits)
>  {
> -if (glamor_prepare_access_box(drawable, GLAMOR_ACCESS_RW, x, y, w, h))
> +int access = GLAMOR_ACCESS_RW;
> +
> +if (gc->alu == GXcopy && glamor_pm_is_solid(gc->depth, gc->planemask))
> +access = GLAMOR_ACCESS_WO;
> +if (glamor_prepare_access_box(drawable, access, x, y, w, h))
>  fbPutImage(drawable, gc, depth, x, y, w, h, leftPad, format, bits);
>  glamor_finish_access(drawable);
>  }

As I noted on IRC, the putimage only affects the composite clip, while
you're prepare/finishing a bounding box, so this would trash the
contents outside of the composite clip.


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] [RFC] glamor: implement write-only prepares

[Michel Dänzer]

On 10.03.2016 10:51, Dave Airlie wrote:
> From: Dave Airlie 
> 
> For some putimages we know we won't ever care about the data we readback,
> we are going to trash it with the putimage contents. So implement
> GLAMOR_ACCESS_WO mode.
> 
> This will avoid doing the readbacks. Use it for putimages that are copy
> with a solid planemask.
> 
> inspired by Ilia and xlock -mode wator which does loads of XYBitmap
> putimages.
> 
> Signed-off-by: Dave Airlie 

Maybe you can also take some inspiration from
https://patchwork.freedesktop.org/patch/31020/ : In
glamor_prep_pixmap_box, it updated the access check and used
GL_STREAM_DRAW for GLAMOR_ACCESS_WO.


> diff --git a/glamor/glamor_prepare.c b/glamor/glamor_prepare.c
> index 5a73e6c..1c53165 100644
> --- a/glamor/glamor_prepare.c
> +++ b/glamor/glamor_prepare.c
> @@ -101,13 +101,14 @@ glamor_prep_pixmap_box(PixmapPtr pixmap, 
> glamor_access_t access, BoxPtr box)
>  priv->map_access = access;
>  }
>  
> -glamor_download_boxes(pixmap, RegionRects(), 
> RegionNumRects(),
> -  0, 0, 0, 0, pixmap->devPrivate.ptr, 
> pixmap->devKind);
> +if (priv->map_access != GLAMOR_ACCESS_WO)
> +glamor_download_boxes(pixmap, RegionRects(), 
> RegionNumRects(),
> +  0, 0, 0, 0, pixmap->devPrivate.ptr, 
> pixmap->devKind);
>  
>  RegionUninit();
>  
>  if (glamor_priv->has_rw_pbo) {
> -if (priv->map_access == GLAMOR_ACCESS_RW)
> +if (priv->map_access == GLAMOR_ACCESS_RW || priv->map_access == 
> GLAMOR_ACCESS_WO)
>  gl_access = GL_READ_WRITE;
>  else
>  gl_access = GL_READ_ONLY;
> @@ -144,7 +145,7 @@ glamor_fini_pixmap(PixmapPtr pixmap)
>  pixmap->devPrivate.ptr = NULL;
>  }
>  
> -if (priv->map_access == GLAMOR_ACCESS_RW) {
> +if (priv->map_access == GLAMOR_ACCESS_RW || priv->map_access == 
> GLAMOR_ACCESS_WO) {
>  glamor_upload_boxes(pixmap,
>  RegionRects(>prepare_region),
>  RegionNumRects(>prepare_region),

My patch used GL_WRITE_ONLY for GLAMOR_ACCESS_WO. Not sure it's worth it
though.


Other than that, your patch looks good to me.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
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] [RFC] glamor: implement write-only prepares

[Dave Airlie]

From: Dave Airlie 

For some putimages we know we won't ever care about the data we readback,
we are going to trash it with the putimage contents. So implement
GLAMOR_ACCESS_WO mode.

This will avoid doing the readbacks. Use it for putimages that are copy
with a solid planemask.

inspired by Ilia and xlock -mode wator which does loads of XYBitmap
putimages.

Signed-off-by: Dave Airlie 
---
 glamor/glamor_image.c   | 6 +-
 glamor/glamor_prepare.c | 9 +
 glamor/glamor_priv.h| 1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c
index 3158749..87fdcf6 100644
--- a/glamor/glamor_image.c
+++ b/glamor/glamor_image.c
@@ -88,7 +88,11 @@ static void
 glamor_put_image_bail(DrawablePtr drawable, GCPtr gc, int depth, int x, int y,
   int w, int h, int leftPad, int format, char *bits)
 {
-if (glamor_prepare_access_box(drawable, GLAMOR_ACCESS_RW, x, y, w, h))
+int access = GLAMOR_ACCESS_RW;
+
+if (gc->alu == GXcopy && glamor_pm_is_solid(gc->depth, gc->planemask))
+access = GLAMOR_ACCESS_WO;
+if (glamor_prepare_access_box(drawable, access, x, y, w, h))
 fbPutImage(drawable, gc, depth, x, y, w, h, leftPad, format, bits);
 glamor_finish_access(drawable);
 }
diff --git a/glamor/glamor_prepare.c b/glamor/glamor_prepare.c
index 5a73e6c..1c53165 100644
--- a/glamor/glamor_prepare.c
+++ b/glamor/glamor_prepare.c
@@ -101,13 +101,14 @@ glamor_prep_pixmap_box(PixmapPtr pixmap, glamor_access_t 
access, BoxPtr box)
 priv->map_access = access;
 }
 
-glamor_download_boxes(pixmap, RegionRects(), 
RegionNumRects(),
-  0, 0, 0, 0, pixmap->devPrivate.ptr, pixmap->devKind);
+if (priv->map_access != GLAMOR_ACCESS_WO)
+glamor_download_boxes(pixmap, RegionRects(), 
RegionNumRects(),
+  0, 0, 0, 0, pixmap->devPrivate.ptr, 
pixmap->devKind);
 
 RegionUninit();
 
 if (glamor_priv->has_rw_pbo) {
-if (priv->map_access == GLAMOR_ACCESS_RW)
+if (priv->map_access == GLAMOR_ACCESS_RW || priv->map_access == 
GLAMOR_ACCESS_WO)
 gl_access = GL_READ_WRITE;
 else
 gl_access = GL_READ_ONLY;
@@ -144,7 +145,7 @@ glamor_fini_pixmap(PixmapPtr pixmap)
 pixmap->devPrivate.ptr = NULL;
 }
 
-if (priv->map_access == GLAMOR_ACCESS_RW) {
+if (priv->map_access == GLAMOR_ACCESS_RW || priv->map_access == 
GLAMOR_ACCESS_WO) {
 glamor_upload_boxes(pixmap,
 RegionRects(>prepare_region),
 RegionNumRects(>prepare_region),
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index a70f10e..6df10b6 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -317,6 +317,7 @@ typedef struct glamor_screen_private {
 typedef enum glamor_access {
 GLAMOR_ACCESS_RO,
 GLAMOR_ACCESS_RW,
+GLAMOR_ACCESS_WO,
 } glamor_access_t;
 
 enum glamor_fbo_state {
-- 
2.5.0

___
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