[PATCH libdrm] tests/modetest: Add modetest_atomic tool

[Benjamin Gaignard]

2018-07-25 14:05 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
> On 25 July 2018 at 12:27, Benjamin Gaignard
> <benjamin.gaignard at linaro.org> wrote:
>> 2018-07-25 12:29 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
>>> On 20 July 2018 at 12:33, Benjamin Gaignard
>>> <benjamin.gaignard at linaro.org> wrote:
>>>> This is a modetest like tool but using atomic API.
>>>>
>>>> With modetest_atomic it is mandatory to specify a mode ("-s")
>>>> and a plane ("-P") to display a pattern on screen.
>>>>
>>>> "-v" does a loop swapping between two framebuffers for each
>>>> active planes.
>>>>
>>>> modetest_atomic doesn't offer cursor support
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at linaro.org>
>>>> ---
>>>>
>>>> The code is based on modetest and keep most of it infrastructure
>>>> like arguments parsing, finding properties id from their name,
>>>> resources dumping or the general way of working.
>>>> It duplicates modetest code but adding compilation flags or
>>>> conditional tests everywhere in modetest would have made it
>>>> more complex and unreadable.
>>>>
>>>> Creating modetest_atomic could allow to test atomic API without
>>>> need to use "big" frameworks like weston, drm_hwcomposer or igt
>>>> with all their dependencies.
>>>> kmscube could also be used to test atomic API but it need EGL.
>>>>
>>>> It have been tested (only) on stm driver with one or two planes
>>>> actived.
>>>>
>>>>  tests/modetest/Makefile.am       |   13 +-
>>>>  tests/modetest/Makefile.sources  |    7 +
>>>>  tests/modetest/modetest_atomic.c | 1546 ++++++++++++++++++++++++++++++++++++++
>>>
>>> Quick diff/grep/wc exercise shows that ~10% of modetest_atomic.c is new code.
>>> I agree that modetest.c is getting kind of big and messy. How about
>>> instead of duplicating it, we flesh out the legacy vs atomic codepaths
>>> into helpers in separate files?
>>>
>>> It will result in [c]leaner code, only one executable
>>> installed/packaged and less build system changes.
>>>
>>> What do you think?
>>> Emil
>>
>> It was my initial thought too: I have tried to add "-a" option to
>> enable atomic way
>> of working but their is small changes to do everywhere and that add
>> lot of conditional tests.
> Can you please try beating something into shape? It's fairly hard to
> judge without anything to look at.

After dump_resource  code in main function all the logic is different
between the two
modetest because, with atomic you to build a request and commit it
while, with legacy API,
you could set mode and after set a plane.
In addition with atomic version you have set a mode and a plane to be
able to display something.
That explain why the end of main function is completely different.

Another example in the set_mode function: in legacy API you have
create a FB and reference/use it
with calls to bo_create, drmModeAddFB2 and drmModeSetCrtc while for
atomic you have to set
properties on connectors, create and set a blob for the mode before
active the crct.
The same diff exist for set_plane.

I can add big if/else blocks but I do think that will make the code
more readable.

>
> Thanks
> Emil