[Mesa-dev] [PATCH EGL/MESA] EGL/mesa: Initial writeup for MESA_query_renderer

[Rob Clark]

On Wed, Nov 7, 2018 at 4:12 PM Veluri Mithun <velurimithun38 at gmail.com> wrote:
>
> Hi all,
> Thanks Adam for your interest in this.
>
>> > +New Tokens
>> > +
>> > +    Accepted as an <attribute> in EGLQueryRendererIntegerMESA and
>> > +    EGLQueryCurrentRendererIntegerMESA:
>> > +
>> > +        EGL_RENDERER_VENDOR_ID_MESA                      0xXXXX
>> > +        EGL_RENDERER_DEVICE_ID_MESA                      0xXXXX
>
>
> except the above 2 tokens we aren't using any tokens in adriconf(https://github.com/jlHertel/adriconf/blob/master/DRIQuery.cpp#L46) If we don't need all the below ones, I'll remove them.

I think might as well keep them.  If they were added to the glx
version of the spec, then they are probably worthwhile to someone.
And getting the spec right the first time is better than having to
create an EGL_MESA_query_renderer2 later

>>
>> > +        EGL_RENDERER_VERSION_MESA                        0xXXXX
>> > +        EGL_RENDERER_ACCELERATED_MESA                    0xXXXX
>> > +        EGL_RENDERER_VIDEO_MEMORY_MESA                   0xXXXX
>> > +        EGL_RENDERER_UNIFIED_MEMORY_ARCHITECTURE_MESA    0xXXXX
>> > +        EGL_RENDERER_OPENGL_CORE_PROFILE_VERSION_MESA    0xXXXX
>> > +        EGL_RENDERER_OPENGL_COMPATIBILITY_PROFILE_VERSION_MESA    0xXXXX
>> > +
>>
>>
>> Normally these are reserved by creating a request against the EGL
>> registry on github. However we happen to have a block of 16 enum values
>> already reserved:
>>
>> >     <enums namespace="EGL" start="0x3290" end="0x329F" vendor="MESA" comment="Reserved for John Kåre Alsaker (Public bug 757)">
>> >             <unused start="0x3290" end="0x329F"/>
>> >     </enums>
>>
>> That "public bug 757" is for the abandoned EGL_MESA_image_sRGB
>> extension:
>>
>> https://www.khronos.org/bugzilla/show_bug.cgi?id=757
>>
>> So I think it's safe to use enums from that range, and we'll still have
>> two free.
>
> I'll assign these, once we finalize all the tokens.
>
>> > +        Bool EGLQueryRendererIntegerMESA(EGLDisplay *dpy, int screen, int renderer,
>> > +                                         int attribute, unsigned int *value);
>
> @Nicolai Hähnle , @Rob Clark
> we are using only this queryRenderInteger function in adriconf. The rest 3 funcions can be exempted??
>

I think keep them, although as Adam suggested, if we can re-work to
just use eglQueryContext() instead (with EGL_NO_CONTEXT to preserve
the ability to query without a context??  Not entirely sure if that is
a good idea in the wayland case when you have multiple GPUs)

Note that it looks like adriconf also needs an equiv to
glXGetDriverConfig()..  not really sure if that is actually documented
as being part of some spec, or just a magic thing?

BR,
-R

>>
>> The corresponding eglQueryCurrentRendererIntegerMESA is not documented.
>> I'm not entirely sure it should even exist, to be honest; I'd prefer if
>> these attributes were instead newly legal values to pass to
>> eglQueryContext. Too late to make that change for GLX I suppose.
>
>
> What is the difference between EGLContext and EGLSurface?
>
> If you look at the usage of query fun in adriocnf(https://github.com/jlHertel/adriconf/blob/master/DRIQuery.cpp#L43). we are iterating with all the available screens. In Wayland do we need to iterate through contexts??
> @Jean Hertel In the above provided link, why are we passing 3rd argument(renderer) as 0 for all the screens?
>
>>
>>
>> > +    [Add to section section 3.3.7 "Rendering Contexts"]
>> > +
>> > +    The attribute name EGL_RENDERER_ID_MESA specified the index of the render
>> > +    against which the context should be created.  The default value of
>> > +    EGL_RENDERER_ID_MESA is 0.
>>
>> This startled me to read, I didn't think GLX_MESA_query_renderer had
>> this. Turns out it does have this text in the extension spec, but the
>> functionality is not actually implemented. Worse, there's no way to
>> enumerate how many renderers a display (or display/screen in GLX) has.
>>
>> To address this, I would:
>>
>> 1) Remove this text from both GLX and EGL extension specs
>>    (Both here and above where the enums are listed)
>> 2) Update both specs with an explicit way to enumerate renderers
>>    (probably something like "if renderer is -1 and the attribute is
>>    XXX_RENDERER_COUNT_MESA, return one integer")
>> 3) Add a layered extension to add this functionality to CreateContext
>>
>> #2 and #3 are optional, I suppose. Probably it's better to have only
>> one way to do that, and in EGL that way is probably using the EGL
>> device extensions instead. But GLX doesn't have anything like that, so
>> this functionality might make sense there.
>>
>
> Thanks,
> Veluri.
>