[PATCH V2] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).

[Michel Dänzer]

On 04.07.2014 18:17, Michel Dänzer wrote:
> On 04.07.2014 17:51, Ander Conselvan de Oliveira wrote:
>> On 07/04/2014 04:13 AM, Michel Dänzer wrote:
>>> On 03.07.2014 21:27, Ander Conselvan de Oliveira wrote:
>>>> On 06/25/2014 05:09 PM, Alvaro Fernando García wrote:
>>>>> Init cursor size to 64x64 if drmGetCap() fails.
>>>>>
>>>>> Use Mesa GBM_BO_USE_CURSOR define (which removes 64x64 restriction)
>>>>>
>>>>> Signed-off-by: Alvaro Fernando García <alvarofernandogarcia at gmail.com>
>>>>> ---
>>>>>    src/compositor-drm.c | 43
>>>>> ++++++++++++++++++++++++++++++++++++-------
>>>>>    1 file changed, 36 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>>>>> index 7d514e4..61ddea1 100644
>>>>> --- a/src/compositor-drm.c
>>>>> +++ b/src/compositor-drm.c
>>>>> @@ -55,6 +55,14 @@
>>>>>    #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
>>>>>    #endif
>>>>>
>>>>> +#ifndef DRM_CAP_CURSOR_WIDTH
>>>>> +#define DRM_CAP_CURSOR_WIDTH 0x8
>>>>> +#endif
>>>>> +
>>>>> +#ifndef DRM_CAP_CURSOR_HEIGHT
>>>>> +#define DRM_CAP_CURSOR_HEIGHT 0x9
>>>>> +#endif
>>>>> +
>>>>>    static int option_current_mode = 0;
>>>>>
>>>>>    enum output_config {
>>>>
>>>> [...]
>>>>
>>>>>
>>>>> @@ -1554,15 +1577,21 @@ drm_output_init_egl(struct drm_output *output,
>>>>> struct drm_compositor *ec)
>>>>>            return -1;
>>>>>        }
>>>>>
>>>>> -    flags = GBM_BO_USE_CURSOR_64X64 | GBM_BO_USE_WRITE;
>>>>> +#ifdef GBM_BO_USE_CURSOR
>>>>> +    flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE;
>>>>> +#else
>>>>> +    flags = GBM_BO_USE_WRITE;
>>>>> +    if (ec->cursor_width == 64 && ec->cursor_height == 64)
>>>>> +        flags = GBM_BO_USE_CURSOR_64X64 | flags;
>>>>> +#endif
>>>>
>>>> Do we really need this? GBM_BO_USE_CURSOR has the same value as the old
>>>> _64X64 flag. GBM will check if the dimensions are 64x64 and fail
>>>> otherwise.
>>>
>>> No, that check was removed when adding GBM_BO_USE_CURSOR.
>>>
>>>
>>>> So this could just be
>>>>
>>>>      flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE;
>>>>
>>>> and a
>>>>
>>>> #ifndef GBM_BO_USE_CURSOR
>>>> #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
>>>> #endif
>>>>
>>>> earlier in the file.
>>>
>>> No, if GBM doesn't define GBM_BO_USE_CURSOR, it will likely fail if the
>>> dimensions are not 64x64.
>>
>> And that is what we expect. We shouldn't rely on a buffer allocated with
>> only GBM_BO_USE_WRITE to use as a hardware cursor.
>>
>> My point is that the code I proposed should work with both old and new
>> GBM. For old GBM, if cursor is not 64x64, gbm won't allocate the bo and
>> the check that follows the allocation will disable hardware cursors. If
>> GBM is new enough, everything works. On top of that, it avoids the ifdef
>> in the middle of a function.
> 
> Hmm, yes, that makes sense.

Alvaro, can you simplify the patch according to Ander's suggestion?


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