[Intel-gfx] [PATCH v6 09/10] drm/framebuffer/tgl: Format modifier for Intel Gen 12 render compression with Clear Color

[Sripada, Radhakrishna]

Hi Nanley,

> -----Original Message-----
> From: Chery, Nanley G
> Sent: Tuesday, October 29, 2019 5:05 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan at intel.com>; Syrjala, Ville
> <ville.syrjala at intel.com>; Sharma, Shashank
> <shashank.sharma at intel.com>; Antognolli, Rafael
> <rafael.antognolli at intel.com>; Ville Syrjala <ville.syrjala at linux.intel.com>;
> Ekstrand, Jason <jason.ekstrand at intel.com>
> Subject: RE: [PATCH v6 09/10] drm/framebuffer/tgl: Format modifier for Intel
> Gen 12 render compression with Clear Color
> 
> +Jason
> 
> Maybe Jason and/or others have some thoughts on the following? Given the
> detailed description of the clear color struct, it seems like we'll have to define
> a new modifier if the struct fields change in a future generation.
> 
> On negative is that we might have to make new modifiers that provide no
> additional benefit (assuming the new/changed fields are unused). On
> positive is that it's probably a good thing that we mentioned the Raw clear
> color fields because I think 3D uses it during rendering operations and we'll
> be sharing from 3D-to-3D. Maybe we should specify how the channels
> should be formatted?
> 
> I haven't thought through how fields like Color Discard Enable affect buffer
> sharing...
> 
> Another comment: I noticed that none of the Y+CCS modifiers explicitly state
> that the CCS must be in the same BO as the Y-tiled main surface. We should
> at least fix that for newly defined modifiers right?
I have not tried to use separate bo for the different surfaces. That is probably worth a try.
I wonder if we are using that for any of the modifiers to have an explicit comment?

Thanks,
RK
> 
> -Nanley
> 
> > ________________________________________
> > From: Sripada, Radhakrishna
> > Sent: Monday, October 28, 2019 1:40 PM
> > To: intel-gfx at lists.freedesktop.org
> > Cc: Pandiyan, Dhinakaran; Syrjala, Ville; Sharma, Shashank;
> > Antognolli, Rafael; Chery, Nanley G; Sripada, Radhakrishna; Ville
> > Syrjala; Kondapally, Kalyan
> > Subject: [PATCH v6 09/10] drm/framebuffer/tgl: Format modifier for
> > Intel Gen 12 render compression with Clear Color
> >
> > 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>
> > ---
> >  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 1aa6d468c048..4aa7f3f9712a
> > 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -434,6 +434,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.20.1
> >
> >