[Mesa-dev] [PATCH 4/4] radeonsi: Print a message when scratch allocation fails.

[Nicolai Hähnle]

On 21.04.2016 09:14, Bas Nieuwenhuizen wrote:
> On Wed, Apr 20, 2016 at 8:33 AM,  <eocallaghan at alterapraxis.com> wrote:
>> On 2016-04-20 11:46, Nicolai Hähnle wrote:
>>>
>>> On 19.04.2016 17:50, Bas Nieuwenhuizen wrote:
>>>>
>>>> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>>>> ---
>>>>    src/gallium/drivers/radeonsi/si_compute.c       | 5 ++++-
>>>>    src/gallium/drivers/radeonsi/si_state_shaders.c | 5 ++++-
>>>>    2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/radeonsi/si_compute.c
>>>> b/src/gallium/drivers/radeonsi/si_compute.c
>>>> index b46a2fe..7d91ac6 100644
>>>> --- a/src/gallium/drivers/radeonsi/si_compute.c
>>>> +++ b/src/gallium/drivers/radeonsi/si_compute.c
>>>> @@ -215,8 +215,11 @@ static bool si_setup_compute_scratch_buffer(struct
>>>> si_context *sctx,
>>>>                                  scratch_needed, 256, false,
>>>> RADEON_DOMAIN_VRAM,
>>>>                                  RADEON_FLAG_NO_CPU_ACCESS);
>>>>
>>>> -               if (!sctx->compute_scratch_buffer)
>>>> +               if (!sctx->compute_scratch_buffer) {
>>>> +                       fprintf(stderr, "Warning: Failed to allocate the
>>>> "
>>>> +                               "scratch buffer\n");
>>>>                          return false;
>>>> +               }
>>>
>>>
>>> Here and below, please change the "Warning" into "radeonsi" so
>>> unsuspecting users will be more likely to understand what's going on.
>>> With that changed, the patch is
>>>
>>> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>>
>> Wait, why not use the std R600_ERR() macro that wraps fprintf() calls?
>
> Because I didn't think of it. Looking through the source code, the
> usage is pretty mixed. Is one preferred over the other?

Obviously, neither did I. As long as we're certain that the __FILE__ in 
the macro expands to something that includes radeon or radeonsi, I think 
both are fine, and we should probably go with the macro. But I don't 
care much either way.

Nicolai

> - Bas
>>
>>>
>>>>          }
>>>>
>>>>          if (sctx->compute_scratch_buffer != shader->scratch_bo &&
>>>> scratch_needed) {
>>>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c
>>>> b/src/gallium/drivers/radeonsi/si_state_shaders.c
>>>> index fef676b..2396b8e 100644
>>>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>>>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>>>> @@ -1692,8 +1692,11 @@ static bool si_update_spi_tmpring_size(struct
>>>> si_context *sctx)
>>>>                                                      scratch_needed_size,
>>>> 256, false,
>>>>                                                      RADEON_DOMAIN_VRAM,
>>>>
>>>> RADEON_FLAG_NO_CPU_ACCESS);
>>>> -                       if (!sctx->scratch_buffer)
>>>> +                       if (!sctx->scratch_buffer) {
>>>> +                               fprintf(stderr, "Warning: Failed to
>>>> allocate the "
>>>> +                                       "scratch buffer\n");
>>>>                                  return false;
>>>> +                       }
>>>>                          sctx->emit_scratch_reloc = true;
>>>>                  }
>>>>
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>