[Mesa-dev] [PATCH 6/7] android: ignore MESA_GLSL_CACHE_DISABLE setting

[Emil Velikov]

On 17 January 2018 at 08:12, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>
> On 16.01.2018 11:02, Jordan Justen wrote:
>>
>> On 2018-01-15 04:31:42, Tapani Pälli wrote:
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>>   src/mesa/drivers/dri/i965/brw_disk_cache.c | 2 ++
>>>   src/util/disk_cache.c                      | 2 ++
>>>   2 files changed, 4 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_disk_cache.c
>>> b/src/mesa/drivers/dri/i965/brw_disk_cache.c
>>> index 65bb52726e..4df4504666 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_disk_cache.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_disk_cache.c
>>> @@ -405,8 +405,10 @@ void
>>>   brw_disk_cache_init(struct brw_context *brw)
>>>   {
>>>   #ifdef ENABLE_SHADER_CACHE
>>> +#ifndef ANDROID
>>>      if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", true))
>>>         return;
>>> +#endif
>>
>>
>> Arguably, if we don't want to enable the shader cache in i965, then we
>> probably wouldn't want to enable EGL_ANDROID_blob_cache that hits the
>> same paths.
>>
>> Since the 18.0 branch is happening soon, two questions arise:
>>
>> 1. Should we enable the shader cache on i965 by default in the 18.0
>>     release.
>>
>> 2. If not, should we enable it on master after the 18.0 branch.
>>
>> I guess if neither of these are yes, we could discuss whether it is
>> okay to enable this Android feature even though we don't want to
>> enable the shader cache on i965.
>>
>>>      char renderer[10];
>>>      MAYBE_UNUSED int len = snprintf(renderer, sizeof(renderer),
>>> "i965_%04x",
>>> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
>>> index d7891e3b70..3c98089e69 100644
>>> --- a/src/util/disk_cache.c
>>> +++ b/src/util/disk_cache.c
>>> @@ -208,9 +208,11 @@ disk_cache_create(const char *gpu_name, const char
>>> *timestamp,
>>>      if (local == NULL)
>>>         goto fail;
>>>   +#ifndef ANDROID
>>>      /* At user request, disable shader cache entirely. */
>>>      if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", false))
>>>         goto fail;
>>> +#endif
>>
>>
>> This just allows someone to force the cache feature off. This should
>> be safe, so I think we should leave it.
>
>
> True, will change the patch. This patch was actually a bit of extra and does
> not need to land on master, we can enable it on master at same time as for
> Linux desktop (whenever that happens). If Android-IA wants to enable it,
> they can use this patch or similar.
>
Setting the env. variable might be easier than patching it out.
Adds the benefit of toggling on/off via a simple config file change,
instead of building a complete image ;-)

Or perhaps the Android security model is blocking envvar manipulation?

-Emil