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

2018-01-18 Thread Tapani Pälli 



On 01/18/2018 04:59 PM, Emil Velikov wrote:

On 17 January 2018 at 08:12, Tapani Pälli  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 
---
   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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-01-18 Thread Emil Velikov 
On 17 January 2018 at 08:12, Tapani Pälli  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 
>>> ---
>>>   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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-01-17 Thread Tapani Pälli 



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 
---
  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.




-Jordan


 /* Determine path for cache based on the first defined name as follows:
  *
--
2.14.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 6/7] android: ignore MESA_GLSL_CACHE_DISABLE setting

2018-01-16 Thread Jordan Justen 
On 2018-01-15 04:31:42, Tapani Pälli wrote:
> Signed-off-by: Tapani Pälli 
> ---
>  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.

-Jordan

> /* Determine path for cache based on the first defined name as follows:
>  *
> -- 
> 2.14.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 6/7] android: ignore MESA_GLSL_CACHE_DISABLE setting

2018-01-15 Thread Tapani Pälli 
Signed-off-by: Tapani Pälli 
---
 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
 
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
 
/* Determine path for cache based on the first defined name as follows:
 *
-- 
2.14.3

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