Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-10 Thread Axel Davy 

On 10/10/2015 17:49, Marek Olšák wrote:

On Sat, Oct 10, 2015 at 4:15 PM, Bas Nieuwenhuizen
 wrote:

Hi Marek,

The revised series is mostly done. I wanted to do more testing and to
try to make sure that the added cache flushes I am doing now (a
CACHE_FLUSH event before a fast clear and on switching framebuffers)
are the minimal needed.


Also, it looks like we don't need DCC decompression at all, right? It
might be better to get rid of it and only use the 3D engine to access
DCC-encoded surfaces.

I still use it for flush_resource. I could make this redundant by
sharing the DCC buffer by appending the DCC buffer to the texture
resource similarly to how the CMASK is appended to the resource of a
MSAA buffer. This has the secondary benefit of not needing to
reference as many resources for command submission.

IIRC, flush_resource is only used for shared (scanout) surfaces where
DCC is always disabled.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

I think it's not a very good idea to rely on that.

It may be true for now, but may change in the future:
For example, perhaps some day wayland will tell egl
the app is not fullscreen and that a non-scanoutable buffer
can be used.

Axel Davy
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-10 Thread Marek Olšák 
On Sat, Oct 10, 2015 at 6:12 PM, Axel Davy  wrote:
> On 10/10/2015 17:49, Marek Olšák wrote:
>>
>> On Sat, Oct 10, 2015 at 4:15 PM, Bas Nieuwenhuizen
>>  wrote:
>>>
>>> Hi Marek,
>>>
>>> The revised series is mostly done. I wanted to do more testing and to
>>> try to make sure that the added cache flushes I am doing now (a
>>> CACHE_FLUSH event before a fast clear and on switching framebuffers)
>>> are the minimal needed.
>>>
 Also, it looks like we don't need DCC decompression at all, right? It
 might be better to get rid of it and only use the 3D engine to access
 DCC-encoded surfaces.
>>>
>>> I still use it for flush_resource. I could make this redundant by
>>> sharing the DCC buffer by appending the DCC buffer to the texture
>>> resource similarly to how the CMASK is appended to the resource of a
>>> MSAA buffer. This has the secondary benefit of not needing to
>>> reference as many resources for command submission.
>>
>> IIRC, flush_resource is only used for shared (scanout) surfaces where
>> DCC is always disabled.
>>
>> Marek
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> I think it's not a very good idea to rely on that.
>
> It may be true for now, but may change in the future:
> For example, perhaps some day wayland will tell egl
> the app is not fullscreen and that a non-scanoutable buffer
> can be used.

If that happens, we'll implement DCC sharing between processes.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-10 Thread Marek Olšák 
On Sat, Oct 10, 2015 at 5:49 PM, Marek Olšák  wrote:
> On Sat, Oct 10, 2015 at 4:15 PM, Bas Nieuwenhuizen
>  wrote:
>> Hi Marek,
>>
>> The revised series is mostly done. I wanted to do more testing and to
>> try to make sure that the added cache flushes I am doing now (a
>> CACHE_FLUSH event before a fast clear and on switching framebuffers)
>> are the minimal needed.
>>
>>> Also, it looks like we don't need DCC decompression at all, right? It
>>> might be better to get rid of it and only use the 3D engine to access
>>> DCC-encoded surfaces.
>>
>> I still use it for flush_resource. I could make this redundant by
>> sharing the DCC buffer by appending the DCC buffer to the texture
>> resource similarly to how the CMASK is appended to the resource of a
>> MSAA buffer. This has the secondary benefit of not needing to
>> reference as many resources for command submission.
>
> IIRC, flush_resource is only used for shared (scanout) surfaces where
> DCC is always disabled.

That said, we might need to keep the DCC decompression for image store
instructions, which don't support compression.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-10 Thread Bas Nieuwenhuizen 
Hi Marek,

The revised series is mostly done. I wanted to do more testing and to
try to make sure that the added cache flushes I am doing now (a
CACHE_FLUSH event before a fast clear and on switching framebuffers)
are the minimal needed.

> Also, it looks like we don't need DCC decompression at all, right? It
> might be better to get rid of it and only use the 3D engine to access
> DCC-encoded surfaces.

I still use it for flush_resource. I could make this redundant by
sharing the DCC buffer by appending the DCC buffer to the texture
resource similarly to how the CMASK is appended to the resource of a
MSAA buffer. This has the secondary benefit of not needing to
reference as many resources for command submission.

Yours sincerely,
Bas Nieuwenhuizen
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-10 Thread Marek Olšák 
On Sat, Oct 10, 2015 at 4:15 PM, Bas Nieuwenhuizen
 wrote:
> Hi Marek,
>
> The revised series is mostly done. I wanted to do more testing and to
> try to make sure that the added cache flushes I am doing now (a
> CACHE_FLUSH event before a fast clear and on switching framebuffers)
> are the minimal needed.
>
>> Also, it looks like we don't need DCC decompression at all, right? It
>> might be better to get rid of it and only use the 3D engine to access
>> DCC-encoded surfaces.
>
> I still use it for flush_resource. I could make this redundant by
> sharing the DCC buffer by appending the DCC buffer to the texture
> resource similarly to how the CMASK is appended to the resource of a
> MSAA buffer. This has the secondary benefit of not needing to
> reference as many resources for command submission.

IIRC, flush_resource is only used for shared (scanout) surfaces where
DCC is always disabled.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-10-08 Thread Marek Olšák 
Hi Bas,

How's it going?

I've gotten a response from Catalyst devs and they're saying CMASK is
unnecessary here. If it's always cleared to 0xff, it's better to
disable it to save some bandwidth.

Also, it looks like we don't need DCC decompression at all, right? It
might be better to get rid of it and only use the 3D engine to access
DCC-encoded surfaces.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-09-24 Thread Bas Nieuwenhuizen 
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.

On Thursday, September 24, 2015 01:36:31 AM Marek Olšák wrote:
> On Fri, Sep 4, 2015 at 9:47 PM, Bas Nieuwenhuizen
> 
>  wrote:
> > The flags to be enabled in the control registers have been taken from
> > Catalyst traces.
> > 
> > Signed-off-by: Bas Nieuwenhuizen 
> > ---
> > 
> >  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, >dcc_buffer->b.b, 0,
> > rtex->surface.dcc_size,> 
> >  0x, 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_CNTL0x0300FC
> > 
> > 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,
> > >resource.b.b, _tmpl);
> > 
> > +   if(rtex->fmask.size) {
> > +

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-09-24 Thread Marek Olšák 
On Thu, Sep 24, 2015 at 2:15 PM, Bas Nieuwenhuizen
 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
>>
>>  wrote:
>> > The flags to be enabled in the control registers have been taken from
>> > Catalyst traces.
>> >
>> > Signed-off-by: Bas Nieuwenhuizen 
>> > ---
>> >
>> >  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, >dcc_buffer->b.b, 0,
>> > rtex->surface.dcc_size,>
>> >  0x, 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_CNTL0x0300FC
>> >
>> > 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 =

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-09-24 Thread Bas Nieuwenhuizen 
On Thursday, September 24, 2015 07:24:50 PM Marek Olšák wrote:
> On Thu, Sep 24, 2015 at 2:15 PM, Bas Nieuwenhuizen
> 
>  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
> >> 
> >>  wrote:
> >> > The flags to be enabled in the control registers have been taken from
> >> > Catalyst traces.
> >> > 
> >> > Signed-off-by: Bas Nieuwenhuizen 
> >> > ---
> >> > 
> >> >  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, >dcc_buffer->b.b, 0,
> >> > rtex->surface.dcc_size,>
> >> > 
> >> >  0x, 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_CNTL0x0300FC
> >> > 
> >> > 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;
> >> >

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-09-24 Thread Marek Olšák 
On Thu, Sep 24, 2015 at 9:47 PM, Bas Nieuwenhuizen
 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
>>
>>  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
>> >>
>> >>  wrote:
>> >> > The flags to be enabled in the control registers have been taken from
>> >> > Catalyst traces.
>> >> >
>> >> > Signed-off-by: Bas Nieuwenhuizen 
>> >> > ---
>> >> >
>> >> >  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, >dcc_buffer->b.b, 0,
>> >> > rtex->surface.dcc_size,>
>> >> >
>> >> >  0x, 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_CNTL0x0300FC
>> >> >
>> >> > 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
>>

Re: [Mesa-dev] [PATCH 3/8] radeonsi: Enable DCC.

2015-09-23 Thread Marek Olšák 
On Fri, Sep 4, 2015 at 9:47 PM, Bas Nieuwenhuizen
 wrote:
> The flags to be enabled in the control registers have been taken from
> Catalyst traces.
>
> Signed-off-by: Bas Nieuwenhuizen 
> ---
>  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, >dcc_buffer->b.b, 0, 
> rtex->surface.dcc_size,
>  0x, 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_CNTL0x0300FC
> 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, 
> >resource.b.b, _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