Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
Hi, On 17-05-17 14:57, Emil Velikov wrote: On 16 May 2017 at 22:42, Hans de Goede wrote: Hi, On 05/16/2017 07:51 PM, Emil Velikov wrote: Hi Hans Please poke if patches fall through the cracks. On 20 March 2017 at 11:05, Hans de Goede wrote: Together with some fixes to xdriinfo this fixes xdriinfo not working with glvnd. Since apps (xdriinfo) expect GetDriverConfig to work without going to need through the dance to setup a glxcontext (which is a reasonable expectation IMHO), the dispatch for this ends up significantly different then any other dispatch function. This patch gets the job done, but I'm not really happy with how this patch turned out, suggestions for a better fix are welcome. Cc: Kyle Brenneman Signed-off-by: Hans de Goede --- src/glx/g_glxglvnddispatchfuncs.c | 18 ++ src/glx/g_glxglvnddispatchindices.h | 1 + 2 files changed, 19 insertions(+) diff --git a/src/glx/g_glxglvnddispatchfuncs.c b/src/glx/g_glxglvnddispatchfuncs.c index b5e3398..040cdf8 100644 --- a/src/glx/g_glxglvnddispatchfuncs.c +++ b/src/glx/g_glxglvnddispatchfuncs.c @@ -4,6 +4,7 @@ */ #include +#include "glxclient.h" #include "glxglvnd.h" #include "glxglvnddispatchfuncs.h" #include "g_glxglvnddispatchindices.h" @@ -50,6 +51,7 @@ const char * const __glXDispatchTableStrings[DI_LAST_INDEX] = { __ATTRIB(GetCurrentDisplayEXT), // glXGetCurrentDrawable implemented by libglvnd // glXGetCurrentReadDrawable implemented by libglvnd +__ATTRIB(GetDriverConfig), Back in Nov 2016 we had a chat with Adam and if I understood things correctly the idea was to kill off the following: glXGetScreenDriver glXGetDriverConfig Neither of those is part of an extension and the only user xdriinfo, is not that useful. Would be great if distributions let it to rest and we don't have to worry about it ever being around ;-) The problem is that driconf uses them and people actually use driconf, see: https://bugzilla.redhat.com/show_bug.cgi?id=1429894 Ouch, had not idea people are still using that behemoth :-( In that case we cannot nuke the API, so let's merge this patch. Small nit: please drop the compilation guard. Those should be always true when compiling the file. Kyle/others - the GLVND bits are in, aren't they? Do you have any comments on the patch? With that the nit Reviewed-by: Emil Velikov Cc: mesa-sta...@lists.freedesktop.org Thank you, pushed with the nit fixed and the r-b and cc added. Regards, Hans ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
On 05/17/2017 12:50 PM, Emil Velikov wrote: On 17 May 2017 at 17:05, Kyle Brenneman wrote: The patch assumes that glXGetDriverConfig will only ever be implemented by Mesa. As long as that's a safe assumption, the change looks right to me. Can you elaborate a bit more here? Both the AMDGPU-PRO stack and the ImgTec driver used in CrOS uses Mesa as a base, thus they export the function. Admittedly the latter does not use GLX, AFAICT. -Emil The GLVND dispatch functions (dispatch_GetDriverConfig in this case) are what gets returned to the application from glXGetProcAddress. Normally, they'd figure out a vendor library based on the function parameters and then look up that vendor's implementation. In this case, though, the dispatch function calls Mesa's glXGetDriverConfig function directly. That said, unless it can figure out a display and screen number from the driver name, I don't think there's much else that the dispatch function could do. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
On 17 May 2017 at 17:05, Kyle Brenneman wrote: > The patch assumes that glXGetDriverConfig will only ever be implemented by > Mesa. As long as that's a safe assumption, the change looks right to me. Can you elaborate a bit more here? Both the AMDGPU-PRO stack and the ImgTec driver used in CrOS uses Mesa as a base, thus they export the function. Admittedly the latter does not use GLX, AFAICT. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
On 05/17/2017 06:57 AM, Emil Velikov wrote: On 16 May 2017 at 22:42, Hans de Goede wrote: Hi, On 05/16/2017 07:51 PM, Emil Velikov wrote: Hi Hans Please poke if patches fall through the cracks. On 20 March 2017 at 11:05, Hans de Goede wrote: Together with some fixes to xdriinfo this fixes xdriinfo not working with glvnd. Since apps (xdriinfo) expect GetDriverConfig to work without going to need through the dance to setup a glxcontext (which is a reasonable expectation IMHO), the dispatch for this ends up significantly different then any other dispatch function. This patch gets the job done, but I'm not really happy with how this patch turned out, suggestions for a better fix are welcome. Cc: Kyle Brenneman Signed-off-by: Hans de Goede --- src/glx/g_glxglvnddispatchfuncs.c | 18 ++ src/glx/g_glxglvnddispatchindices.h | 1 + 2 files changed, 19 insertions(+) diff --git a/src/glx/g_glxglvnddispatchfuncs.c b/src/glx/g_glxglvnddispatchfuncs.c index b5e3398..040cdf8 100644 --- a/src/glx/g_glxglvnddispatchfuncs.c +++ b/src/glx/g_glxglvnddispatchfuncs.c @@ -4,6 +4,7 @@ */ #include +#include "glxclient.h" #include "glxglvnd.h" #include "glxglvnddispatchfuncs.h" #include "g_glxglvnddispatchindices.h" @@ -50,6 +51,7 @@ const char * const __glXDispatchTableStrings[DI_LAST_INDEX] = { __ATTRIB(GetCurrentDisplayEXT), // glXGetCurrentDrawable implemented by libglvnd // glXGetCurrentReadDrawable implemented by libglvnd +__ATTRIB(GetDriverConfig), Back in Nov 2016 we had a chat with Adam and if I understood things correctly the idea was to kill off the following: glXGetScreenDriver glXGetDriverConfig Neither of those is part of an extension and the only user xdriinfo, is not that useful. Would be great if distributions let it to rest and we don't have to worry about it ever being around ;-) The problem is that driconf uses them and people actually use driconf, see: https://bugzilla.redhat.com/show_bug.cgi?id=1429894 Ouch, had not idea people are still using that behemoth :-( In that case we cannot nuke the API, so let's merge this patch. Small nit: please drop the compilation guard. Those should be always true when compiling the file. Kyle/others - the GLVND bits are in, aren't they? Do you have any comments on the patch? With that the nit Reviewed-by: Emil Velikov Cc: mesa-sta...@lists.freedesktop.org Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev The patch assumes that glXGetDriverConfig will only ever be implemented by Mesa. As long as that's a safe assumption, the change looks right to me. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
On 16 May 2017 at 22:42, Hans de Goede wrote: > Hi, > > > On 05/16/2017 07:51 PM, Emil Velikov wrote: >> >> Hi Hans >> >> Please poke if patches fall through the cracks. >> >> On 20 March 2017 at 11:05, Hans de Goede wrote: >>> >>> Together with some fixes to xdriinfo this fixes xdriinfo not working >>> with glvnd. >>> >>> Since apps (xdriinfo) expect GetDriverConfig to work without going to >>> need through the dance to setup a glxcontext (which is a reasonable >>> expectation IMHO), the dispatch for this ends up significantly different >>> then any other dispatch function. >>> >>> This patch gets the job done, but I'm not really happy with how this >>> patch turned out, suggestions for a better fix are welcome. >>> >>> Cc: Kyle Brenneman >>> Signed-off-by: Hans de Goede >>> --- >>> src/glx/g_glxglvnddispatchfuncs.c | 18 ++ >>> src/glx/g_glxglvnddispatchindices.h | 1 + >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/src/glx/g_glxglvnddispatchfuncs.c >>> b/src/glx/g_glxglvnddispatchfuncs.c >>> index b5e3398..040cdf8 100644 >>> --- a/src/glx/g_glxglvnddispatchfuncs.c >>> +++ b/src/glx/g_glxglvnddispatchfuncs.c >>> @@ -4,6 +4,7 @@ >>> */ >>> #include >>> >>> +#include "glxclient.h" >>> #include "glxglvnd.h" >>> #include "glxglvnddispatchfuncs.h" >>> #include "g_glxglvnddispatchindices.h" >>> @@ -50,6 +51,7 @@ const char * const >>> __glXDispatchTableStrings[DI_LAST_INDEX] = { >>> __ATTRIB(GetCurrentDisplayEXT), >>> // glXGetCurrentDrawable implemented by libglvnd >>> // glXGetCurrentReadDrawable implemented by libglvnd >>> +__ATTRIB(GetDriverConfig), >> >> >> Back in Nov 2016 we had a chat with Adam and if I understood things >> correctly the idea was to kill off the following: >> >> glXGetScreenDriver >> glXGetDriverConfig >> >> Neither of those is part of an extension and the only user xdriinfo, >> is not that useful. >> Would be great if distributions let it to rest and we don't have to >> worry about it ever being around ;-) > > > The problem is that driconf uses them and people actually use driconf, > see: https://bugzilla.redhat.com/show_bug.cgi?id=1429894 > Ouch, had not idea people are still using that behemoth :-( In that case we cannot nuke the API, so let's merge this patch. Small nit: please drop the compilation guard. Those should be always true when compiling the file. Kyle/others - the GLVND bits are in, aren't they? Do you have any comments on the patch? With that the nit Reviewed-by: Emil Velikov Cc: mesa-sta...@lists.freedesktop.org Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
Hi, On 05/16/2017 07:51 PM, Emil Velikov wrote: Hi Hans Please poke if patches fall through the cracks. On 20 March 2017 at 11:05, Hans de Goede wrote: Together with some fixes to xdriinfo this fixes xdriinfo not working with glvnd. Since apps (xdriinfo) expect GetDriverConfig to work without going to need through the dance to setup a glxcontext (which is a reasonable expectation IMHO), the dispatch for this ends up significantly different then any other dispatch function. This patch gets the job done, but I'm not really happy with how this patch turned out, suggestions for a better fix are welcome. Cc: Kyle Brenneman Signed-off-by: Hans de Goede --- src/glx/g_glxglvnddispatchfuncs.c | 18 ++ src/glx/g_glxglvnddispatchindices.h | 1 + 2 files changed, 19 insertions(+) diff --git a/src/glx/g_glxglvnddispatchfuncs.c b/src/glx/g_glxglvnddispatchfuncs.c index b5e3398..040cdf8 100644 --- a/src/glx/g_glxglvnddispatchfuncs.c +++ b/src/glx/g_glxglvnddispatchfuncs.c @@ -4,6 +4,7 @@ */ #include +#include "glxclient.h" #include "glxglvnd.h" #include "glxglvnddispatchfuncs.h" #include "g_glxglvnddispatchindices.h" @@ -50,6 +51,7 @@ const char * const __glXDispatchTableStrings[DI_LAST_INDEX] = { __ATTRIB(GetCurrentDisplayEXT), // glXGetCurrentDrawable implemented by libglvnd // glXGetCurrentReadDrawable implemented by libglvnd +__ATTRIB(GetDriverConfig), Back in Nov 2016 we had a chat with Adam and if I understood things correctly the idea was to kill off the following: glXGetScreenDriver glXGetDriverConfig Neither of those is part of an extension and the only user xdriinfo, is not that useful. Would be great if distributions let it to rest and we don't have to worry about it ever being around ;-) The problem is that driconf uses them and people actually use driconf, see: https://bugzilla.redhat.com/show_bug.cgi?id=1429894 Regards, Hans ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glxglvnddispatch: Add missing dispatch for GetDriverConfig
Hi Hans Please poke if patches fall through the cracks. On 20 March 2017 at 11:05, Hans de Goede wrote: > Together with some fixes to xdriinfo this fixes xdriinfo not working > with glvnd. > > Since apps (xdriinfo) expect GetDriverConfig to work without going to > need through the dance to setup a glxcontext (which is a reasonable > expectation IMHO), the dispatch for this ends up significantly different > then any other dispatch function. > > This patch gets the job done, but I'm not really happy with how this > patch turned out, suggestions for a better fix are welcome. > > Cc: Kyle Brenneman > Signed-off-by: Hans de Goede > --- > src/glx/g_glxglvnddispatchfuncs.c | 18 ++ > src/glx/g_glxglvnddispatchindices.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/src/glx/g_glxglvnddispatchfuncs.c > b/src/glx/g_glxglvnddispatchfuncs.c > index b5e3398..040cdf8 100644 > --- a/src/glx/g_glxglvnddispatchfuncs.c > +++ b/src/glx/g_glxglvnddispatchfuncs.c > @@ -4,6 +4,7 @@ > */ > #include > > +#include "glxclient.h" > #include "glxglvnd.h" > #include "glxglvnddispatchfuncs.h" > #include "g_glxglvnddispatchindices.h" > @@ -50,6 +51,7 @@ const char * const __glXDispatchTableStrings[DI_LAST_INDEX] > = { > __ATTRIB(GetCurrentDisplayEXT), > // glXGetCurrentDrawable implemented by libglvnd > // glXGetCurrentReadDrawable implemented by libglvnd > +__ATTRIB(GetDriverConfig), Back in Nov 2016 we had a chat with Adam and if I understood things correctly the idea was to kill off the following: glXGetScreenDriver glXGetDriverConfig Neither of those is part of an extension and the only user xdriinfo, is not that useful. Would be great if distributions let it to rest and we don't have to worry about it ever being around ;-) -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev