[Mesa-dev] [PATCH 1/2] gbm: Add a flag to enable creation of rotated scanout buffers

[Michel Dänzer]

On 27.10.2015 22:13, Ville Syrjälä wrote:
> On Fri, Oct 23, 2015 at 06:25:55PM -0700, Vivek Kasireddy wrote:
>> On Fri, 23 Oct 2015 15:29:08 +0300
>> Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
>>
>>> On Fri, Oct 23, 2015 at 12:18:39PM +0900, Michel Dänzer wrote:
>>>> On 23.10.2015 10:44, 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.
>>>>>
>>>>> Cc: Kristian Hogsberg <krh at bitplanet.net>
>>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
>>>>> ---
>>>>>  include/GL/internal/dri_interface.h | 1 +
>>>>>  src/gbm/backends/dri/gbm_dri.c      | 9 +++++++--
>>>>>  src/gbm/main/gbm.h                  | 5 +++++
>>>>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/GL/internal/dri_interface.h
>>>>> b/include/GL/internal/dri_interface.h index a0f155a..2271217
>>>>> 100644 --- a/include/GL/internal/dri_interface.h
>>>>> +++ b/include/GL/internal/dri_interface.h
>>>>> @@ -1091,6 +1091,7 @@ 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_SCANOUT_ROTATED_90_270
>>>>> 0x0010 
>>>>>  
>>>>>  /**
>>>>
>>>> Thank you for splitting out the driver change. Sorry I didn't think
>>>> of this before, but it might be worth splitting out the
>>>> dri_interface.h change as well. I'm fine either way, though.
>>>>
>>>>
>>>>> diff --git a/src/gbm/backends/dri/gbm_dri.c
>>>>> b/src/gbm/backends/dri/gbm_dri.c index 57cdeac..cde63de 100644
>>>>> --- a/src/gbm/backends/dri/gbm_dri.c
>>>>> +++ b/src/gbm/backends/dri/gbm_dri.c
>>>>> @@ -539,7 +539,7 @@ 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_SCANOUT_ROTATED_90_270)) return 0;
>>>>>        break;
>>>>>     default:
>>>>> @@ -748,6 +748,8 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>>>>>  
>>>>>     if (usage & GBM_BO_USE_SCANOUT)
>>>>>        dri_use |= __DRI_IMAGE_USE_SCANOUT;
>>>>> +   if (usage & GBM_BO_USE_SCANOUT_ROTATED_90_270)
>>>>> +      dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATED_90_270;
>>>>>     if (usage & GBM_BO_USE_CURSOR)
>>>>>        dri_use |= __DRI_IMAGE_USE_CURSOR;
>>>>>     if (dri->image->base.version >= 2 &&
>>>>> @@ -786,7 +788,8 @@ create_dumb(struct gbm_device *gbm,
>>>>>  
>>>>>     is_cursor = (usage & GBM_BO_USE_CURSOR) != 0 &&
>>>>>        format == GBM_FORMAT_ARGB8888;
>>>>> -   is_scanout = (usage & GBM_BO_USE_SCANOUT) != 0 &&
>>>>> +   is_scanout = (usage & (GBM_BO_USE_SCANOUT |
>>>>> +      GBM_BO_USE_SCANOUT_ROTATED_90_270)) != 0 &&
>>>>>        format == GBM_FORMAT_XRGB8888;
>>>>>     if (!is_cursor && !is_scanout) {
>>>>>        errno = EINVAL;
>>>>> @@ -880,6 +883,8 @@ gbm_dri_bo_create(struct gbm_device *gbm,
>>>>>  
>>>>>     if (usage & GBM_BO_USE_SCANOUT)
>>>>>        dri_use |= __DRI_IMAGE_USE_SCANOUT;
>>>>> +   if (usage & GBM_BO_USE_SCANOUT_ROTATED_90_270)
>>>>> +      dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATED_90_270;
>>>>>     if (usage & GBM_BO_USE_CURSOR)
>>>>>        dri_use |= __DRI_IMAGE_USE_CURSOR;
>>>>>     if (usage & GBM_BO_USE_LINEAR)
>>>>> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
>>>>> index 2708e50..2ef7bd8 100644
>>>>> --- a/src/gbm/main/gbm.h
>>>>> +++ b/src/gbm/main/gbm.h
>>>>> @@ -213,6 +213,11 @@ 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 90/270 rotated buffers.
>>>>> +    */
>>>>> +   GBM_BO_USE_SCANOUT_ROTATED_90_270 = (1 << 5),
>>>>>  };
>>>>>  
>>>>>  int
>>>>>
>>>>
>>>> I asked internally, and apparently our display hardware requires a
>>>> rotation specific tiling mode for 180 degree rotation as well. In
>>>> order to avoid having to add *_SCANOUT_ROTATED_180 later, would
>>>> *_SCANOUT_ROTATED work for you as well? Or would using Y-tiling for
>>>> 180 degree rotation be an issue?
>>>
>>> What about a bit per angle? To avoid hardware specifics.
>>
>> Hi Ville,
>> I am not sure what's the best way to move forward as you know our
>> (Intel) new hardware needs Y-tiling only for 90/270.
> 
> Not sure what's the problem. A bit per angle should work for everyone
> AFAICS. The already existing scanout flag could be considered to mean
> 0 degrees.

Right.

Or, in order to avoid churn in every place where GBM_BO_USE_SCANOUT is
used in the code (and to save one bit ;), we could use independent enum
values for rotation, e.g.:

   GBM_BO_USE_ROTATION_90 = (1 << 5),
   GBM_BO_USE_ROTATION_180 = (2 << 5),
   GBM_BO_USE_ROTATION_270 = (3 << 5),


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