[PATCH] Add freesync ioctl interface

[Christian König]

Am 04.08.2016 um 08:41 schrieb Michel Dänzer:
> On 03.08.2016 10:04, Dave Airlie wrote:
>> On 2 August 2016 at 12:26, Hawking Zhang <Hawking.Zhang at amd.com> wrote:
>>> Change-Id: I38cb3a80e75a904cee875ae47bc0a39a3d471aca
>>> Signed-off-by: Hawking Zhang <Hawking.Zhang at amd.com>
>>> ---
>>>   include/drm/amdgpu_drm.h | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
>>> index 46a3c40..a4f816c 100644
>>> --- a/include/drm/amdgpu_drm.h
>>> +++ b/include/drm/amdgpu_drm.h
>>> @@ -48,6 +48,7 @@
>>>   #define DRM_AMDGPU_GEM_USERPTR         0x11
>>>   #define DRM_AMDGPU_WAIT_FENCES         0x12
>>>   #define DRM_AMDGPU_GEM_FIND_BO          0x13
>>> +#define DRM_AMDGPU_FREESYNC             0x14
>>>
>>>   #define DRM_IOCTL_AMDGPU_GEM_CREATE    DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
>>>   #define DRM_IOCTL_AMDGPU_GEM_MMAP      DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
>>> @@ -63,6 +64,7 @@
>>>   #define DRM_IOCTL_AMDGPU_GEM_USERPTR   DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
>>>   #define DRM_IOCTL_AMDGPU_WAIT_FENCES   DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
>>>   #define DRM_IOCTL_AMDGPU_GEM_FIND_BO      DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_FIND_BO, struct drm_amdgpu_gem_find_bo)
>>> +#define DRM_IOCTL_AMDGPU_FREESYNC       DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FREESYNC, struct drm_amdgpu_freesync)
>>>
>>>   #define AMDGPU_GEM_DOMAIN_CPU          0x1
>>>   #define AMDGPU_GEM_DOMAIN_GTT          0x2
>>> @@ -706,4 +708,17 @@ struct drm_amdgpu_virtual_range {
>>>          uint64_t start;
>>>          uint64_t end;
>>>   };
>>> +
>>> +/*
>>> + * Definition of free sync enter and exit signals
>>> + * We may have more options in the future
>>> + */
>>> +#define AMDGPU_FREESYNC_FULLSCREEN_ENTER                1
>>> +#define AMDGPU_FREESYNC_FULLSCREEN_EXIT                 2
>>> +
>>> +struct drm_amdgpu_freesync {
>>> +        __u32 op;                       /* AMDGPU_FREESYNC_FULLSCREEN_ENTER or */
>>> +                                        /* AMDGPU_FREESYNC_FULLSCREEN_ENTER */
>>> +        __u32 spare[7];
>>> +};
>>>   #endif
>> Isn't freesync meant to be a generic non-driver useful thing?
>>
>> This should be integrated with atomic modesetting API or just the KMS APIs.
> I don't see that as required or even particularly useful until there's
> an API which apps can use to control variable refresh rate.

VDPAU already does this quite well and for OpenGL you usually say that 
you want the next frame to be pushed out as fast as possible when you 
don't use any additional extensions to set that.

> The goal
> here is merely to allow running existing apps with variable refresh
> rate, which just requires some kind of toggle(s) to enable/disable it.
>
>
> That said, I also see some issues with this patch:
>
> There's a bit of a conflation between "FreeSync" and "fullscreen"
> concepts. I think at this point it's really about a sort of an "is
> FreeSync allowed?" state rather than about fullscreen.

I would say we first of all need a new flag for drm_mode_modeinfo to 
note if a certain mode is a freesync mode or not and what the min/max 
parameters are.

>
> It might be nice to track the "is FreeSync allowed?" state per-CRTC, as
> I wonder if FreeSync might be useful e.g. for smoother TearFree operation.
>
> And if it's just a per-CRTC state, something like an output property
> might be more appropriate than a dedicated ioctl.

That will obvious work as well, but essentially freesync is a parameter 
of the video mode in use if I'm not completely mistaken.


Regards,

Christian.