[1/2] anv/android: Set the BO flags in bo_cache_import

Submitted by Mauro Rossi on June 3, 2018, 6:41 p.m.

Details

Message ID 20180603184159.14847-1-issor.oruam@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mauro Rossi June 3, 2018, 6:41 p.m.
Change to avoid building error:

external/mesa/src/intel/vulkan/anv_android.c:131:72:
error: too few arguments to function call, expected 5, have 4
   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
            ~~~~~~~~~~~~~~~~~~~                                        ^
1 error generated.

Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in bo_cache_alloc/import")
Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
---
 src/intel/vulkan/anv_android.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
index 7e07dbaaa4..ed5da5b537 100644
--- a/src/intel/vulkan/anv_android.c
+++ b/src/intel/vulkan/anv_android.c
@@ -128,7 +128,7 @@  anv_image_from_gralloc(VkDevice device_h,
     */
    int dma_buf = gralloc_info->handle->data[0];
 
-   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
+   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, 0, &bo);
    if (result != VK_SUCCESS) {
       return vk_errorf(device->instance, device, result,
                        "failed to import dma-buf from VkNativeBufferANDROID");

Comments

On 03/06/18 19:41, Mauro Rossi wrote:
> Change to avoid building error:
>
> external/mesa/src/intel/vulkan/anv_android.c:131:72:
> error: too few arguments to function call, expected 5, have 4
>     result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
>              ~~~~~~~~~~~~~~~~~~~                                        ^
> 1 error generated.
>
> Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in bo_cache_alloc/import")
> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> ---
>   src/intel/vulkan/anv_android.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
> index 7e07dbaaa4..ed5da5b537 100644
> --- a/src/intel/vulkan/anv_android.c
> +++ b/src/intel/vulkan/anv_android.c
> @@ -128,7 +128,7 @@ anv_image_from_gralloc(VkDevice device_h,
>       */
>      int dma_buf = gralloc_info->handle->data[0];
>   
> -   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
> +   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, 0, &bo);

If you look at anv_intel.c here are the flags that we use :

    uint64_t bo_flags = 0;
    if (device->instance->physicalDevice.supports_48bit_addresses)
       bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;

I'm pretty sure we want the same, potentially also the async flag?

>      if (result != VK_SUCCESS) {
>         return vk_errorf(device->instance, device, result,
>                          "failed to import dma-buf from VkNativeBufferANDROID");
Hi there,

2018-06-03 21:52 GMT+02:00 Lionel Landwerlin <lionel.g.landwerlin@intel.com>
:

> On 03/06/18 19:41, Mauro Rossi wrote:
>
>> Change to avoid building error:
>>
>> external/mesa/src/intel/vulkan/anv_android.c:131:72:
>> error: too few arguments to function call, expected 5, have 4
>>     result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
>>              ~~~~~~~~~~~~~~~~~~~                                        ^
>> 1 error generated.
>>
>> Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in
>> bo_cache_alloc/import")
>> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
>> ---
>>   src/intel/vulkan/anv_android.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/anv_android.c
>> b/src/intel/vulkan/anv_android.c
>> index 7e07dbaaa4..ed5da5b537 100644
>> --- a/src/intel/vulkan/anv_android.c
>> +++ b/src/intel/vulkan/anv_android.c
>> @@ -128,7 +128,7 @@ anv_image_from_gralloc(VkDevice device_h,
>>       */
>>      int dma_buf = gralloc_info->handle->data[0];
>>   -   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf,
>> &bo);
>> +   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, 0,
>> &bo);
>>
>
> If you look at anv_intel.c here are the flags that we use :
>
>    uint64_t bo_flags = 0;
>    if (device->instance->physicalDevice.supports_48bit_addresses)
>       bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>
> I'm pretty sure we want the same, potentially also the async flag?


Please I need review on the correctness or improvements,
because I just put 0 as bo_flags as a deduction based on similar change
in src/intel/vulkan/anv_queue.c where in anv_bo_cache_import() call the
bo_flag was set to 0

Thanks
M.


>
>
>      if (result != VK_SUCCESS) {
>>         return vk_errorf(device->instance, device, result,
>>                          "failed to import dma-buf from
>> VkNativeBufferANDROID");
>>
>
>
>
On 03/06/18 21:22, Mauro Rossi wrote:
> Hi there,
>
> 2018-06-03 21:52 GMT+02:00 Lionel Landwerlin 
> <lionel.g.landwerlin@intel.com <mailto:lionel.g.landwerlin@intel.com>>:
>
>     On 03/06/18 19:41, Mauro Rossi wrote:
>
>         Change to avoid building error:
>
>         external/mesa/src/intel/vulkan/anv_android.c:131:72:
>         error: too few arguments to function call, expected 5, have 4
>             result = anv_bo_cache_import(device, &device->bo_cache,
>         dma_buf, &bo);
>                      ~~~~~~~~~~~~~~~~~~~                   ^
>         1 error generated.
>
>         Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in
>         bo_cache_alloc/import")
>         Signed-off-by: Mauro Rossi <issor.oruam@gmail.com
>         <mailto:issor.oruam@gmail.com>>
>         ---
>           src/intel/vulkan/anv_android.c | 2 +-
>           1 file changed, 1 insertion(+), 1 deletion(-)
>
>         diff --git a/src/intel/vulkan/anv_android.c
>         b/src/intel/vulkan/anv_android.c
>         index 7e07dbaaa4..ed5da5b537 100644
>         --- a/src/intel/vulkan/anv_android.c
>         +++ b/src/intel/vulkan/anv_android.c
>         @@ -128,7 +128,7 @@ anv_image_from_gralloc(VkDevice device_h,
>               */
>              int dma_buf = gralloc_info->handle->data[0];
>           -   result = anv_bo_cache_import(device, &device->bo_cache,
>         dma_buf, &bo);
>         +   result = anv_bo_cache_import(device, &device->bo_cache,
>         dma_buf, 0, &bo);
>
>
>     If you look at anv_intel.c here are the flags that we use :
>
>        uint64_t bo_flags = 0;
>        if (device->instance->physicalDevice.supports_48bit_addresses)
>           bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>
>     I'm pretty sure we want the same, potentially also the async flag?
>
>
> Please I need review on the correctness or improvements,
> because I just put 0 as bo_flags as a deduction based on similar change
> in src/intel/vulkan/anv_queue.c where in anv_bo_cache_import() call 
> the bo_flag was set to 0
>
> Thanks
> M.

Here is what looks good to me (maybe Jason can confirm) :

    uint64_t bo_flags = 0;
    if (device->instance->physicalDevice.supports_48bit_addresses)
       bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
    if (pdevice->use_softpin) /* You'll need to grab a pointer to the 
physical device here */
       bo_flags |= EXEC_OBJECT_PINNED;

anv_queue.c is for fences which are buffers we don't actually touch for 
the GPU.
They're just used for creating/dealing with dependency graph of workloads.

It seems anv_intel.c needs updating, or maybe we should just drop that 
extensions now that we have other means of importing BOs.

Thanks for looking into this,

-
Lionel

>
>
>              if (result != VK_SUCCESS) {
>                 return vk_errorf(device->instance, device, result,
>                                  "failed to import dma-buf from
>         VkNativeBufferANDROID");
>
>
>
>
On June 3, 2018 15:31:11 Lionel Landwerlin <lionel.g.landwerlin@intel.com> 
wrote:
> On 03/06/18 21:22, Mauro Rossi wrote:
>> Hi there,
>>
>> 2018-06-03 21:52 GMT+02:00 Lionel Landwerlin <lionel.g.landwerlin@intel.com>:
>>
>> On 03/06/18 19:41, Mauro Rossi wrote:
>>
>> Change to avoid building error:
>>
>> external/mesa/src/intel/vulkan/anv_android.c:131:72:
>> error: too few arguments to function call, expected 5, have 4
>> result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
>> ~~~~~~~~~~~~~~~~~~~                                        ^
>> 1 error generated.
>>
>> Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in bo_cache_alloc/import")
>> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
>> ---
>> src/intel/vulkan/anv_android.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
>> index 7e07dbaaa4..ed5da5b537 100644
>> --- a/src/intel/vulkan/anv_android.c
>> +++ b/src/intel/vulkan/anv_android.c
>> @@ -128,7 +128,7 @@ anv_image_from_gralloc(VkDevice device_h,
>> */
>> int dma_buf = gralloc_info->handle->data[0];
>> -   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
>> +   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, 0, &bo);
>>
>> If you look at anv_intel.c here are the flags that we use :
>>
>> uint64_t bo_flags = 0;
>> if (device->instance->physicalDevice.supports_48bit_addresses)
>> bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>>
>> I'm pretty sure we want the same, potentially also the async flag?
>> Please I need review on the correctness or improvements,
>> because I just put 0 as bo_flags as a deduction based on similar change
>> in src/intel/vulkan/anv_queue.c where in anv_bo_cache_import() call the 
>> bo_flag was set to 0
>>
>> Thanks
>> M.
>
> Here is what looks good to me (maybe Jason can confirm) :
>
> uint64_t bo_flags = 0;
> if (device->instance->physicalDevice.supports_48bit_addresses)
> bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> if (pdevice->use_softpin) /* You'll need to grab a pointer to the physical 
> device here */
> bo_flags |= EXEC_OBJECT_PINNED;

That looks good to me. Sorry about the mess.

>
> anv_queue.c is for fences which are buffers we don't actually touch for the 
> GPU.
> They're just used for creating/dealing with dependency graph of workloads.
>
> It seems anv_intel.c needs updating, or maybe we should just drop that 
> extensions now that we have other means of importing BOs.
>
> Thanks for looking into this,
>
> -
> Lionel
>
>
>>
>>
>>
>>     if (result != VK_SUCCESS) {
>>        return vk_errorf(device->instance, device, result,
>>                         "failed to import dma-buf from VkNativeBufferANDROID");
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Hi,

2018-06-04 0:31 GMT+02:00 Lionel Landwerlin <lionel.g.landwerlin@intel.com>:

> On 03/06/18 21:22, Mauro Rossi wrote:
>
> Hi there,
>
> 2018-06-03 21:52 GMT+02:00 Lionel Landwerlin <
> lionel.g.landwerlin@intel.com>:
>
>> On 03/06/18 19:41, Mauro Rossi wrote:
>>
>>> Change to avoid building error:
>>>
>>> external/mesa/src/intel/vulkan/anv_android.c:131:72:
>>> error: too few arguments to function call, expected 5, have 4
>>>     result = anv_bo_cache_import(device, &device->bo_cache, dma_buf,
>>> &bo);
>>>              ~~~~~~~~~~~~~~~~~~~                                        ^
>>> 1 error generated.
>>>
>>> Fixes: b0d50247a7 ("anv/allocator: Set the BO flags in
>>> bo_cache_alloc/import")
>>> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
>>> ---
>>>   src/intel/vulkan/anv_android.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/intel/vulkan/anv_android.c
>>> b/src/intel/vulkan/anv_android.c
>>> index 7e07dbaaa4..ed5da5b537 100644
>>> --- a/src/intel/vulkan/anv_android.c
>>> +++ b/src/intel/vulkan/anv_android.c
>>> @@ -128,7 +128,7 @@ anv_image_from_gralloc(VkDevice device_h,
>>>       */
>>>      int dma_buf = gralloc_info->handle->data[0];
>>>   -   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf,
>>> &bo);
>>> +   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, 0,
>>> &bo);
>>>
>>
>> If you look at anv_intel.c here are the flags that we use :
>>
>>    uint64_t bo_flags = 0;
>>    if (device->instance->physicalDevice.supports_48bit_addresses)
>>       bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>>
>> I'm pretty sure we want the same, potentially also the async flag?
>
>
> Please I need review on the correctness or improvements,
> because I just put 0 as bo_flags as a deduction based on similar change
> in src/intel/vulkan/anv_queue.c where in anv_bo_cache_import() call the
> bo_flag was set to 0
>
> Thanks
> M.
>
>
> Here is what looks good to me (maybe Jason can confirm) :
>
>    uint64_t bo_flags = 0;
>    if (device->instance->physicalDevice.supports_48bit_addresses)
>       bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>    if (pdevice->use_softpin) /* You'll need to grab a pointer to the
> physical device here */
>

The above expression should be ok:

   if (device->instance->physicalDevice.use_softpin)

If all builds I'm sending v2 of PATCH 1/2 soon
Thanks

Mauro


>
>
      bo_flags |= EXEC_OBJECT_PINNED;
>
> anv_queue.c is for fences which are buffers we don't actually touch for
> the GPU.
> They're just used for creating/dealing with dependency graph of workloads.
>
> It seems anv_intel.c needs updating, or maybe we should just drop that
> extensions now that we have other means of importing BOs.
>
> Thanks for looking into this,
>
> -
> Lionel
>
>
>
>>
>>
>>      if (result != VK_SUCCESS) {
>>>         return vk_errorf(device->instance, device, result,
>>>                          "failed to import dma-buf from
>>> VkNativeBufferANDROID");
>>>
>>
>>
>>
>
>