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

[Michel Dänzer]

On 10.03.2016 10:51, Dave Airlie wrote:
> From: Dave Airlie <airl...@redhat.com>
> 
> 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 <airl...@redhat.com>

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(&region), 
> RegionNumRects(&region),
> -                          0, 0, 0, 0, pixmap->devPrivate.ptr, 
> pixmap->devKind);
> +    if (priv->map_access != GLAMOR_ACCESS_WO)
> +        glamor_download_boxes(pixmap, RegionRects(&region), 
> RegionNumRects(&region),
> +                              0, 0, 0, 0, pixmap->devPrivate.ptr, 
> pixmap->devKind);
>  
>      RegionUninit(&region);
>  
>      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(&priv->prepare_region),
>                              RegionNumRects(&priv->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