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

[Tapani Pälli]

On 01/18/2018 04:59 PM, Emil Velikov wrote:
> 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?

This environment variable would need to be there when Surfaceflinger 
starts, so I guess it means having that in init.environ.rc. I can try if 
it gets propagated to everyone from there. Then one needs to do only 
stop/start cycle or reboot and no need to build and copy new mesa library.

One flexible option would be also to use drirc (which we do have), but 
then you need to do start/stop cycle as well.

In any case, I think this patch should be scrapped, toggling it on can 
be separate patch in Android tree.

// Tapani