[Mesa-dev] [PATCH 01/12] i965/rbc: Allow integer formats as advertised in isl_format.c

[Jason Ekstrand]

On Sep 2, 2016 8:55 AM, "Pohjolainen, Topi" <topi.pohjolainen at gmail.com>
wrote:
>
> On Fri, Sep 02, 2016 at 07:59:15AM -0700, Jason Ekstrand wrote:
> >    On Sep 2, 2016 3:07 AM, "Pohjolainen, Topi"
> >    <[1]topi.pohjolainen at gmail.com> wrote:
> >    >
> >    > On Fri, Sep 02, 2016 at 08:34:27AM +0300, Pohjolainen, Topi wrote:
> >    > > On Thu, Sep 01, 2016 at 02:33:48PM -0700, Jason Ekstrand wrote:
> >    > > >    On Wed, Aug 31, 2016 at 8:17 AM, Topi Pohjolainen
> >    > > >    <[1][2]topi.pohjolainen at intel.com> wrote:
> >    > > >
> >    > > >      From: Topi Pohjolainen <[2][3]topi.pohjolainen at gmail.com>
> >    > > >      Blorp consults brw_is_color_fast_clear_compatible() to see
> >    if any
> >    > > >      restrictions apply for fast clear in addition to the
> >    capablities
> >    > > >      advertised in isl_format.c::format_info[]. On Gen8+
integer
> >    formats
> >    > > >      are backlisted for plain old fast clear but there is no
> >    reason why
> >    > > >      lossless compression shouldn't be supported. In fact,
> >    lossless
> >    > > >      compression of integer formats is already supported for
> >    normal
> >    > > >      render paths.
> >    > > >      This patch prepares for dropping the delayed allocating of
> >    the mcs
> >    > > >      buffer for lossless compression. Until now the skip of
fast
> >    clear
> >    > > >      also prevented the mcs being allocated and hence the
> >    lossless
> >    > > >      compression being effectively turned off for integer
> >    formats.
> >    > > >      Once the mcs buffer is allocated beforehand, the assertion
> >    addressed
> >    > > >      here would start triggering.
> >    > > >      Signed-off-by: Topi Pohjolainen
> >    <[3][4]topi.pohjolainen at intel.com>
> >    > > >      ---
> >    > > >       src/mesa/drivers/dri/i965/brw_blorp.c | 6 +++++-
> >    > > >       1 file changed, 5 insertions(+), 1 deletion(-)
> >    > > >      diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> >    > > >      b/src/mesa/drivers/dri/i965/brw_blorp.c
> >    > > >      index c902f2e..7e257e9 100644
> >    > > >      --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> >    > > >      +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> >    > > >      @@ -761,8 +761,12 @@ do_single_blorp_clear(struct
> >    brw_context *brw,
> >    > > >      struct gl_framebuffer *fb,
> >    > > >                /* Compressed buffers can be cleared also using
> >    normal
> >    > > >      rep-clear. In
> >    > > >                 * such case they bahave such as if they were
> >    drawn using
> >    > > >      normal 3D
> >    > > >                 * render pipeline, and we simply mark the mcs
as
> >    dirty.
> >    > > >      +          *
> >    > > >      +          * Fast clear of integer formats is not
supported
> >    on
> >    > > >      Gen8+. See
> >    > > >      +          * brw_is_color_fast_clear_compatible(). This,
> >    however,
> >    > > >      doesn't
> >    > > >      +          * prevent lossless compression on Gen9+.
> >    > > >                 */
> >    > > >      -         assert(partial_clear);
> >    > > >      +         assert(partial_clear ||
> >    _mesa_is_format_integer_color(
> >    > > >      format));
> >    > > >
> >    > > >    How about
assert(isl_format_supports_lossless_compression())?
> >    > >
> >    > > Instead of _mesa_is_format_integer_color()? Sounds good to me.
> >    >
> >    > Well, I suppose we could just drop the assert? This is in a
> >    conditional
> >    > block:
> >
> >    Yeah, really the code that's protected by the assert has nothing to
do
> >    with fast clears.  Instead, it's just "you rendered into a compressed
> >    render target with compression, flag it as unresolved".  We probably
> >    need similar code in brw_blorp_blit_miptrees of we don't have it
> >    already.
>
> Just in the end of brw_blorp_blit_miptrees() we have:
>
>   if (intel_miptree_is_lossless_compressed(brw, dst_mt))
>       dst_mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_UNRESOLVED;

Awesome! I didn't bother to look. :-/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160902/31d4058d/attachment.html>