Re: [Mesa-dev] [PATCH] anv: fix Get*MemoryRequirements for !LLC

[Nanley Chery]

On Tue, Feb 14, 2017 at 09:37:20AM -0800, Jason Ekstrand wrote:
> On Tue, Feb 14, 2017 at 9:35 AM, Connor Abbott  wrote:
> 
> > On Tue, Feb 14, 2017 at 12:33 PM, Jason Ekstrand 
> > wrote:
> > > On Tue, Feb 14, 2017 at 9:23 AM, Connor Abbott 
> > wrote:
> > >>
> > >> Even though we supported both coherent and non-coherent memory types, we
> > >> effectively forced apps to use the coherent types by accident. Found by
> > >> inspection, only compile tested.
> > >>
> > >> Signed-off-by: Connor Abbott 
> > >> ---
> > >> I sent this out a while ago, but it seems like it was lost.
> > >> ---
> > >>  src/intel/vulkan/anv_device.c | 14 --
> > >>  1 file changed, 8 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/src/intel/vulkan/anv_device.c
> > b/src/intel/vulkan/anv_device.c
> > >> index 46b83a3..57d70b6 100644
> > >> --- a/src/intel/vulkan/anv_device.c
> > >> +++ b/src/intel/vulkan/anv_device.c
> > >> @@ -1554,11 +1554,12 @@ VkResult anv_InvalidateMappedMemoryRanges(
> > >>  }
> > >>
> > >>  void anv_GetBufferMemoryRequirements(
> > >> -VkDevicedevice,
> > >> +VkDevice_device,
> > >>  VkBuffer_buffer,
> > >>  VkMemoryRequirements*   pMemoryRequirements)
> > >>  {
> > >> ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
> > >> +   ANV_FROM_HANDLE(anv_device, device, _device);
> > >>
> > >> /* The Vulkan spec (git aaed022) says:
> > >>  *
> > >> @@ -1567,20 +1568,21 @@ void anv_GetBufferMemoryRequirements(
> > >>  *only if the memory type `i` in the
> > >> VkPhysicalDeviceMemoryProperties
> > >>  *structure for the physical device is supported.
> > >>  *
> > >> -* We support exactly one memory type.
> > >> +* We support exactly one memory type on LLC, two on non-LLC.
> > >>  */
> > >> -   pMemoryRequirements->memoryTypeBits = 1;
> > >> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
> > >
> > >
> > > Is it two or three?  The comment and code don't match.
> >
> > It's a bitfield. See the comment above.
> 
> 
> Drp... Yeah, this looks fine.
> 
> Reviewed-by: Jason Ekstrand 
> 
> I'll run it through CI and push it.
> 
> 

Looks like this patch should fix this bug:
https://bugs.freedesktop.org/show_bug.cgi?id=97579

> > >
> > >>
> > >>
> > >> pMemoryRequirements->size = buffer->size;
> > >> pMemoryRequirements->alignment = 16;
> > >>  }
> > >>
> > >>  void anv_GetImageMemoryRequirements(
> > >> -VkDevicedevice,
> > >> +VkDevice_device,
> > >>  VkImage _image,
> > >>  VkMemoryRequirements*   pMemoryRequirements)
> > >>  {
> > >> ANV_FROM_HANDLE(anv_image, image, _image);
> > >> +   ANV_FROM_HANDLE(anv_device, device, _device);
> > >>
> > >> /* The Vulkan spec (git aaed022) says:
> > >>  *
> > >> @@ -1589,9 +1591,9 @@ void anv_GetImageMemoryRequirements(
> > >>  *only if the memory type `i` in the
> > >> VkPhysicalDeviceMemoryProperties
> > >>  *structure for the physical device is supported.
> > >>  *
> > >> -* We support exactly one memory type.
> > >> +* We support exactly one memory type on LLC, two on non-LLC.
> > >>  */
> > >> -   pMemoryRequirements->memoryTypeBits = 1;
> > >> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
> > >>
> > >> pMemoryRequirements->size = image->size;
> > >> pMemoryRequirements->alignment = image->alignment;
> > >> --
> > >> 2.9.3
> > >>
> > >> ___
> > >> 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

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: fix Get*MemoryRequirements for !LLC

[Jason Ekstrand]

On Tue, Feb 14, 2017 at 9:35 AM, Connor Abbott  wrote:

> On Tue, Feb 14, 2017 at 12:33 PM, Jason Ekstrand 
> wrote:
> > On Tue, Feb 14, 2017 at 9:23 AM, Connor Abbott 
> wrote:
> >>
> >> Even though we supported both coherent and non-coherent memory types, we
> >> effectively forced apps to use the coherent types by accident. Found by
> >> inspection, only compile tested.
> >>
> >> Signed-off-by: Connor Abbott 
> >> ---
> >> I sent this out a while ago, but it seems like it was lost.
> >> ---
> >>  src/intel/vulkan/anv_device.c | 14 --
> >>  1 file changed, 8 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> >> index 46b83a3..57d70b6 100644
> >> --- a/src/intel/vulkan/anv_device.c
> >> +++ b/src/intel/vulkan/anv_device.c
> >> @@ -1554,11 +1554,12 @@ VkResult anv_InvalidateMappedMemoryRanges(
> >>  }
> >>
> >>  void anv_GetBufferMemoryRequirements(
> >> -VkDevicedevice,
> >> +VkDevice_device,
> >>  VkBuffer_buffer,
> >>  VkMemoryRequirements*   pMemoryRequirements)
> >>  {
> >> ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
> >> +   ANV_FROM_HANDLE(anv_device, device, _device);
> >>
> >> /* The Vulkan spec (git aaed022) says:
> >>  *
> >> @@ -1567,20 +1568,21 @@ void anv_GetBufferMemoryRequirements(
> >>  *only if the memory type `i` in the
> >> VkPhysicalDeviceMemoryProperties
> >>  *structure for the physical device is supported.
> >>  *
> >> -* We support exactly one memory type.
> >> +* We support exactly one memory type on LLC, two on non-LLC.
> >>  */
> >> -   pMemoryRequirements->memoryTypeBits = 1;
> >> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
> >
> >
> > Is it two or three?  The comment and code don't match.
>
> It's a bitfield. See the comment above.


Drp... Yeah, this looks fine.

Reviewed-by: Jason Ekstrand 

I'll run it through CI and push it.


> >
> >>
> >>
> >> pMemoryRequirements->size = buffer->size;
> >> pMemoryRequirements->alignment = 16;
> >>  }
> >>
> >>  void anv_GetImageMemoryRequirements(
> >> -VkDevicedevice,
> >> +VkDevice_device,
> >>  VkImage _image,
> >>  VkMemoryRequirements*   pMemoryRequirements)
> >>  {
> >> ANV_FROM_HANDLE(anv_image, image, _image);
> >> +   ANV_FROM_HANDLE(anv_device, device, _device);
> >>
> >> /* The Vulkan spec (git aaed022) says:
> >>  *
> >> @@ -1589,9 +1591,9 @@ void anv_GetImageMemoryRequirements(
> >>  *only if the memory type `i` in the
> >> VkPhysicalDeviceMemoryProperties
> >>  *structure for the physical device is supported.
> >>  *
> >> -* We support exactly one memory type.
> >> +* We support exactly one memory type on LLC, two on non-LLC.
> >>  */
> >> -   pMemoryRequirements->memoryTypeBits = 1;
> >> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
> >>
> >> pMemoryRequirements->size = image->size;
> >> pMemoryRequirements->alignment = image->alignment;
> >> --
> >> 2.9.3
> >>
> >> ___
> >> 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

Re: [Mesa-dev] [PATCH] anv: fix Get*MemoryRequirements for !LLC

[Connor Abbott]

On Tue, Feb 14, 2017 at 12:33 PM, Jason Ekstrand  wrote:
> On Tue, Feb 14, 2017 at 9:23 AM, Connor Abbott  wrote:
>>
>> Even though we supported both coherent and non-coherent memory types, we
>> effectively forced apps to use the coherent types by accident. Found by
>> inspection, only compile tested.
>>
>> Signed-off-by: Connor Abbott 
>> ---
>> I sent this out a while ago, but it seems like it was lost.
>> ---
>>  src/intel/vulkan/anv_device.c | 14 --
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
>> index 46b83a3..57d70b6 100644
>> --- a/src/intel/vulkan/anv_device.c
>> +++ b/src/intel/vulkan/anv_device.c
>> @@ -1554,11 +1554,12 @@ VkResult anv_InvalidateMappedMemoryRanges(
>>  }
>>
>>  void anv_GetBufferMemoryRequirements(
>> -VkDevicedevice,
>> +VkDevice_device,
>>  VkBuffer_buffer,
>>  VkMemoryRequirements*   pMemoryRequirements)
>>  {
>> ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
>> +   ANV_FROM_HANDLE(anv_device, device, _device);
>>
>> /* The Vulkan spec (git aaed022) says:
>>  *
>> @@ -1567,20 +1568,21 @@ void anv_GetBufferMemoryRequirements(
>>  *only if the memory type `i` in the
>> VkPhysicalDeviceMemoryProperties
>>  *structure for the physical device is supported.
>>  *
>> -* We support exactly one memory type.
>> +* We support exactly one memory type on LLC, two on non-LLC.
>>  */
>> -   pMemoryRequirements->memoryTypeBits = 1;
>> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
>
>
> Is it two or three?  The comment and code don't match.

It's a bitfield. See the comment above.

>
>>
>>
>> pMemoryRequirements->size = buffer->size;
>> pMemoryRequirements->alignment = 16;
>>  }
>>
>>  void anv_GetImageMemoryRequirements(
>> -VkDevicedevice,
>> +VkDevice_device,
>>  VkImage _image,
>>  VkMemoryRequirements*   pMemoryRequirements)
>>  {
>> ANV_FROM_HANDLE(anv_image, image, _image);
>> +   ANV_FROM_HANDLE(anv_device, device, _device);
>>
>> /* The Vulkan spec (git aaed022) says:
>>  *
>> @@ -1589,9 +1591,9 @@ void anv_GetImageMemoryRequirements(
>>  *only if the memory type `i` in the
>> VkPhysicalDeviceMemoryProperties
>>  *structure for the physical device is supported.
>>  *
>> -* We support exactly one memory type.
>> +* We support exactly one memory type on LLC, two on non-LLC.
>>  */
>> -   pMemoryRequirements->memoryTypeBits = 1;
>> +   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
>>
>> pMemoryRequirements->size = image->size;
>> pMemoryRequirements->alignment = image->alignment;
>> --
>> 2.9.3
>>
>> ___
>> 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

[Mesa-dev] [PATCH] anv: fix Get*MemoryRequirements for !LLC

[Connor Abbott]

Even though we supported both coherent and non-coherent memory types, we
effectively forced apps to use the coherent types by accident. Found by
inspection, only compile tested.

Signed-off-by: Connor Abbott 
---
I sent this out a while ago, but it seems like it was lost.
---
 src/intel/vulkan/anv_device.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 46b83a3..57d70b6 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -1554,11 +1554,12 @@ VkResult anv_InvalidateMappedMemoryRanges(
 }
 
 void anv_GetBufferMemoryRequirements(
-VkDevicedevice,
+VkDevice_device,
 VkBuffer_buffer,
 VkMemoryRequirements*   pMemoryRequirements)
 {
ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
+   ANV_FROM_HANDLE(anv_device, device, _device);
 
/* The Vulkan spec (git aaed022) says:
 *
@@ -1567,20 +1568,21 @@ void anv_GetBufferMemoryRequirements(
 *only if the memory type `i` in the VkPhysicalDeviceMemoryProperties
 *structure for the physical device is supported.
 *
-* We support exactly one memory type.
+* We support exactly one memory type on LLC, two on non-LLC.
 */
-   pMemoryRequirements->memoryTypeBits = 1;
+   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
 
pMemoryRequirements->size = buffer->size;
pMemoryRequirements->alignment = 16;
 }
 
 void anv_GetImageMemoryRequirements(
-VkDevicedevice,
+VkDevice_device,
 VkImage _image,
 VkMemoryRequirements*   pMemoryRequirements)
 {
ANV_FROM_HANDLE(anv_image, image, _image);
+   ANV_FROM_HANDLE(anv_device, device, _device);
 
/* The Vulkan spec (git aaed022) says:
 *
@@ -1589,9 +1591,9 @@ void anv_GetImageMemoryRequirements(
 *only if the memory type `i` in the VkPhysicalDeviceMemoryProperties
 *structure for the physical device is supported.
 *
-* We support exactly one memory type.
+* We support exactly one memory type on LLC, two on non-LLC.
 */
-   pMemoryRequirements->memoryTypeBits = 1;
+   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
 
pMemoryRequirements->size = image->size;
pMemoryRequirements->alignment = image->alignment;
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] anv: fix Get*MemoryRequirements for !LLC

[Connor Abbott]

AFAIK buffers and images can be backed by coherent or non-coherent
memory types. Found by inspection, only compile tested.

Signed-off-by: Connor Abbott 
---
 src/vulkan/anv_device.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/vulkan/anv_device.c b/src/vulkan/anv_device.c
index dfc29e4..453d66f 100644
--- a/src/vulkan/anv_device.c
+++ b/src/vulkan/anv_device.c
@@ -1259,11 +1259,12 @@ VkResult anv_InvalidateMappedMemoryRanges(
 }
 
 void anv_GetBufferMemoryRequirements(
-VkDevicedevice,
+VkDevice_device,
 VkBuffer_buffer,
 VkMemoryRequirements*   pMemoryRequirements)
 {
ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
+   ANV_FROM_HANDLE(anv_device, device, _device);
 
/* The Vulkan spec (git aaed022) says:
 *
@@ -1272,20 +1273,21 @@ void anv_GetBufferMemoryRequirements(
 *only if the memory type `i` in the VkPhysicalDeviceMemoryProperties
 *structure for the physical device is supported.
 *
-* We support exactly one memory type.
+* We support exactly one memory type on LLC, two on non-LLC.
 */
-   pMemoryRequirements->memoryTypeBits = 1;
+   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
 
pMemoryRequirements->size = buffer->size;
pMemoryRequirements->alignment = 16;
 }
 
 void anv_GetImageMemoryRequirements(
-VkDevicedevice,
+VkDevice_device,
 VkImage _image,
 VkMemoryRequirements*   pMemoryRequirements)
 {
ANV_FROM_HANDLE(anv_image, image, _image);
+   ANV_FROM_HANDLE(anv_device, device, _device);
 
/* The Vulkan spec (git aaed022) says:
 *
@@ -1294,9 +1296,9 @@ void anv_GetImageMemoryRequirements(
 *only if the memory type `i` in the VkPhysicalDeviceMemoryProperties
 *structure for the physical device is supported.
 *
-* We support exactly one memory type.
+* We support exactly one memory type on LLC, two on non-LLC.
 */
-   pMemoryRequirements->memoryTypeBits = 1;
+   pMemoryRequirements->memoryTypeBits = device->info.has_llc ? 1 : 3;
 
pMemoryRequirements->size = image->size;
pMemoryRequirements->alignment = image->alignment;
-- 
2.4.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev