[Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color

Tue Dec 1 00:18:26 UTC 2020

> -----Original Message-----
> From: Imre Deak <imre.deak at intel.com>
> Sent: Friday, November 27, 2020 10:06 AM
> To: Daniel Vetter <daniel at ffwll.ch>; Chery, Nanley G
> <nanley.g.chery at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>; Daniel
> Vetter <daniel.vetter at ffwll.ch>; Rafael Antognolli
> <rafael.antognolli at intel.com>; Kondapally, Kalyan
> <kalyan.kondapally at intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan at intel.com>; dri-devel at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for Intel
> Gen 12 render compression with Clear Color
> 
> On Fri, Nov 27, 2020 at 04:19:20PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 27, 2020 at 04:31:00PM +0200, Imre Deak wrote:
> > > Hi Daniel, Jani,
> > >
> > > is it ok to merge this patch along with 2/2 via the i915 tree?
> >
> > Ack from mesa (userspace in general, but mesa is kinda mandatory) is
> > missing I think. With that
> > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Thanks.
> 
> Nanley, could you ACK the patchset if they look ok from Mesa's POV? It
> works as expected at least with the igt/kms_ccs RC-CC subtest.
> 

Hi Imre,

I have a question and a couple comments:

Is the map of the clear color address creating a new synchronization point between the GPU and CPU? If so, I wonder how this will impact performance. There was some talk of asynchronously updating the clear color register a while back. 

We probably don't have to update the header, but we noticed in our testing that the clear color prefers an alignment greater than 64B. Unfortunately, I can't find any bspec note about this. As long as the buffer creators are aware though, I think we should be fine. I don't know if this is the best forum to bring it up, but I thought I'd share.

Seems like the upper converted clear color is untested due to the lack of RGBX16 support. I suppose that if there are any issues there, they can be fixed later...

-Nanley

> --Imre
> 
> > > On Mon, Nov 23, 2020 at 08:26:30PM +0200, Imre Deak wrote:
> > > > From: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> > > >
> > > > Gen12 display can decompress surfaces compressed by render engine with
> > > > Clear Color, add a new modifier as the driver needs to know the surface
> > > > was compressed by render engine.
> > > >
> > > > V2: Description changes as suggested by Rafael.
> > > > V3: Mention the Clear Color size of 64 bits in the comments(DK)
> > > > v4: Fix trailing whitespaces
> > > > v5: Explain Clear Color in the documentation.
> > > > v6: Documentation Nitpicks(Nanley)
> > > >
> > > > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > Cc: Kalyan Kondapally <kalyan.kondapally at intel.com>
> > > > Cc: Rafael Antognolli <rafael.antognolli at intel.com>
> > > > Cc: Nanley Chery <nanley.g.chery at intel.com>
> > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > > ---
> > > >  include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++
> > > >  1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/include/uapi/drm/drm_fourcc.h
> b/include/uapi/drm/drm_fourcc.h
> > > > index ca48ed0e6bc1..0a1b2c4c4bee 100644
> > > > --- a/include/uapi/drm/drm_fourcc.h
> > > > +++ b/include/uapi/drm/drm_fourcc.h
> > > > @@ -527,6 +527,25 @@ extern "C" {
> > > >   */
> > > >  #define I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS
> fourcc_mod_code(INTEL, 7)
> > > >
> > > > +/*
> > > > + * Intel Color Control Surface with Clear Color (CCS) for Gen-12 render
> > > > + * compression.
> > > > + *
> > > > + * The main surface is Y-tiled and is at plane index 0 whereas CCS is linear
> > > > + * and at index 1. The clear color is stored at index 2, and the pitch should
> > > > + * be ignored. The clear color structure is 256 bits. The first 128 bits
> > > > + * represents Raw Clear Color Red, Green, Blue and Alpha color each
> represented
> > > > + * by 32 bits. The raw clear color is consumed by the 3d engine and
> generates
> > > > + * the converted clear color of size 64 bits. The first 32 bits store the
> Lower
> > > > + * Converted Clear Color value and the next 32 bits store the Higher
> Converted
> > > > + * Clear Color value when applicable. The Converted Clear Color values
> are
> > > > + * consumed by the DE. The last 64 bits are used to store Color Discard
> Enable
> > > > + * and Depth Clear Value Valid which are ignored by the DE. A CCS cache
> line
> > > > + * corresponds to an area of 4x1 tiles in the main surface. The main
> surface
> > > > + * pitch is required to be a multiple of 4 tile widths.
> > > > + */
> > > > +#define I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC
> fourcc_mod_code(INTEL, 8)
> > > > +
> > > >  /*
> > > >   * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> > > >   *
> > > > --
> > > > 2.25.1
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch