[Mesa-dev] [PATCH 2/2] gbm: Add flags to enable creation of rotated scanout buffers (v3)

[Michel Dänzer]

On 06.11.2015 12:08, Vivek Kasireddy wrote:
> For certain platforms that support rotated scanout buffers, currently,
> there is no way to create them with the GBM DRI interface. These flags
> will instruct the DRI driver to create the buffer by setting
> additional requirements such as tiling mode.
> 
> v2: Reserve a bit per angle. (Ville and Michel)
> 
> v3:
> - Combine all GBM_BO_USE_SCANOUT_ROTATION_* flags into
>   GBM_BO_USE_SCANOUT_ANY macro (Michel)
> - Pull the code that updates dri_use based on the rotation flag
>   into a separate function.

[...]

> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index 57cdeac..741e509 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -124,6 +124,20 @@ image_get_buffers(__DRIdrawable *driDrawable,
>  }
>  
>  static void
> +gbm_to_dri_flag(uint32_t usage,
> +                unsigned *dri_use)
> +{
> +   if (usage & GBM_BO_USE_SCANOUT)
> +      *dri_use |= __DRI_IMAGE_USE_SCANOUT;
> +   if (usage & GBM_BO_USE_SCANOUT_ROTATION_90)
> +      *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_90;
> +   if (usage & GBM_BO_USE_SCANOUT_ROTATION_180)
> +      *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_180;
> +   if (usage & GBM_BO_USE_SCANOUT_ROTATION_270)
> +      *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_270;
> +}

I like the idea of this helper function, but it could handle
GBM_BO_USE_CURSOR and GBM_BO_USE_LINEAR as well. Ideally, there would be
a separate preparatory change which creates the helper function and
makes gbm_dri_bo_import and gbm_dri_bo_create use it; then this change
can just add the new flags in the helper function. If that's too much
trouble, the handling of the other flags can be moved into the helper
function in a followup change.


> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> index 8db2153..4bda089 100644
> --- a/src/gbm/main/gbm.h
> +++ b/src/gbm/main/gbm.h
> @@ -214,6 +214,13 @@ enum gbm_bo_flags {
>      * Buffer is linear, i.e. not tiled.
>      */
>     GBM_BO_USE_LINEAR = (1 << 4),
> +   /**
> +    * Buffer would be rotated and some platforms have additional tiling
> +    * requirements for rotated scanout buffers.
> +    */
> +   GBM_BO_USE_SCANOUT_ROTATION_90 = (1 << 5),
> +   GBM_BO_USE_SCANOUT_ROTATION_180 = (1 << 6),
> +   GBM_BO_USE_SCANOUT_ROTATION_270 = (1 << 7),
>  };

Hmm, we should probably explicitly specify the orientation of the 90 and
270 degree rotations. Clockwise? (Same in patch 1)


> @@ -240,6 +247,10 @@ gbm_bo_create(struct gbm_device *gbm,
>  #define GBM_BO_IMPORT_WL_BUFFER         0x5501
>  #define GBM_BO_IMPORT_EGL_IMAGE         0x5502
>  #define GBM_BO_IMPORT_FD                0x5503
> +#define GBM_BO_USE_SCANOUT_ANY (GBM_BO_USE_SCANOUT |		  \
> +				GBM_BO_USE_SCANOUT_ROTATION_90 |  \
> +				GBM_BO_USE_SCANOUT_ROTATION_180 | \
> +				GBM_BO_USE_SCANOUT_ROTATION_170)
>  
>  struct gbm_import_fd_data {
>     int fd;
> 

Please add the define after the definition of enum gbm_bo_flags instead
of here.


Other than that, these are starting to look pretty good to me.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer