[PATCH 13/19] present: Add driver facing window flip mode hooks

[Roman Gilg]

The idea with this whole "flip mode API" was to separate the window
flip mode from the screen flip mode as much as possible and touch the
old code only as little as possible, such that the probability for
regressions decreases and the possibility for adding other modes or
just partial functionality for one of the modes in the future without
large rewrites maximizes. Regarding future additions I was thinking of

a) crtc flips as a possible third mode (or some other mode currently
not on our radar),
b) non-copy flips in window flip mode.

Also I assume a driver is normally either only interested in window or
screen flips. So let's assume a driver wants to do window flips: it
can decide upon this by setting the respective present_wnmd_info
struct and call its init function independently of any version shift
in the screen flip struct. This clear separation is a good thing in my
opinion.

I understand your point and there are for sure some good reasons to do
it this way (although I would argue that there are also good reasons
to do it like I did now and I hope the ones I mentioned are already
quite convincing). But anyways I have designed the code structure now
with this "flip mode API" including separate driver hooks and I
neither have the time nor the mental fortitude to rewrite it again in
order to merge the two API modes, which would basically completely
remove the separation I came up with.

On Mon, Jan 29, 2018 at 7:08 PM, Adam Jackson <ajax at nwnk.net> wrote:
> On Mon, 2018-01-29 at 14:34 +0100, Roman Gilg wrote:
>
>> @@ -100,6 +128,21 @@ typedef struct present_screen_info {
>>
>>  } present_screen_info_rec, *present_screen_info_ptr;
>>
>> +typedef struct present_wnmd_info {
>> +    uint32_t                            version;
>> +
>> +    present_get_crtc_ptr                get_crtc;
>> +    present_wnmd_get_ust_msc_ptr        get_ust_msc;
>> +    present_wnmd_queue_vblank_ptr       queue_vblank;
>> +    present_wnmd_abort_vblank_ptr       abort_vblank;
>> +    present_flush_ptr                   flush;
>> +    uint32_t                            capabilities;
>> +    present_check_flip_ptr              check_flip;
>> +    present_wnmd_flip_ptr               flip;
>> +    present_wnmd_unflip_ptr             unflip;
>> +
>> +} present_wnmd_info_rec, *present_wnmd_info_ptr;
>
> I'm not a huge fan of this. present_screen_info is versioned for
> exactly this reason, if we want to add new functionality we bump the
> 'version' field and add new function pointers to the end of the vtable.
>
> The mode-dependent bit of present_destroy_window would admittedly be a
> bit awkward to add to this table, since it's the driver that would fill
> it in. It might be nice to refactor that path so even the screen-mode
> code tracked flips per-window (and just only ever _did_ any on the root
> window). Which, tbh, might not be a bad idea in general: add a
> check_flip_window to present_screen_info, and for the "screen mode"
> code just fail if the window isn't the root.
>
> - ajax