On 19 January 2018 at 06:53, Tapani Pälli <tapani.pa...@intel.com> wrote: > > > On 01/18/2018 04:55 PM, Emil Velikov wrote: >> >> On 17 January 2018 at 16:11, Tapani Pälli <tapani.pa...@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.pa...@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. > I don't see why that would be an issue. Both ctx::cache and screen::cache are pointer to a single instance. Hence, as we deref. screen::cache and update the callbacks, everything will be fine from ctx POV - no need to update for each make_current call/etc.
Am I having a dull moment here? Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev