[Mesa-stable] [PATCH 2/2] auxiliary: use vl_drm_screen_create method for surfaceless

[Sharma, Deepak]

Hi Emil,

Is your suggestion is to build mesa in chrome using --with-platforms=drm and --with-platforms=surfaceless both ? I am not sure if that is required for chrome.
The approach we are taking here is to build mesa only with --with-platforms=surfaceless and leverage existing 
VA_DISPLAY_DRM* to get VAAPI working on Chrome for platform.

Thanks,
Deepak

-----Original Message-----
From: Emil Velikov [mailto:emil.l.velikov at gmail.com] 
Sent: Tuesday, October 17, 2017 3:38 AM
To: Guttula, Suresh <Suresh.Guttula at amd.com>
Cc: ML mesa-dev <mesa-dev at lists.freedesktop.org>; Sharma, Deepak <Deepak.Sharma at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; ML mesa-stable <mesa-stable at lists.freedesktop.org>; Emil Velikov <emil.velikov at collabora.com>
Subject: Re: [Mesa-stable] [PATCH 2/2] auxiliary: use vl_drm_screen_create method for surfaceless

HI Suresh,

Please try to avoid HTML emails. If that's not possible, make sure your reply is more readable.
At the moment it has 3 different fonts and 3 different colours, which makes is distracting and hard to read.

Please tweak the quoting format - older replies should be a level deeper. See [1] [2]

On 17 October 2017 at 07:36, Guttula, Suresh <Suresh.Guttula at amd.com> wrote:
> On 11 October 2017 at 07:13, Guttula, Suresh <Suresh.Guttula at amd.com> wrote:
>> HI,
>>
>>>- why do we need "surfaceless" support
>>    ChromeOS supports surfacelsess and we need this va enablement for 
>> surfaceless in chromium.
> Ack, that should have been part of the commit message.
>    >>>I will update the commit message.
>
>>> - does upstream VAAPI has surfaceless platform
>>     Yes. It uses headless support of VA-API for decoding.
> There's no VA_DISPLAY_SURFACELESS in libva [1]. Thus adding one here 
> is _very_ confusing and misleading.
>   >>>Sorry I understood wrongly the question, I thought you are asking 
> about mesa-vaapi. In libva it is using drm path only. If I understood 
> correctly , no need of any macro  VA_DISPLAY_SURFACELESS in libva as 
> there is no problem to use drm path for egl platform surfaceless. The 
> problem exists in mesa side as the check is added to enable va based on platform.
> https://github.com/01org/libva/blob/master/va/va_backend.h#L39
>   >>>libva uses “VA_DISPLAY_DRM_RENDERNODES”  in this case. In libva 
> ,Chromium (Ozone) for egl surfaceless platform goes for drm display .
> https://cs.chromium.org/chromium/src/media/gpu/vaapi_wrapper.cc?rcl=e1
> a85cf02acf0b4ccaad6e37afcf41d1fd26ce24&l=1188
>
>>>  - why is the surfaceless implementation identical to the DRM one
>>     If I understand your question correctly, In case of surfaceless 
>> platform ,it uses headless support of VAAPI, which will use drm 
>> implementation. If I miss something here please provide some more 
>> details on the question.
>>
Precisely - in hind sight, one might have called the libva display VA_DISPLAY_SURFACELESS.
Regardless, it's just a name so not a bit deal how it's called, as long as things are consistent.

You're adding surfaceless for Mesa VAAPI (backend) that interacts with libva VA_DISPLAY_DRM* (frontend). That's the confusing/misleading part I'm talking about.

> To put it otherwise:
>
> You're "adding" support for surfaceless for the sake of adding a name.
> There's no functional difference nor upstream (see the libva question
> above) demand for it.
>    >>>The reason for adding "surfaceless"  in mesa is the condition 
> checks for platform "drm/wayland/x11" to enable va.
> But in case of chromium ,we build mesa with_egl_platforms=surfaceless 
> and not mesa_gbm because chromium uses minigbm .So echo $platform is 
> surfaceless, even it is using drm path, condition check fail because 
> of platform type picked as surfaceless and va is not enabled.
>
Or in other words:
 - CrOS uses its own GBM,
 - using --with-platforms=gbm requires Mesa's GBM

Thus the solution here really is:
 - decouple the link-time dependency to a (once-off) runtime one
 - and(?) demote the configure error to a warning ;-) Right?

While we're there we could/should:
 - drop the (first_pointer == gbm_create_device) hack Replace with dladdr(first_pointer, &info) + strcmp(info.dli_sname,
"gbm_create_device") combo
 - make egl_dri2.c free of calls into gbm - only gbm_device_destroy remains Move the remaining gbm_device_destroy to platform_drm.c

Bonus points:
 - Add ABI and/or version check for Mesa GBM <> EGL interop.

Does that make sense? It seems like a more robust solution, IMHO.

Thanks
Emil

[1] https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
[2] https://www.extendoffice.com/documents/outlook/4006-outlook-reply-quote.html#a1