Hi Mark,
Above all - welcome to nouveau. Glad to see fresh blood joining.
On Mon, 17 Feb 2020 at 17:41, Mark Menzynski <mmenz...@redhat.com> wrote:
>
> Adds shader disk caching for nvc0 to reduce the need to every time compile
> shaders. Shaders are saved into disk_shader_cache from nvc0_screen structure.
>
> It serializes the input nv50_ir_prog_info to compute the hash key and
> also to do a byte compare between the original nv50_ir_prog_info and the one
> saved in the cache. If keys match and also the byte compare returns they
> are equal, shaders are same, and the compiled nv50_ir_prog_info_out from the
> cache can be used instead of compiling input info.
>
> Seems to be significantly improving loading times. Piglit tests seem
> to be OK.
>
Really happy to see this happening. Do you mind adding some rough
numbers in the commit message.
Out of curiosity: Is there any specific reason why nv50 isn't also wired up?
Is it a case of didn't test, or there's something more subtle?
> Signed-off-by: Mark Menzynski <mmenz...@redhat.com>
> ---
> .../drivers/nouveau/nvc0/nvc0_context.h | 1 +
> .../drivers/nouveau/nvc0/nvc0_program.c | 49 ++++++++++++++++---
> .../drivers/nouveau/nvc0/nvc0_shader_state.c | 3 +-
> src/gallium/drivers/nouveau/nvc0/nvc0_state.c | 2 +
> 4 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> index 8a2a8f2797e..4b83d1afeb4 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> @@ -321,6 +321,7 @@ extern struct draw_stage *nvc0_draw_render_stage(struct
> nvc0_context *);
>
> /* nvc0_program.c */
> bool nvc0_program_translate(struct nvc0_program *, uint16_t chipset,
> + struct disk_cache *,
> struct pipe_debug_callback *);
> bool nvc0_program_upload(struct nvc0_context *, struct nvc0_program *);
> void nvc0_program_destroy(struct nvc0_context *, struct nvc0_program *);
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> index 1a5073292e8..06b6f7b4db5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> @@ -24,6 +24,7 @@
>
> #include "compiler/nir/nir.h"
> #include "tgsi/tgsi_ureg.h"
> +#include "util/blob.h"
>
> #include "nvc0/nvc0_context.h"
>
> @@ -568,11 +569,19 @@ nvc0_program_dump(struct nvc0_program *prog)
>
> bool
> nvc0_program_translate(struct nvc0_program *prog, uint16_t chipset,
> + struct disk_cache *disk_shader_cache,
> struct pipe_debug_callback *debug)
> {
> + struct blob blob;
> struct nv50_ir_prog_info *info;
> struct nv50_ir_prog_info_out info_out = {};
> - int ret;
> +
> + void *cached_data = NULL;
> + size_t cached_size;
> + bool shader_found = false;
> +
> + int ret = 0;
> + cache_key key;
>
> info = CALLOC_STRUCT(nv50_ir_prog_info);
> if (!info)
> @@ -631,14 +640,38 @@ nvc0_program_translate(struct nvc0_program *prog,
> uint16_t chipset,
> info->assignSlots = nvc0_program_assign_varying_slots;
>
> /* these fields might be overwritten by the compiler */
> - info_out.bin.smemSize = prog->cp.smem_size;
> - info_out.io.genUserClip = prog->vp.num_ucps;
> -
> - ret = nv50_ir_generate_code(info, &info_out);
> - if (ret) {
> - NOUVEAU_ERR("shader translation failed: %i\n", ret);
> - goto out;
> + info->bin.smemSize = prog->cp.smem_size;
> + info->io.genUserClip = prog->vp.num_ucps;
> +
> + blob_init(&blob);
> + nv50_ir_prog_info_serialize(&blob, info);
> +
> + if (disk_shader_cache) {
Will this work correctly, if the cache is explicitly disabled?
It's been years since I looked at this code so I wonder if the two
original "info_out... = prog->..." shouldn't stay as-is.
At the same time it will make sense to move the "info->... =
prog->..." + serialise() within the if (disk_shader_cache) hunk.
blob_init() seems safe, so it can stay as-is.
HTH
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
