[Mesa-dev] [PATCH] r600g: Include SH and SMX when invalidating read caches
Tom Stellard
tom at stellard.net
Mon Jun 24 10:26:59 PDT 2013
On Sun, Jun 23, 2013 at 05:56:15PM -0400, Alex Deucher wrote:
> On Sun, Jun 23, 2013 at 2:24 PM, Marek Olšák <maraeo at gmail.com> wrote:
> > Hi Alex,
> >
> > rctx->framebuffer.state.nr_cbufs might not contain what you think it
> > does, because the framebuffer that needs flushing may have been
> > replaced by a new framebuffer and the cache flushing of the old
> > framebuffer usually takes place before the first draw to the new
> > framebuffer. To solve this, we can either set all CB bits, or move
> > setting CP_COHER_CNTL outside of r600_flush_emit.
>
> I think it should be ok to just set all the CB bits. My only concern
> was whether there would be a problems with flushing unbound CBs. I
> think it should be ok. Martin, can you try the attached patch?
>
I have tested this patch as well as Martin's patch and neither patch
introduces any regressions for compute on Cayman.
-Tom
> Alex
>
> >
> > Marek
> >
> > On Sun, Jun 23, 2013 at 7:41 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
> >> On Sat, Jun 22, 2013 at 11:53 AM, Martin Andersson <g02maran at gmail.com> wrote:
> >>> On Sat, Jun 22, 2013 at 12:22 PM, Marek Olšák <maraeo at gmail.com> wrote:
> >>>> Reviewed-by: Marek Olšák <maraeo at gmail.com>
> >>>>
> >>>> BTW, SMX is a write cache, to maybe it shouldn't be part of this patch.
> >>>
> >>> I made a little experiment where i ran
> >>> "ext_framebuffer_multisample-unaligned-blit 4 color downsample -auto"
> >>> 10000 times and found that without SMX the test failed 177 times and
> >>> with SMX it didn't fail at all. So I do think the SMX cache should be
> >>> invalidated somewhere.
> >>>
> >>> Before http://cgit.freedesktop.org/mesa/mesa/commit/?id=4539f8e20af286d1f521eb016c89c6d9af0b801c
> >>> it was under R600_CONTEXT_FLUSH_AND_INV, is that a better place?
> >>>
> >>> If that is the proper place for SMX should SH also be there, since it
> >>> was also there before the patch, or do you have any other suggestions?
> >>
> >> Does something like this help? You might play with some variants of
> >> this patch. This might break compute however as Tom had some problems
> >> with CB flushes on cayman class hw in the past.
> >>
> >> Alex
> >>
> >>
> >>>
> >>>> Marek
> >>>>
> >>>> On Sun, Jun 16, 2013 at 1:27 PM, Martin Andersson <g02maran at gmail.com> wrote:
> >>>>> Not including the SH and SMX caches when invalidating read caches causes
> >>>>> random failures on some piglit tests when VA is enabled.
> >>>>>
> >>>>> Since the failures are random, and there other problems also causing random
> >>>>> failures, it's hard to know exactly what tests were effected, but these
> >>>>> tests now consistently pass:
> >>>>>
> >>>>> fast_color_clear/all-colors
> >>>>> fast_color_clear/redundant-clear
> >>>>> spec/!OpenGL 1.1/draw-pixels samples={2,4,6,8}
> >>>>> spec/!OpenGL 1.1/drawbuffer-modes
> >>>>> ---
> >>>>> src/gallium/drivers/r600/r600_hw_context.c | 2 ++
> >>>>> 1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> >>>>> index 944b666..df20e56 100644
> >>>>> --- a/src/gallium/drivers/r600/r600_hw_context.c
> >>>>> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> >>>>> @@ -231,6 +231,8 @@ void r600_flush_emit(struct r600_context *rctx)
> >>>>> if (rctx->flags & R600_CONTEXT_INVAL_READ_CACHES) {
> >>>>> cp_coher_cntl |= S_0085F0_VC_ACTION_ENA(1) |
> >>>>> S_0085F0_TC_ACTION_ENA(1) |
> >>>>> + S_0085F0_SH_ACTION_ENA(1) |
> >>>>> + S_0085F0_SMX_ACTION_ENA(1) |
> >>>>> S_0085F0_FULL_CACHE_ENA(1);
> >>>>> emit_flush = 1;
> >>>>> }
> >>>>> --
> >>>>> 1.8.3
> >>>>>
> >>>>> _______________________________________________
> >>>>> mesa-dev mailing list
> >>>>> mesa-dev at lists.freedesktop.org
> >>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>> _______________________________________________
> >>> mesa-dev mailing list
> >>> mesa-dev at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> From 040284d2d3d01b3a2181a024afab8cfa1a5143d2 Mon Sep 17 00:00:00 2001
> From: Alex Deucher <alexander.deucher at amd.com>
> Date: Sun, 23 Jun 2013 13:36:42 -0400
> Subject: [PATCH] r600g: adjust flush flags (v2)
>
> 1. flush SH with read caches
> 2. add flag for DB flushes
> 3. add flag for CB flushes
>
> v2: flush all CBs, remove redundant emit_state variable.
>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> src/gallium/drivers/r600/evergreen_state.c | 2 +
> src/gallium/drivers/r600/r600_hw_context.c | 30 ++++++++++++++++++++++++---
> src/gallium/drivers/r600/r600_pipe.h | 2 +
> src/gallium/drivers/r600/r600_state.c | 2 +
> 4 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index 3ebb157..59fa412 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -1715,6 +1715,7 @@ static void evergreen_set_framebuffer_state(struct pipe_context *ctx,
>
> if (rctx->framebuffer.state.nr_cbufs) {
> rctx->flags |= R600_CONTEXT_WAIT_3D_IDLE | R600_CONTEXT_FLUSH_AND_INV;
> + rctx->flags |= R600_CONTEXT_INVAL_CB_CACHES;
>
> if (rctx->framebuffer.state.cbufs[0]->texture->nr_samples > 1) {
> rctx->flags |= R600_CONTEXT_FLUSH_AND_INV_CB_META;
> @@ -1722,6 +1723,7 @@ static void evergreen_set_framebuffer_state(struct pipe_context *ctx,
> }
> if (rctx->framebuffer.state.zsbuf) {
> rctx->flags |= R600_CONTEXT_WAIT_3D_IDLE | R600_CONTEXT_FLUSH_AND_INV;
> + rctx->flags |= R600_CONTEXT_INVAL_DB_CACHES;
>
> rtex = (struct r600_texture*)rctx->framebuffer.state.zsbuf->texture;
> if (rtex->htile) {
> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> index 944b666..087d4dd 100644
> --- a/src/gallium/drivers/r600/r600_hw_context.c
> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> @@ -185,7 +185,6 @@ void r600_flush_emit(struct r600_context *rctx)
> struct radeon_winsys_cs *cs = rctx->rings.gfx.cs;
> unsigned cp_coher_cntl = 0;
> unsigned wait_until = 0;
> - unsigned emit_flush = 0;
>
> if (!rctx->flags) {
> return;
> @@ -231,8 +230,32 @@ void r600_flush_emit(struct r600_context *rctx)
> if (rctx->flags & R600_CONTEXT_INVAL_READ_CACHES) {
> cp_coher_cntl |= S_0085F0_VC_ACTION_ENA(1) |
> S_0085F0_TC_ACTION_ENA(1) |
> + S_0085F0_SH_ACTION_ENA(1) |
> S_0085F0_FULL_CACHE_ENA(1);
> - emit_flush = 1;
> + }
> +
> + if (rctx->flags & R600_CONTEXT_INVAL_DB_CACHES) {
> + cp_coher_cntl |= S_0085F0_DB_ACTION_ENA(1) |
> + S_0085F0_DB_DEST_BASE_ENA(1) |
> + S_0085F0_SMX_ACTION_ENA(1);
> + }
> +
> + if (rctx->flags & R600_CONTEXT_INVAL_CB_CACHES) {
> + cp_coher_cntl |= S_0085F0_CB_ACTION_ENA(1) |
> + S_0085F0_CB0_DEST_BASE_ENA(1) |
> + S_0085F0_CB1_DEST_BASE_ENA(1) |
> + S_0085F0_CB2_DEST_BASE_ENA(1) |
> + S_0085F0_CB3_DEST_BASE_ENA(1) |
> + S_0085F0_CB4_DEST_BASE_ENA(1) |
> + S_0085F0_CB5_DEST_BASE_ENA(1) |
> + S_0085F0_CB6_DEST_BASE_ENA(1) |
> + S_0085F0_CB7_DEST_BASE_ENA(1) |
> + S_0085F0_SMX_ACTION_ENA(1);
> + if (rctx->chip_class >= EVERGREEN)
> + cp_coher_cntl |= S_0085F0_CB8_DEST_BASE_ENA(1) |
> + S_0085F0_CB9_DEST_BASE_ENA(1) |
> + S_0085F0_CB10_DEST_BASE_ENA(1) |
> + S_0085F0_CB11_DEST_BASE_ENA(1);
> }
>
> if (rctx->flags & R600_CONTEXT_STREAMOUT_FLUSH) {
> @@ -241,10 +264,9 @@ void r600_flush_emit(struct r600_context *rctx)
> S_0085F0_SO2_DEST_BASE_ENA(1) |
> S_0085F0_SO3_DEST_BASE_ENA(1) |
> S_0085F0_SMX_ACTION_ENA(1);
> - emit_flush = 1;
> }
>
> - if (emit_flush) {
> + if (cp_coher_cntl) {
> cs->buf[cs->cdw++] = PKT3(PKT3_SURFACE_SYNC, 3, 0);
> cs->buf[cs->cdw++] = cp_coher_cntl; /* CP_COHER_CNTL */
> cs->buf[cs->cdw++] = 0xffffffff; /* CP_COHER_SIZE */
> diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
> index 2a81434..89ecab3 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -72,6 +72,8 @@
> #define R600_CONTEXT_FLUSH_AND_INV_CB_META (1 << 5)
> #define R600_CONTEXT_PS_PARTIAL_FLUSH (1 << 6)
> #define R600_CONTEXT_FLUSH_AND_INV_DB_META (1 << 7)
> +#define R600_CONTEXT_INVAL_DB_CACHES (1 << 8)
> +#define R600_CONTEXT_INVAL_CB_CACHES (1 << 9)
>
> #define R600_QUERY_DRAW_CALLS (PIPE_QUERY_DRIVER_SPECIFIC + 0)
> #define R600_QUERY_REQUESTED_VRAM (PIPE_QUERY_DRIVER_SPECIFIC + 1)
> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> index 068d871..e610761 100644
> --- a/src/gallium/drivers/r600/r600_state.c
> +++ b/src/gallium/drivers/r600/r600_state.c
> @@ -1553,6 +1553,7 @@ static void r600_set_framebuffer_state(struct pipe_context *ctx,
>
> if (rctx->framebuffer.state.nr_cbufs) {
> rctx->flags |= R600_CONTEXT_WAIT_3D_IDLE | R600_CONTEXT_FLUSH_AND_INV;
> + rctx->flags |= R600_CONTEXT_INVAL_CB_CACHES;
>
> if (rctx->chip_class >= R700 &&
> rctx->framebuffer.state.cbufs[0]->texture->nr_samples > 1) {
> @@ -1561,6 +1562,7 @@ static void r600_set_framebuffer_state(struct pipe_context *ctx,
> }
> if (rctx->framebuffer.state.zsbuf) {
> rctx->flags |= R600_CONTEXT_WAIT_3D_IDLE | R600_CONTEXT_FLUSH_AND_INV;
> + rctx->flags |= R600_CONTEXT_INVAL_DB_CACHES;
>
> rtex = (struct r600_texture*)rctx->framebuffer.state.zsbuf->texture;
> if (rctx->chip_class >= R700 && rtex->htile) {
> --
> 1.7.7.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list