[Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.
Marek Olšák
maraeo at gmail.com
Thu Sep 24 14:10:30 PDT 2015
On Thu, Sep 24, 2015 at 9:47 PM, Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
> On Thursday, September 24, 2015 07:24:50 PM Marek Olšák wrote:
>> On Thu, Sep 24, 2015 at 2:15 PM, Bas Nieuwenhuizen
>>
>> <bas at basnieuwenhuizen.nl> wrote:
>> > Hi Marek,
>> >
>> > Thanks for the review and my apologies, it seems I underestimated the
>> > potential for regressions in this series.
>> >
>> > See below for some comments in reaction to the review.
>> >
>> > For a v2 is it preferred that I rebase it to master, or keep basing it on
>> > the old version? There are some function renames that impact this series.
>> We'd like to merge this eventually, so rebasing would be nice.
>>
>> > On Thursday, September 24, 2015 01:36:31 AM Marek Olšák wrote:
>> >> On Fri, Sep 4, 2015 at 9:47 PM, Bas Nieuwenhuizen
>> >>
>> >> <bas at basnieuwenhuizen.nl> wrote:
>> >> > The flags to be enabled in the control registers have been taken from
>> >> > Catalyst traces.
>> >> >
>> >> > Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>> >> > ---
>> >> >
>> >> > src/gallium/drivers/radeon/r600_pipe_common.h | 1 +
>> >> > src/gallium/drivers/radeon/r600_texture.c | 2 ++
>> >> > src/gallium/drivers/radeon/r600d_common.h | 1 +
>> >> > src/gallium/drivers/radeonsi/si_blit.c | 16 ++++++++++++----
>> >> > src/gallium/drivers/radeonsi/si_pipe.c | 2 ++
>> >> > src/gallium/drivers/radeonsi/si_pipe.h | 1 +
>> >> > src/gallium/drivers/radeonsi/si_state.c | 27
>> >> > +++++++++++++++++++++++---- src/gallium/drivers/radeonsi/sid.h
>> >> >
>> >> > | 1 +
>> >> >
>> >> > 8 files changed, 43 insertions(+), 8 deletions(-)
>> >> >
>> >> > diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h
>> >> > b/src/gallium/drivers/radeon/r600_pipe_common.h index f05318f..dac270e
>> >> > 100644
>> >> > --- a/src/gallium/drivers/radeon/r600_pipe_common.h
>> >> > +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
>> >> > @@ -244,6 +244,7 @@ struct r600_surface {
>> >> >
>> >> > unsigned cb_color_dim; /* EG only */
>> >> > unsigned cb_color_pitch; /* EG and later */
>> >> > unsigned cb_color_slice; /* EG and later */
>> >> >
>> >> > + unsigned cb_dcc_base; /* VI and later */
>> >> >
>> >> > unsigned cb_color_attrib; /* EG and later */
>> >> > unsigned cb_dcc_control; /* VI and later */
>> >> > unsigned cb_color_fmask; /* CB_COLORn_FMASK (EG and
>> >> > later)
>> >> > or CB_COLORn_FRAG (r600) */>
>> >> >
>> >> > diff --git a/src/gallium/drivers/radeon/r600_texture.c
>> >> > b/src/gallium/drivers/radeon/r600_texture.c index 46e735e..017f5e7
>> >> > 100644
>> >> > --- a/src/gallium/drivers/radeon/r600_texture.c
>> >> > +++ b/src/gallium/drivers/radeon/r600_texture.c
>> >> > @@ -495,6 +495,8 @@ static void vi_texture_alloc_dcc_separate(struct
>> >> > r600_common_screen *rscreen,>
>> >> >
>> >> > r600_screen_clear_buffer(rscreen, &rtex->dcc_buffer->b.b, 0,
>> >> > rtex->surface.dcc_size,>
>> >> >
>> >> > 0xFFFFFFFF, true);
>> >> >
>> >> > +
>> >> > + rtex->cb_color_info |= VI_S_028C70_DCC_ENABLE(1);
>> >> >
>> >> > }
>> >> >
>> >> > static unsigned r600_texture_get_htile_size(struct r600_common_screen
>> >> > *rscreen,>
>> >> >
>> >> > diff --git a/src/gallium/drivers/radeon/r600d_common.h
>> >> > b/src/gallium/drivers/radeon/r600d_common.h index 115042d..a3d182c
>> >> > 100644
>> >> > --- a/src/gallium/drivers/radeon/r600d_common.h
>> >> > +++ b/src/gallium/drivers/radeon/r600d_common.h
>> >> > @@ -202,6 +202,7 @@
>> >> >
>> >> > #define EG_S_028C70_FAST_CLEAR(x) (((x) & 0x1)
>> >> > <<
>> >> > 17) #define SI_S_028C70_FAST_CLEAR(x) (((x) &
>> >> > 0x1) << 13)>
>> >> >
>> >> > +#define VI_S_028C70_DCC_ENABLE(x) (((x) & 0x1)
>> >> > <<
>> >> > 28)>
>> >> >
>> >> > /*CIK+*/
>> >> > #define R_0300FC_CP_STRMOUT_CNTL 0x0300FC
>> >> >
>> >> > diff --git a/src/gallium/drivers/radeonsi/si_blit.c
>> >> > b/src/gallium/drivers/radeonsi/si_blit.c index 13bb457..98913e5 100644
>> >> > --- a/src/gallium/drivers/radeonsi/si_blit.c
>> >> > +++ b/src/gallium/drivers/radeonsi/si_blit.c
>> >> > @@ -264,7 +264,7 @@ static void si_blit_decompress_color(struct
>> >> > pipe_context *ctx,>
>> >> >
>> >> > return;
>> >> >
>> >> > for (level = first_level; level <= last_level; level++) {
>> >> >
>> >> > - if (!(rtex->dirty_level_mask & (1 << level)))
>> >> > + if (!(rtex->dirty_level_mask & (1 << level)) &&
>> >> > !(rtex->dcc_compressed_level_mask & (1 << level)))>
>> >> >
>> >> > continue;
>> >> >
>> >> > /* The smaller the mipmap level, the less layers there
>> >> > are
>> >> >
>> >> > @@ -274,6 +274,7 @@ static void si_blit_decompress_color(struct
>> >> > pipe_context *ctx,>
>> >> >
>> >> > for (layer = first_layer; layer <= checked_last_layer;
>> >> > layer++) {
>> >> >
>> >> > struct pipe_surface *cbsurf, surf_tmpl;
>> >> >
>> >> > + void * custom_blend;
>> >> >
>> >> > surf_tmpl.format = rtex->resource.b.b.format;
>> >> > surf_tmpl.u.tex.level = level;
>> >> >
>> >> > @@ -281,10 +282,17 @@ static void si_blit_decompress_color(struct
>> >> > pipe_context *ctx,>
>> >> >
>> >> > surf_tmpl.u.tex.last_layer = layer;
>> >> > cbsurf = ctx->create_surface(ctx,
>> >> > &rtex->resource.b.b, &surf_tmpl);
>> >> >
>> >> > + if(rtex->fmask.size) {
>> >> > + custom_blend =
>> >> > sctx->custom_blend_decompress; + } else
>> >> > if(rtex->dcc_buffer) {
>> >> > + /* also eliminates the fast clear if
>> >> > necessary */ + custom_blend =
>> >> > sctx->custom_blend_dcc_decompress; + } else {
>> >> > + custom_blend =
>> >> > sctx->custom_blend_fastclear; + }
>> >>
>> >> The order of these is wrong. DCC decompression should be first, which
>> >> also automatically invokes FMASK and CMASK decompression. Next should
>> >> be FMASK decompression, which automatically invokes CMASK
>> >> decompression.
>> >>
>> >> There are also 2 things that need to be changed in this file or
>> >> r600_texture.c:
>> >>
>> >> 1) CMASK fast clear isn't possible with DCC and shouldn't be used. It
>> >> should be disallowed in the commit that
>> >> enables DCC.
>> >
>> > Right, I have been looking at this since I found some related corruption
>> > last week. However, I come to a slightly different conclusion about the
>> > clear values than in your other mail:
>> >
>> > 0x00 (0, 0, 0, 0)
>> > 0x40 (0, 0, 0, 1)
>> > 0x80 (1, 1, 1, 0)
>> > 0xc0 (1, 1, 1, 1)
>> >
>> > and 0x20 for a general CMASK-like clear that uses the colorbuffer clear
>> > color.
>> No. CB always uses the clear color registers no matter what the DCC
>> buffer contains, so the only way to check if the clear color is set
>> correctly is to read it using texture units.
>>
>> > Furthermore, it seems I can eliminate these with
>> > V_028808_CB_ELIMINATE_FAST_CLEAR. I need to test though whether this works
>> > in all cases a CMASK clear would work in the non-DCC case and if there
>> > are any caveats.
>>
>> You're wasting time with CMASK fast clear. It's not officially
>> supported with DCC.
>
>
> So for fast clears other than the four simple colors, do we need to use
> DCC_DECOMPRESS instead of ELIMINATE_FAST_CLEAR, or is anything other than
> those four colors not supported for a fast clear with DCC?
DCC can only encode those 4 colors for texturing, but CB uses the
clear color registers instead and therefore supports any clear color.
>
> The reason I am interested in using ELIMINATE_FAST_CLEAR is that it is faster
> than the decompress and fglrx actually uses it. i.e. if I do
>
> loop:
> clear
> one draw
> read texture from shader
>
> I get something like the following in the CS:
>
> set CMASK to 0xFF
> loop:
> set DCC to 0x20
> draw with DCC, CMASK enabled and CB_COLOR_CONTROL=0xcc0010 (i.e.CB_NORMAL)
> draw with DCC, CMASK enabled and CB_COLOR_CONTROL=0xcc0020
> (i.e.CB_ELIMINATE_FAST_CLEAR)
> other draws etc.
>
> So I would suppose that this is actually stable enough to be used. I would be
> fine with implementing an alternative for this series and revisiting this issue
> at a later date.
Interesting. I think ELIMINATE_FAST_CLEAR requires at least one of DCC
or CMASK, but not both. I'm just guessing here.
0x20 means (0,0,0,0), but CB uses the clear color registers, so
(0,0,0,0) is never actually used by CB.
Marek
More information about the mesa-dev
mailing list