[Mesa-dev] [PATCH 4/4] egl/dri2: add image extension to swrast_core_extensions

[Emil Velikov]

On 28 June 2017 at 11:17, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Hi Gurchetan,
>
> Pardon for the delay.
>
> On 22 June 2017 at 00:40, Gurchetan Singh <gurchetansingh at chromium.org> wrote:
>> Emil,
>>
>> If I understand you correctly, you're proposing to add the ability to use
>> the kms_swrast driver in platform_x11.c (the host is a standard Ubuntu box
>> for the emulator use case, not CrOS) alongside swrast.
>>
>> In that case, we would need to:
>>
>> 1) Have a dri2_initialize_x11_kms_swrast function that's called when some
>> environment variable is set instead of dri2_initialize_x11_swrast.
> There is the obvious caveat that LIBGL_ALWAYS_SOFTWARE should be set.
>
> Other than that - we can try kms_swrast and fallback to swrast.
> If the former is very incomplete relative to the latter then we might
> consider adding another envvar.
>
>> 2) dri2_initialize_x11_kms_swrast would need access to the host card fd
>> (dri_kms_init_screen requires this) and call dri2_load_driver instead of
>> dri2_load_driver_swrast .
> Indeed - in the GBM case, we get that from the user. In here, we
> _should_ be able to use the existing one passed from X. See below for
> more.
>
>> 3) Use dri2_loader_extensions instead of swrast_loader_extensions,
>> dri2_x11_display_vtbl instead dri2_x11_swrast_display_vtbl etc.
>>
> Correct again.
>
>> I'm having trouble getting this to work, and I was wondering if what I'm
>> trying to do is what you want.  Attached is the patch I'm trying (it
>> compiles, but will crash your display).
>>
> Please keep patches a) inline and b) functional changes separate from
> code movement.
> Fishing for ~5 diff in a 100 line patch isn't fun :-\
>
> Re the issue:
>  - opening card0 without authenticating is a serious no-go
>  - at the same time we shouldn't need the open - reuse the fd, as
> mentioned above.
>  - if above does not work use the renderD for the given fd ->
> open(drmGetRenderDeviceNameFromFd(fd)...)
>
Upon second thought - I'm not too sure if my original suggestion may
not work without some serious work.

Which seemingly is not required for EGL_KHR_gl_image - lucky me got
mislead by more missing checks out our codebase :-\

Some background:
The __DRI_IMAGE extension adds the following
    .createImageFromName
    .createImageFromRenderbuffer
    .destroyImage
    .createImage
    .queryImage
    .dupImage
    .validateUsage
    .createImageFromNames
    .fromPlanar
    .createImageFromTexture
    .createImageFromFds
    .createImageFromDmaBufs
    .blitImage
    .getCapabilities
    .mapImage
    .unmapImage


Of which createImageFromTexture and createImageFromRenderbuffer seems
to be required for  KHR_gl_texture_*_image and
KHR_gl_renderbuffer_image respectively (see dri2_create_image_khr).


Although overall implementation-wise _DRI_IMAGE is as follows:

For software gallium drivers:
- MESA_drm_image, MESA_image_dma_buf_export are not possible since they require
a) fd set to -1, and required for the drmGetCap() ioctl
b) .queryImage is not implemented
resource_get_handle -> {softpipe,llvmpipe} displaytarget_get_handle ->
sw/dri - NOOP, sw/kms-dri fully implemented
c) .createImageFrom{Name,Fds,DmaBuf,DmaBufs2}
resource_from_handle -> {softpipe,llvmpipe} displaytarget_from_handle
-> sw/dri - NOOP, sw/kms-dri fully implemented

Overall st/dri:
- KHR_gl_renderbuffer_image although advertised does not work
a) .createImageFromRenderbuffer is a stub (see
dri2_create_image_from_renderbuffer) :-\


Tl;Dr; your initial approach may work with some minor changes -
comments coming shortly.
There's handful of bugs in Mesa we need to address as well.

Thanks
Emil