[Mesa-dev] [PATCH 2/7] egl: add support for EGL_ANDROID_blob_cache

[Tapani Pälli]

On 01/18/2018 04:55 PM, Emil Velikov wrote:
> On 17 January 2018 at 16:11, Tapani Pälli <tapani.palli at intel.com> wrote:
>>
>>
>> On 17.01.2018 13:28, Nicolai Hähnle wrote:
>>>
>>> On 16.01.2018 18:45, Emil Velikov wrote:
>>>>
>>>> Hi Tapani,
>>>>
>>>> On 15 January 2018 at 12:31, Tapani Pälli <tapani.palli at intel.com> wrote:
>>>>
>>>>> +static void
>>>>> +update_blob_cache_functions(struct dri2_egl_display *dri2_dpy,
>>>>> +                            struct dri2_egl_context *dri2_ctx)
>>>>> +{
>>>>> +   if (!dri2_dpy || !dri2_ctx)
>>>>> +      return;
>>>>
>>>> AFAICT dri2_dpy can never be NULL.
>>>>
>>>>> +
>>>>> +   /* No blob support. */
>>>>> +   if (!dri2_dpy->blob)
>>>>> +      return;
>>>>> +
>>>>> +   /* No functions to set. */
>>>>> +   if (!dri2_dpy->blob_cache_set)
>>>>> +      return;
>>>>> +
>>>>> +   dri2_dpy->blob->set_cache_funcs(dri2_ctx->dri_context,
>>>>> +                                   dri2_dpy->blob_cache_set,
>>>>> +                                   dri2_dpy->blob_cache_get);
>>>>> +}
>>>>> +
>>>>
>>>> I'm wondering why you opted to make set_cache_funcs dri_context
>>>> specific as opposed to dri_screen.
>>>> The latter seems to align better to EGLDisplay.
>>>>
>>>> Plus doing so will simplify the existing code - no hunk in
>>>> dri2_make_current, no dri2_dpy->blob/blob_cache_set checks, etc.
>>>
>>>
>>> Yes, please make it a screen thing. It just makes more sense, and there's
>>> precedent in Gallium, where the disk-cache is a per-pipe_screen object as
>>> well.
>>
>>
>> I chose context because eventually I need to access disk_cache which is part
>> of gl_context. I'm not sure how would I propagate the set/get there from
>> dri_screen?
>>
> Gallium does the following during create_context. I'm not sure if
> there's any particular reason why i965 cannot do the same.
> Tim, you've worked a fair bit in the area do you see any drawbacks?
> 
> ctx->Cache = pipe->screen->det_dist_shader_cache(pipe->screen);
> 

One problem is that client might set the callbacks only after context 
creation so we need to be able to do this during set_cache_funcs(). Now 
it works fine because we pass context there.

// Tapani