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 <a...@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 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel