Hi Samuel,
(some comments further down)
On 11:30 PM - Oct 18 2015, Samuel Pitoiset wrote:
> Like for nvc0, this will allow to split different types of queries and
> to prepare the way for both global performance counters and MP counters.
>
> While we are at it, make use of nv50_query struct instead of pipe_query.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
> ---
> src/gallium/drivers/nouveau/Makefile.sources | 1 +
> src/gallium/drivers/nouveau/nv50/nv50_context.h | 12 +------
> src/gallium/drivers/nouveau/nv50/nv50_query.c | 29 ++--------------
> src/gallium/drivers/nouveau/nv50/nv50_query.h | 40
> ++++++++++++++++++++++
> .../drivers/nouveau/nv50/nv50_shader_state.c | 4 +--
> src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 3 +-
> 6 files changed, 49 insertions(+), 40 deletions(-)
> create mode 100644 src/gallium/drivers/nouveau/nv50/nv50_query.h
>
> diff --git a/src/gallium/drivers/nouveau/Makefile.sources
> b/src/gallium/drivers/nouveau/Makefile.sources
> index c18e9f5..06d9d97 100644
> --- a/src/gallium/drivers/nouveau/Makefile.sources
> +++ b/src/gallium/drivers/nouveau/Makefile.sources
> @@ -73,6 +73,7 @@ NV50_C_SOURCES := \
> nv50/nv50_program.h \
> nv50/nv50_push.c \
> nv50/nv50_query.c \
> + nv50/nv50_query.h \
> nv50/nv50_resource.c \
> nv50/nv50_resource.h \
> nv50/nv50_screen.c \
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h
> b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> index 69c1212..fb74a97 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> @@ -16,6 +16,7 @@
> #include "nv50/nv50_program.h"
> #include "nv50/nv50_resource.h"
> #include "nv50/nv50_transfer.h"
> +#include "nv50/nv50_query.h"
>
> #include "nouveau_context.h"
> #include "nouveau_debug.h"
> @@ -195,17 +196,6 @@ void nv50_default_kick_notify(struct nouveau_pushbuf *);
> /* nv50_draw.c */
> extern struct draw_stage *nv50_draw_render_stage(struct nv50_context *);
>
> -/* nv50_query.c */
> -void nv50_init_query_functions(struct nv50_context *);
> -void nv50_query_pushbuf_submit(struct nouveau_pushbuf *, uint16_t method,
> - struct pipe_query *, unsigned result_offset);
> -void nv84_query_fifo_wait(struct nouveau_pushbuf *, struct pipe_query *);
> -void nva0_so_target_save_offset(struct pipe_context *,
> - struct pipe_stream_output_target *,
> - unsigned index, bool seralize);
> -
> -#define NVA0_QUERY_STREAM_OUTPUT_BUFFER_OFFSET (PIPE_QUERY_TYPES + 0)
> -
> /* nv50_shader_state.c */
> void nv50_vertprog_validate(struct nv50_context *);
> void nv50_gmtyprog_validate(struct nv50_context *);
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
> b/src/gallium/drivers/nouveau/nv50/nv50_query.c
> index 5368ee7..7718d69 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
> @@ -25,6 +25,7 @@
> #define NV50_PUSH_EXPLICIT_SPACE_CHECKING
>
> #include "nv50/nv50_context.h"
> +#include "nv50/nv50_query.h"
> #include "nv_object.xml.h"
>
> #define NV50_QUERY_STATE_READY 0
> @@ -39,29 +40,8 @@
> * queries anyway.
> */
>
> -struct nv50_query {
> - uint32_t *data;
> - uint16_t type;
> - uint16_t index;
> - uint32_t sequence;
> - struct nouveau_bo *bo;
> - uint32_t base;
> - uint32_t offset; /* base + i * 32 */
> - uint8_t state;
> - bool is64bit;
> - int nesting; /* only used for occlusion queries */
> - struct nouveau_mm_allocation *mm;
> - struct nouveau_fence *fence;
> -};
> -
> #define NV50_QUERY_ALLOC_SPACE 256
>
> -static inline struct nv50_query *
> -nv50_query(struct pipe_query *pipe)
> -{
> - return (struct nv50_query *)pipe;
> -}
> -
> static bool
> nv50_query_allocate(struct nv50_context *nv50, struct nv50_query *q, int
> size)
> {
> @@ -363,9 +343,8 @@ nv50_query_result(struct pipe_context *pipe, struct
> pipe_query *pq,
> }
>
> void
> -nv84_query_fifo_wait(struct nouveau_pushbuf *push, struct pipe_query *pq)
> +nv84_query_fifo_wait(struct nouveau_pushbuf *push, struct nv50_query *q)
> {
> - struct nv50_query *q = nv50_query(pq);
> unsigned offset = q->offset;
>
> PUSH_SPACE(push, 5);
> @@ -453,10 +432,8 @@ nv50_render_condition(struct pipe_context *pipe,
>
> void
> nv50_query_pushbuf_submit(struct nouveau_pushbuf *push, uint16_t method,
> - struct pipe_query *pq, unsigned result_offset)
> + struct nv50_query *q, unsigned result_offset)
> {
> - struct nv50_query *q = nv50_query(pq);
> -
> nv50_query_update(q);
> if (q->state != NV50_QUERY_STATE_READY)
> nouveau_bo_wait(q->bo, NOUVEAU_BO_RD, push->client);
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.h
> b/src/gallium/drivers/nouveau/nv50/nv50_query.h
> new file mode 100644
> index 0000000..722af0c
> --- /dev/null
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_query.h
> @@ -0,0 +1,40 @@
Shouldn't there be a copyright notice (or whatever that text is called) here?
> +#ifndef __NV50_QUERY_H__
> +#define __NV50_QUERY_H__
> +
> +#include "pipe/p_context.h"
> +
> +#include "nouveau_context.h"
> +#include "nouveau_mm.h"
> +
> +#define NVA0_QUERY_STREAM_OUTPUT_BUFFER_OFFSET (PIPE_QUERY_TYPES + 0)
> +
> +struct nv50_query {
> + uint32_t *data;
> + uint16_t type;
> + uint16_t index;
> + uint32_t sequence;
> + struct nouveau_bo *bo;
> + uint32_t base;
> + uint32_t offset; /* base + i * 32 */
> + uint8_t state;
> + bool is64bit;
> + int nesting; /* only used for occlusion queries */
> + struct nouveau_mm_allocation *mm;
> + struct nouveau_fence *fence;
> +};
> +
> +static inline struct nv50_query *
> +nv50_query(struct pipe_query *pipe)
> +{
> + return (struct nv50_query *)pipe;
> +}
> +
> +void nv50_init_query_functions(struct nv50_context *);
> +void nv50_query_pushbuf_submit(struct nouveau_pushbuf *, uint16_t,
> + struct nv50_query *, unsigned result_offset);
> +void nv84_query_fifo_wait(struct nouveau_pushbuf *, struct nv50_query *);
> +void nva0_so_target_save_offset(struct pipe_context *,
> + struct pipe_stream_output_target *,
> + unsigned, bool);
> +
> +#endif
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> index 941555f..958d044 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> @@ -629,7 +629,7 @@ nv50_stream_output_validate(struct nv50_context *nv50)
> const unsigned n = nv50->screen->base.class_3d >= NVA0_3D_CLASS ? 4 :
> 3;
>
> if (n == 4 && !targ->clean)
> - nv84_query_fifo_wait(push, targ->pq);
> + nv84_query_fifo_wait(push, nv50_query(targ->pq));
This is not related to your patch, but there should be an `assert(targ->pq)`
here, as `nv84_query_fifo_wait` dereferences it without checking (or add a check
in `nv84_query_fifo_wait`. Or maybe `targ->pq` can't be NULL, in that case the
`assert` a few lines below is useless and could be remove.
> BEGIN_NV04(push, NV50_3D(STRMOUT_ADDRESS_HIGH(i)), n);
> PUSH_DATAh(push, buf->address + targ->pipe.buffer_offset);
> PUSH_DATA (push, buf->address + targ->pipe.buffer_offset);
> @@ -639,7 +639,7 @@ nv50_stream_output_validate(struct nv50_context *nv50)
> if (!targ->clean) {
> assert(targ->pq);
> nv50_query_pushbuf_submit(push, NVA0_3D_STRMOUT_OFFSET(i),
> - targ->pq, 0x4);
> + nv50_query(targ->pq), 0x4);
You're casting `targ->pq` only twice, so not sure if it would make sense to
define a `nv50_query` pointer at the start of the function (along with the
assert, if neeeded), and cast `targ->pq` only once.
> } else {
> BEGIN_NV04(push, NVA0_3D(STRMOUT_OFFSET(i)), 1);
> PUSH_DATA(push, 0);
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> index f5f4708..dbc6632 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> @@ -745,7 +745,8 @@ nva0_draw_stream_output(struct nv50_context *nv50,
> PUSH_DATA (push, 0);
> BEGIN_NV04(push, NVA0_3D(DRAW_TFB_STRIDE), 1);
> PUSH_DATA (push, so->stride);
> - nv50_query_pushbuf_submit(push, NVA0_3D_DRAW_TFB_BYTES, so->pq, 0x4);
> + nv50_query_pushbuf_submit(push, NVA0_3D_DRAW_TFB_BYTES,
> + nv50_query(so->pq), 0x4);
Again, if an assert is needed for the comments above, you might need to add one
here too.
Pierre
> BEGIN_NV04(push, NV50_3D(VERTEX_END_GL), 1);
> PUSH_DATA (push, 0);
>
> --
> 2.6.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
