| Message ID | 20181012203844.29891-1-keithp@keithp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | radv: Allow physical device interfaces to be included in device extensions | expand |
On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <keithp@keithp.com> wrote: > > According to the Vulkan spec: > > "Vulkan 1.0 initially required all new physical-device-level extension > functionality to be structured within an instance extension. In order > to avoid using an instance extension, which often requires loader > support, physical-device-level extension functionality may be > implemented within device extensions" > > The code that checks for enabled extension APIs currently only passes > functions with VkDevice or VkCommandBuffer as their first > argument. This patch extends that to also allow functions with > VkPhysicalDevice parameters, in support of the above quote from the > Vulkan spec. > Also "To obtain a function pointer for a physical-device-level command from a device extension, an application can use vkGetInstanceProcAddr. " As far as I can tell the device_command member is only to make sure we return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2) we still have to return NULL there for functions which take VkPhysicalDevice? So the old behavior seems correct to me. > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py > index 377b544c2aa..69e6fc3e0eb 100644 > --- a/src/amd/vulkan/radv_entrypoints_gen.py > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): > self.return_type = return_type > self.params = params > self.guard = guard > - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') > + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') > > def prefixed_name(self, prefix): > assert self.name.startswith('vk') > -- > 2.19.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> wrote: > On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <keithp@keithp.com> wrote: > > > > According to the Vulkan spec: > > > > "Vulkan 1.0 initially required all new physical-device-level extension > > functionality to be structured within an instance extension. In order > > to avoid using an instance extension, which often requires loader > > support, physical-device-level extension functionality may be > > implemented within device extensions" > > > > The code that checks for enabled extension APIs currently only passes > > functions with VkDevice or VkCommandBuffer as their first > > argument. This patch extends that to also allow functions with > > VkPhysicalDevice parameters, in support of the above quote from the > > Vulkan spec. > > > > Also "To obtain a function pointer for a physical-device-level command > from a device extension, an application can use vkGetInstanceProcAddr. > " > > As far as I can tell the device_command member is only to make sure we > return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2) > we still have to return NULL there for functions which take > VkPhysicalDevice? So the old behavior seems correct to me. > I think anv is ignoring that line in the table which is why it works for us. If only someone wrote tests for these things... I think the correct interpretation would be that any physical device functions that are part of a core version or instance extension should yield NULL but any physical device functions from a device extension should return a valid function pointer. Sadly, that behavior is kind-of a pain to implement. :-( > > Signed-off-by: Keith Packard <keithp@keithp.com> > > --- > > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py > b/src/amd/vulkan/radv_entrypoints_gen.py > > index 377b544c2aa..69e6fc3e0eb 100644 > > --- a/src/amd/vulkan/radv_entrypoints_gen.py > > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): > > self.return_type = return_type > > self.params = params > > self.guard = guard > > - self.device_command = len(params) > 0 and (params[0].type == > 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == > 'VkCommandBuffer') > > + self.device_command = len(params) > 0 and (params[0].type == > 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == > 'VkQueue' or params[0].type == 'VkCommandBuffer') > > > > def prefixed_name(self, prefix): > > assert self.name.startswith('vk') > > -- > > 2.19.1 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > <div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>> wrote:<br> ><br> > According to the Vulkan spec:<br> ><br> > "Vulkan 1.0 initially required all new physical-device-level extension<br> > functionality to be structured within an instance extension. In order<br> > to avoid using an instance extension, which often requires loader<br> > support, physical-device-level extension functionality may be<br> > implemented within device extensions"<br> ><br> > The code that checks for enabled extension APIs currently only passes<br> > functions with VkDevice or VkCommandBuffer as their first<br> > argument. This patch extends that to also allow functions with<br> > VkPhysicalDevice parameters, in support of the above quote from the<br> > Vulkan spec.<br> ><br> <br> Also "To obtain a function pointer for a physical-device-level command<br> from a device extension, an application can use vkGetInstanceProcAddr.<br> "<br> <br> As far as I can tell the device_command member is only to make sure we<br> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2)<br> we still have to return NULL there for functions which take<br> VkPhysicalDevice? So the old behavior seems correct to me.<br></blockquote><div><br></div><div>I think anv is ignoring that line in the table which is why it works for us. If only someone wrote tests for these things...</div><div><br></div><div>I think the correct interpretation would be that any physical device functions that are part of a core version or instance extension should yield NULL but any physical device functions from a device extension should return a valid function pointer. Sadly, that behavior is kind-of a pain to implement. :-(<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > Signed-off-by: Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>><br> > ---<br> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +-<br> > 1 file changed, 1 insertion(+), 1 deletion(-)<br> ><br> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py<br> > index 377b544c2aa..69e6fc3e0eb 100644<br> > --- a/src/amd/vulkan/radv_entrypoints_gen.py<br> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py<br> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase):<br> > self.return_type = return_type<br> > self.params = params<br> > self.guard = guard<br> > - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> > + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> ><br> > def prefixed_name(self, prefix):<br> > assert self.name.startswith('vk')<br> > --<br> > 2.19.1<br> ><br> > _______________________________________________<br> > mesa-dev mailing list<br> > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> _______________________________________________<br> mesa-dev mailing list<br> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> </blockquote></div></div>
On Sat, Oct 13, 2018 at 6:12 PM Jason Ekstrand <jason@jlekstrand.net> wrote: > > On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> wrote: >> >> On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <keithp@keithp.com> wrote: >> > >> > According to the Vulkan spec: >> > >> > "Vulkan 1.0 initially required all new physical-device-level extension >> > functionality to be structured within an instance extension. In order >> > to avoid using an instance extension, which often requires loader >> > support, physical-device-level extension functionality may be >> > implemented within device extensions" >> > >> > The code that checks for enabled extension APIs currently only passes >> > functions with VkDevice or VkCommandBuffer as their first >> > argument. This patch extends that to also allow functions with >> > VkPhysicalDevice parameters, in support of the above quote from the >> > Vulkan spec. >> > >> >> Also "To obtain a function pointer for a physical-device-level command >> from a device extension, an application can use vkGetInstanceProcAddr. >> " >> >> As far as I can tell the device_command member is only to make sure we >> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2) >> we still have to return NULL there for functions which take >> VkPhysicalDevice? So the old behavior seems correct to me. > > > I think anv is ignoring that line in the table which is why it works for us. If only someone wrote tests for these things... > > I think the correct interpretation would be that any physical device functions that are part of a core version or instance extension should yield NULL but any physical device functions from a device extension should return a valid function pointer. Sadly, that behavior is kind-of a pain to implement. :-( How would you read that into the spec? As quoted above it explicitly tells you to use vkGetInstanceProcAddr for VkPhysicalDevice functions, even if they are based on a device extension. (And you cannot really use vkGetDeviceProcAddr anyway as the typical usecase is before you've created a device). > >> >> > Signed-off-by: Keith Packard <keithp@keithp.com> >> > --- >> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py >> > index 377b544c2aa..69e6fc3e0eb 100644 >> > --- a/src/amd/vulkan/radv_entrypoints_gen.py >> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py >> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): >> > self.return_type = return_type >> > self.params = params >> > self.guard = guard >> > - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') >> > + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') >> > >> > def prefixed_name(self, prefix): >> > assert self.name.startswith('vk') >> > -- >> > 2.19.1 >> > >> > _______________________________________________ >> > mesa-dev mailing list >> > mesa-dev@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Sat, Oct 13, 2018 at 11:24 AM Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> wrote: > On Sat, Oct 13, 2018 at 6:12 PM Jason Ekstrand <jason@jlekstrand.net> > wrote: > > > > On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen < > bas@basnieuwenhuizen.nl> wrote: > >> > >> On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <keithp@keithp.com> > wrote: > >> > > >> > According to the Vulkan spec: > >> > > >> > "Vulkan 1.0 initially required all new physical-device-level extension > >> > functionality to be structured within an instance extension. In order > >> > to avoid using an instance extension, which often requires loader > >> > support, physical-device-level extension functionality may be > >> > implemented within device extensions" > >> > > >> > The code that checks for enabled extension APIs currently only passes > >> > functions with VkDevice or VkCommandBuffer as their first > >> > argument. This patch extends that to also allow functions with > >> > VkPhysicalDevice parameters, in support of the above quote from the > >> > Vulkan spec. > >> > > >> > >> Also "To obtain a function pointer for a physical-device-level command > >> from a device extension, an application can use vkGetInstanceProcAddr. > >> " > >> > >> As far as I can tell the device_command member is only to make sure we > >> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2) > >> we still have to return NULL there for functions which take > >> VkPhysicalDevice? So the old behavior seems correct to me. > > > > > > I think anv is ignoring that line in the table which is why it works for > us. If only someone wrote tests for these things... > > > > I think the correct interpretation would be that any physical device > functions that are part of a core version or instance extension should > yield NULL but any physical device functions from a device extension should > return a valid function pointer. Sadly, that behavior is kind-of a pain to > implement. :-( > > How would you read that into the spec? As quoted above it explicitly > tells you to use vkGetInstanceProcAddr for VkPhysicalDevice functions, > even if they are based on a device extension. (And you cannot really > use vkGetDeviceProcAddr anyway as the typical usecase is before you've > created a device). > Because I was reading the wrong chunk of spec. :-( You are correct and radv is like doing the right thing and anv is doing the wrong thing. --Jason > > > >> > >> > Signed-off-by: Keith Packard <keithp@keithp.com> > >> > --- > >> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py > b/src/amd/vulkan/radv_entrypoints_gen.py > >> > index 377b544c2aa..69e6fc3e0eb 100644 > >> > --- a/src/amd/vulkan/radv_entrypoints_gen.py > >> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > >> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): > >> > self.return_type = return_type > >> > self.params = params > >> > self.guard = guard > >> > - self.device_command = len(params) > 0 and (params[0].type == > 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == > 'VkCommandBuffer') > >> > + self.device_command = len(params) > 0 and (params[0].type == > 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == > 'VkQueue' or params[0].type == 'VkCommandBuffer') > >> > > >> > def prefixed_name(self, prefix): > >> > assert self.name.startswith('vk') > >> > -- > >> > 2.19.1 > >> > > >> > _______________________________________________ > >> > mesa-dev mailing list > >> > mesa-dev@lists.freedesktop.org > >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > >> _______________________________________________ > >> mesa-dev mailing list > >> mesa-dev@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > <div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Oct 13, 2018 at 11:24 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, Oct 13, 2018 at 6:12 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br> ><br> > On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> wrote:<br> >><br> >> On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>> wrote:<br> >> ><br> >> > According to the Vulkan spec:<br> >> ><br> >> > "Vulkan 1.0 initially required all new physical-device-level extension<br> >> > functionality to be structured within an instance extension. In order<br> >> > to avoid using an instance extension, which often requires loader<br> >> > support, physical-device-level extension functionality may be<br> >> > implemented within device extensions"<br> >> ><br> >> > The code that checks for enabled extension APIs currently only passes<br> >> > functions with VkDevice or VkCommandBuffer as their first<br> >> > argument. This patch extends that to also allow functions with<br> >> > VkPhysicalDevice parameters, in support of the above quote from the<br> >> > Vulkan spec.<br> >> ><br> >><br> >> Also "To obtain a function pointer for a physical-device-level command<br> >> from a device extension, an application can use vkGetInstanceProcAddr.<br> >> "<br> >><br> >> As far as I can tell the device_command member is only to make sure we<br> >> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2)<br> >> we still have to return NULL there for functions which take<br> >> VkPhysicalDevice? So the old behavior seems correct to me.<br> ><br> ><br> > I think anv is ignoring that line in the table which is why it works for us. If only someone wrote tests for these things...<br> ><br> > I think the correct interpretation would be that any physical device functions that are part of a core version or instance extension should yield NULL but any physical device functions from a device extension should return a valid function pointer. Sadly, that behavior is kind-of a pain to implement. :-(<br> <br> How would you read that into the spec? As quoted above it explicitly<br> tells you to use vkGetInstanceProcAddr for VkPhysicalDevice functions,<br> even if they are based on a device extension. (And you cannot really<br> use vkGetDeviceProcAddr anyway as the typical usecase is before you've<br> created a device).<br></blockquote><div><br></div><div>Because I was reading the wrong chunk of spec. :-( You are correct and radv is like doing the right thing and anv is doing the wrong thing.</div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> ><br> >><br> >> > Signed-off-by: Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>><br> >> > ---<br> >> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +-<br> >> > 1 file changed, 1 insertion(+), 1 deletion(-)<br> >> ><br> >> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > index 377b544c2aa..69e6fc3e0eb 100644<br> >> > --- a/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase):<br> >> > self.return_type = return_type<br> >> > self.params = params<br> >> > self.guard = guard<br> >> > - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> >> > + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> >> ><br> >> > def prefixed_name(self, prefix):<br> >> > assert self.name.startswith('vk')<br> >> > --<br> >> > 2.19.1<br> >> ><br> >> > _______________________________________________<br> >> > mesa-dev mailing list<br> >> > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> >> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> >> _______________________________________________<br> >> mesa-dev mailing list<br> >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> </blockquote></div></div>
On Sat, Oct 13, 2018 at 11:27 AM Jason Ekstrand <jason@jlekstrand.net> wrote: > On Sat, Oct 13, 2018 at 11:24 AM Bas Nieuwenhuizen < > bas@basnieuwenhuizen.nl> wrote: > >> On Sat, Oct 13, 2018 at 6:12 PM Jason Ekstrand <jason@jlekstrand.net> >> wrote: >> > >> > On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen < >> bas@basnieuwenhuizen.nl> wrote: >> >> >> >> On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <keithp@keithp.com> >> wrote: >> >> > >> >> > According to the Vulkan spec: >> >> > >> >> > "Vulkan 1.0 initially required all new physical-device-level >> extension >> >> > functionality to be structured within an instance extension. In >> order >> >> > to avoid using an instance extension, which often requires loader >> >> > support, physical-device-level extension functionality may be >> >> > implemented within device extensions" >> >> > >> >> > The code that checks for enabled extension APIs currently only passes >> >> > functions with VkDevice or VkCommandBuffer as their first >> >> > argument. This patch extends that to also allow functions with >> >> > VkPhysicalDevice parameters, in support of the above quote from the >> >> > Vulkan spec. >> >> > >> >> >> >> Also "To obtain a function pointer for a physical-device-level command >> >> from a device extension, an application can use vkGetInstanceProcAddr. >> >> " >> >> >> >> As far as I can tell the device_command member is only to make sure we >> >> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2) >> >> we still have to return NULL there for functions which take >> >> VkPhysicalDevice? So the old behavior seems correct to me. >> > >> > >> > I think anv is ignoring that line in the table which is why it works >> for us. If only someone wrote tests for these things... >> > >> > I think the correct interpretation would be that any physical device >> functions that are part of a core version or instance extension should >> yield NULL but any physical device functions from a device extension should >> return a valid function pointer. Sadly, that behavior is kind-of a pain to >> implement. :-( >> >> How would you read that into the spec? As quoted above it explicitly >> tells you to use vkGetInstanceProcAddr for VkPhysicalDevice functions, >> even if they are based on a device extension. (And you cannot really >> use vkGetDeviceProcAddr anyway as the typical usecase is before you've >> created a device). >> > > Because I was reading the wrong chunk of spec. :-( You are correct and > radv is like doing the right thing and anv is doing the wrong thing. > Actually, I think anv is doing the right thing too. Now I have no idea why Keith was having problems. --Jason > > >> > >> >> >> >> > Signed-off-by: Keith Packard <keithp@keithp.com> >> >> > --- >> >> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > >> >> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py >> b/src/amd/vulkan/radv_entrypoints_gen.py >> >> > index 377b544c2aa..69e6fc3e0eb 100644 >> >> > --- a/src/amd/vulkan/radv_entrypoints_gen.py >> >> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py >> >> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): >> >> > self.return_type = return_type >> >> > self.params = params >> >> > self.guard = guard >> >> > - self.device_command = len(params) > 0 and (params[0].type >> == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == >> 'VkCommandBuffer') >> >> > + self.device_command = len(params) > 0 and (params[0].type >> == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == >> 'VkQueue' or params[0].type == 'VkCommandBuffer') >> >> > >> >> > def prefixed_name(self, prefix): >> >> > assert self.name.startswith('vk') >> >> > -- >> >> > 2.19.1 >> >> > >> >> > _______________________________________________ >> >> > mesa-dev mailing list >> >> > mesa-dev@lists.freedesktop.org >> >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> _______________________________________________ >> >> mesa-dev mailing list >> >> mesa-dev@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > <div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Oct 13, 2018 at 11:27 AM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Oct 13, 2018 at 11:24 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, Oct 13, 2018 at 6:12 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br> ><br> > On Sat, Oct 13, 2018 at 10:58 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> wrote:<br> >><br> >> On Fri, Oct 12, 2018 at 10:38 PM Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>> wrote:<br> >> ><br> >> > According to the Vulkan spec:<br> >> ><br> >> > "Vulkan 1.0 initially required all new physical-device-level extension<br> >> > functionality to be structured within an instance extension. In order<br> >> > to avoid using an instance extension, which often requires loader<br> >> > support, physical-device-level extension functionality may be<br> >> > implemented within device extensions"<br> >> ><br> >> > The code that checks for enabled extension APIs currently only passes<br> >> > functions with VkDevice or VkCommandBuffer as their first<br> >> > argument. This patch extends that to also allow functions with<br> >> > VkPhysicalDevice parameters, in support of the above quote from the<br> >> > Vulkan spec.<br> >> ><br> >><br> >> Also "To obtain a function pointer for a physical-device-level command<br> >> from a device extension, an application can use vkGetInstanceProcAddr.<br> >> "<br> >><br> >> As far as I can tell the device_command member is only to make sure we<br> >> return NULL from vkGetDeviceProcAddr, and per the spec (3.1 table 2)<br> >> we still have to return NULL there for functions which take<br> >> VkPhysicalDevice? So the old behavior seems correct to me.<br> ><br> ><br> > I think anv is ignoring that line in the table which is why it works for us. If only someone wrote tests for these things...<br> ><br> > I think the correct interpretation would be that any physical device functions that are part of a core version or instance extension should yield NULL but any physical device functions from a device extension should return a valid function pointer. Sadly, that behavior is kind-of a pain to implement. :-(<br> <br> How would you read that into the spec? As quoted above it explicitly<br> tells you to use vkGetInstanceProcAddr for VkPhysicalDevice functions,<br> even if they are based on a device extension. (And you cannot really<br> use vkGetDeviceProcAddr anyway as the typical usecase is before you've<br> created a device).<br></blockquote><div><br></div><div>Because I was reading the wrong chunk of spec. :-( You are correct and radv is like doing the right thing and anv is doing the wrong thing.</div></div></div></blockquote><div><br></div><div>Actually, I think anv is doing the right thing too. Now I have no idea why Keith was having problems.</div><div><br></div><div>--Jason</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> ><br> >><br> >> > Signed-off-by: Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>><br> >> > ---<br> >> > src/amd/vulkan/radv_entrypoints_gen.py | 2 +-<br> >> > 1 file changed, 1 insertion(+), 1 deletion(-)<br> >> ><br> >> > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > index 377b544c2aa..69e6fc3e0eb 100644<br> >> > --- a/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > +++ b/src/amd/vulkan/radv_entrypoints_gen.py<br> >> > @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase):<br> >> > self.return_type = return_type<br> >> > self.params = params<br> >> > self.guard = guard<br> >> > - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> >> > + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer')<br> >> ><br> >> > def prefixed_name(self, prefix):<br> >> > assert self.name.startswith('vk')<br> >> > --<br> >> > 2.19.1<br> >> ><br> >> > _______________________________________________<br> >> > mesa-dev mailing list<br> >> > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> >> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> >> _______________________________________________<br> >> mesa-dev mailing list<br> >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> </blockquote></div></div> </blockquote></div></div>
Jason Ekstrand <jason@jlekstrand.net> writes: > Actually, I think anv is doing the right thing too. Now I have no idea why > Keith was having problems. anv is happily returning a valid pointer and radv is not? In any case, I've switched to using vkGetInstanceProcAddr for the VkPhysicalDevice function and it works fine with both drivers.
On October 13, 2018 19:50:00 "Keith Packard" <keithp@keithp.com> wrote: > Jason Ekstrand <jason@jlekstrand.net> writes: > >> Actually, I think anv is doing the right thing too. Now I have no idea why >> Keith was having problems. > > anv is happily returning a valid pointer and radv is not? > > In any case, I've switched to using vkGetInstanceProcAddr for the > VkPhysicalDevice function and it works fine with both drivers. Using vkGetInstanceProcAddr is the right thing to do. The fact that anv allows you to is a bug which should be investigated and fixed. I looked at it a bit today and I'm still not sure how it's happening. --Jason
diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py index 377b544c2aa..69e6fc3e0eb 100644 --- a/src/amd/vulkan/radv_entrypoints_gen.py +++ b/src/amd/vulkan/radv_entrypoints_gen.py @@ -352,7 +352,7 @@ class Entrypoint(EntrypointBase): self.return_type = return_type self.params = params self.guard = guard - self.device_command = len(params) > 0 and (params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') + self.device_command = len(params) > 0 and (params[0].type == 'VkPhysicalDevice' or params[0].type == 'VkDevice' or params[0].type == 'VkQueue' or params[0].type == 'VkCommandBuffer') def prefixed_name(self, prefix): assert self.name.startswith('vk')
According to the Vulkan spec: "Vulkan 1.0 initially required all new physical-device-level extension functionality to be structured within an instance extension. In order to avoid using an instance extension, which often requires loader support, physical-device-level extension functionality may be implemented within device extensions" The code that checks for enabled extension APIs currently only passes functions with VkDevice or VkCommandBuffer as their first argument. This patch extends that to also allow functions with VkPhysicalDevice parameters, in support of the above quote from the Vulkan spec. Signed-off-by: Keith Packard <keithp@keithp.com> --- src/amd/vulkan/radv_entrypoints_gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)