[Mesa-dev] [PATCH] gbm: Add a flag to enable creation of rotated scanout buffers (v2)

[Ilia Mirkin]

On Thu, Nov 5, 2015 at 1:34 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 05.11.2015 11:32, Vivek Kasireddy wrote:
>> For certain platforms that support rotated scanout buffers, currently,
>> there is no way to create them with the GBM DRI interface. This flag
>> 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)
>>
>> Cc: Michel Danzer <michel at daenzer.net>
>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
>> ---
>>  include/GL/internal/dri_interface.h |  3 +++
>>  src/gbm/backends/dri/gbm_dri.c      | 19 +++++++++++++++++--
>>  src/gbm/main/gbm.h                  |  7 +++++++
>>  3 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
>> index 6bbd3fa..cd1bf62 100644
>> --- a/include/GL/internal/dri_interface.h
>> +++ b/include/GL/internal/dri_interface.h
>> @@ -1100,6 +1100,9 @@ struct __DRIdri2ExtensionRec {
>>  #define __DRI_IMAGE_USE_SCANOUT              0x0002
>>  #define __DRI_IMAGE_USE_CURSOR               0x0004 /* Depricated */
>>  #define __DRI_IMAGE_USE_LINEAR               0x0008
>> +#define __DRI_IMAGE_USE_ROTATION_90  0x0010
>> +#define __DRI_IMAGE_USE_ROTATION_180 0x0020
>> +#define __DRI_IMAGE_USE_ROTATION_270 0x0040
>>
>>
>>  /**
>
> I still think the dri_interface.h change should be a separate patch.
>
>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
>> index 57cdeac..a997946 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -539,7 +539,8 @@ gbm_dri_is_format_supported(struct gbm_device *gbm,
>>        break;
>>     case GBM_BO_FORMAT_ARGB8888:
>>     case GBM_FORMAT_ARGB8888:
>> -      if (usage & GBM_BO_USE_SCANOUT)
>> +      if (usage & (GBM_BO_USE_SCANOUT | GBM_BO_USE_ROTATION_90 |
>> +          GBM_BO_USE_ROTATION_180 | GBM_BO_USE_ROTATION_270))
>>           return 0;
>>        break;
>>     default:
>
> If you stick to this approach (see below), I suggest adding a
> GBM_BO_USE_SCANOUT_ANY define which combines all
> GBM_BO_USE_SCANOUT(_ROTATION_*) values, to make these checks less
> cumbersome.
>
>
>> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
>> index 8db2153..b200ca6 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_ROTATION_90 = (1 << 5),
>> +   GBM_BO_USE_ROTATION_180 = (1 << 6),
>> +   GBM_BO_USE_ROTATION_270 = (1 << 7),
>>  };
>>
>>  int
>>
>
> With this approach, these should be named GBM_BO_USE_SCANOUT_ROTATION_*
> to make it clearer that they are alternatives to GBM_BO_USE_SCANOUT.
>
> One issue with this approach is: What happens if several
> GBM_BO_USE_SCANOUT_ROTATION_* flags are set, and the hardware has
> conflicting requirements for different rotation angles?

Might make sense to make GBM_BO_USE_ROTATION be a 2-bit field... 0 =
SCANOUT, 1 = 90, 2 = 180, 3 = 270? that way conflicting flags can
never happen.

Just a drive-by comment...

  -ilia