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

I was confused since that is not how it works with i965. We create cache during context creation and store it in context (gl_context::Cache). This happens for each context and they get a unique disk_cache pointer.

I understand your proposal would be to move Cache to screen structure and utilize this same disk_cache instance from each context. I guess it should be possible and could be done separately before blob cache implementation.

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

Reply via email to