[Mesa-dev] [PATCH] gbm: Add gbm_bo_write entry point

[Ander Conselvan de Oliveira]

I did a simple test here and the patch worked. I have just a couple of 
comments.

On 05/02/2012 10:32 PM, Kristian Høgsberg wrote:
> This new gbm entry point allows writing data into a gbm bo.  The bo has
> to be created with the GBM_BO_USE_WRITE flag, and it's only required to
> work for GBM_BO_USE_CURSOR_64X64 bos.
>
> The gbm API is designed to be the glue layer between EGL and KMS, but there
> was never a mechanism initialize a buffer suitable for use with KMS
> hw cursors.  The hw cursor bo is typically not compatible with anything EGL
> can render to, and thus there's no way to get data into such a bo.
>
> gbm_bo_write() fills that gap while staying out of the efficient
> cpu->gpu pixel transfer business.
> ---
>   include/GL/internal/dri_interface.h       |   10 +++++++++-
>   src/gbm/backends/dri/gbm_dri.c            |   18 ++++++++++++++++++
>   src/gbm/main/gbm.c                        |   19 +++++++++++++++++++
>   src/gbm/main/gbm.h                        |    9 +++++++++
>   src/gbm/main/gbmint.h                     |    1 +
>   src/mesa/drivers/dri/intel/intel_screen.c |   24 ++++++++++++++++++++++--
>   6 files changed, 78 insertions(+), 3 deletions(-)
>

[...]

> diff --git a/src/mesa/drivers/dri/intel/intel_screen.c b/src/mesa/drivers/dri/intel/intel_screen.c
> index 9db5606..e945d14 100644
> --- a/src/mesa/drivers/dri/intel/intel_screen.c
> +++ b/src/mesa/drivers/dri/intel/intel_screen.c
> @@ -305,6 +305,11 @@ intel_create_image(__DRIscreen *screen,
>         tiling = I915_TILING_NONE;
>      }
>
> +   /* We only support write for cursor drm images */
> +   if ((use&  __DRI_IMAGE_USE_WRITE)&&
> +       use != (__DRI_IMAGE_USE_WRITE | __DRI_IMAGE_USE_CURSOR))
> +      return NULL;
> +

Shouldn't we have a similar check in intel_validate_usage()?

>      image = CALLOC(sizeof *image);
>      if (image == NULL)
>         return NULL;
> @@ -411,15 +416,30 @@ intel_validate_usage(__DRIimage *image, unsigned int use)
>      return GL_TRUE;
>   }
>
> +static int
> +intel_image_write(__DRIimage *image, const void *buf, size_t count)
> +{
> +   if (image->region->map_refcount)
> +      return -1;
> +   /* if use != WRITE || region->map_refcount; return -1; */

Did you intend to check for write usage here?

Other than this,
Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>