[Mesa-dev] [PATCH 01/11] gallium: Basic compute interface.

[Marek Olšák]

On Fri, Mar 23, 2012 at 1:40 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index aaeeb81..de990fe 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -63,6 +63,7 @@ struct pipe_vertex_element;
>  struct pipe_video_buffer;
>  struct pipe_video_decoder;
>  struct pipe_viewport_state;
> +struct pipe_compute_state;
>  union pipe_color_union;
>
>  /**
> @@ -140,6 +141,9 @@ struct pipe_context {
>    void   (*bind_geometry_sampler_states)(struct pipe_context *,
>                                           unsigned num_samplers,
>                                           void **samplers);
> +   void   (*bind_compute_sampler_states)(struct pipe_context *,
> +                                         unsigned num_samplers,
> +                                         void **samplers);

Hi,

could you please add an "unsigned start_slot" parameter in
bind_compute_sampler_states? The function would then look like this:

void   (*bind_compute_sampler_states)(struct pipe_context *,
                                      unsigned start_slot,
                                      unsigned num_samplers,
                                      void **samplers);

Where start_slot+num_samplers <= max_samplers, and max_samplers =
get(SHADER_CAP_MAX_TEXTURE_SAMPLERS).

The reason for this is that state changes will be faster if you only
need to change a subset of the bound states (less work for the pipe
driver). The disadvantage is that the states will have to be unbound
using something like this:

void *null_ptrs[MAX_SAMPLERS] = {NULL};
bind_compute_sampler_states(pipe, 0, max_samplers, null_ptrs);

I am aware that the other functions don't have the start_slot
parameter and I'd like to update them so that they do (I have started
reworking set_vertex_buffers), but that will be a lot of work. In the
case of compute, where the interface hasn't landed yet, it would be
wise to add start_slot as soon as possible, so that we don't have to
rewrite drivers for it later on.

>    void   (*delete_sampler_state)(struct pipe_context *, void *);
>
>    void * (*create_rasterizer_state)(struct pipe_context *,
> @@ -219,6 +223,10 @@ struct pipe_context {
>                                       unsigned num_views,
>                                       struct pipe_sampler_view **);
>
> +   void (*set_compute_sampler_views)(struct pipe_context *,
> +                                     unsigned num_views,
> +                                     struct pipe_sampler_view **);
> +

Please add start_slot in set_compute_sampler_views as well.

>    void (*set_vertex_buffers)( struct pipe_context *,
>                                unsigned num_buffers,
>                                const struct pipe_vertex_buffer * );
> @@ -417,6 +425,64 @@ struct pipe_context {
>     */
>    struct pipe_video_buffer *(*create_video_buffer)( struct pipe_context *context,
>                                                      const struct pipe_video_buffer *templat );
> +
> +   /**
> +    * Compute kernel execution
> +    */
> +   /*@{*/
> +   /**
> +    * Define the compute program and parameters to be used by
> +    * pipe_context::launch_grid.
> +    */
> +   void *(*create_compute_state)(struct pipe_context *context,
> +                                const struct pipe_compute_state *);
> +   void (*bind_compute_state)(struct pipe_context *, void *);
> +   void (*delete_compute_state)(struct pipe_context *, void *);
> +
> +   /**
> +    * Bind an array of buffers to be mapped into the address space of
> +    * the GLOBAL resource.  Any buffers that were previously bound to
> +    * the GLOBAL resource are unbound after this call.
> +    *
> +    * \param n          number of buffers to map.
> +    * \param resources  array of pointers to the buffers to map, it
> +    *                   should contain at least \a n elements.
> +    * \param handles    array of pointers to the memory locations that
> +    *                   will be filled with the respective base
> +    *                   addresses each buffer will be mapped to.
> +    *                   It should contain at least \a n elements.
> +    *
> +    * Note that the driver isn't required to make any guarantees about
> +    * the contents of the \a handles array being valid anytime except
> +    * during the subsequent calls to pipe_context::launch_grid.  This
> +    * means that the only sensible location handles[i] may point to is
> +    * somewhere within the INPUT buffer itself.  This is so to
> +    * accommodate implementations that lack virtual memory but
> +    * nevertheless migrate buffers on the fly, leading to resource
> +    * base addresses that change on each kernel invocation or are
> +    * unknown to the pipe driver.
> +    */
> +   void (*set_global_binding)(struct pipe_context *context, int n,
> +                             struct pipe_resource **resources,
> +                             uint32_t **handles);

I think start_slot would be useful in set_global_binding too.

Best regards,
Marek