Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Render decompression support for Gen9 and above

[Kannan, Vandana]

> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Friday, April 29, 2016 8:15 PM
> To: Kannan, Vandana 
> Cc: intel-gfx@lists.freedesktop.org; Smith, Gary K
> 
> Subject: Re: [PATCH v2 2/2] drm/i915: Render decompression support for
> Gen9 and above
> 
> On Fri, Apr 29, 2016 at 08:27:00PM +0530, Vandana Kannan wrote:
> > This patch includes enabling render decompression (RC) after checking
> > all the requirements (format, tiling, rotation etc.).
> >
> > TODO:
> > 1. Disable stereo 3D when render decomp is enabled (bit 7:6) 2. Render
> > decompression must not be used in VTd pass-through mode 3. Program
> > hashing select CHICKEN_MISC1 bit 15 4. For Gen10, add support for RGB
> > 1010102 5. RC-FBC workaround 6. RC watermark calculation
> >
> > The reason for using a plane property instead of fb modifier:- In
> > Android, OGL passes a render compressed buffer to hardware composer
> > (HWC), which would then request a flip on that buffer after checking
> > if the target can support render compressed buffer. For example, only
> > planes
> > 1 and 2 on pipes 1 and 2 can support RC. In case the target cannot
> > support it, HWC will revert back to OGL requesting for uncompressed
> > buffer.
> > Here,
> > if plane property is used, OGL would send back the buffer (same ID)
> > after decompression, marked uncompressed. If fb modifier was used, a
> > different version of the buffer would have to be maintained for
> > different combinations - in the simple case of render compressed vs
> > uncompressed buffer, there would be 2 fbs with 2 IDs. So, in this
> > case, OGL would give a reference to a fb with a different ID.
> > To avoid the difficulty of keeping track of multiple fbs and the
> > subsequent complexity in debug, the architecture forum decided to go
> > ahead with a plane property for RC.
> >
> > [Mayuresh] Added the plane check in skl_check_compression()
> >
> > v2: Ville's review comments addressed
> > - Removed WAs specific to SKL-C and BXT-A
> > - Assign capabilities according to pipe and plane during property
> > creation
> > - in skl_check_compression(), check for conditions that must be
> > satisifed
> >
> > Maintaining the check for pixel format, even though compressed fb of
> > format other RGB should not have been created, to be on the safer
> side.
> > Added checks for BGR too.
> > Bitmask is being used for the property to accommodate 2 more options
> > expected to be added in future.
> >
> > This patch depends on Ville's patch series on fb->offsets.
> > (Ref: git://github.com/vsyrjala/linux.git fb_offsets_14)
> >
> > Testing is in progress for v2 of the patch.
> >
> > Signed-off-by: Vandana Kannan 
> > Cc: Ville Syrjälä 
> > Cc: Smith, Gary K 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |   1 +
> >  drivers/gpu/drm/i915/i915_reg.h   |  22 +
> >  drivers/gpu/drm/i915/intel_atomic_plane.c |  24 +-
> >  drivers/gpu/drm/i915/intel_display.c  | 135
> +-
> >  drivers/gpu/drm/i915/intel_drv.h  |  10 +++
> >  drivers/gpu/drm/i915/intel_sprite.c   |  27 +-
> >  6 files changed, 213 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 32f0597..ba32e7c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1901,6 +1901,7 @@ struct drm_i915_private {
> >
> > struct drm_property *broadcast_rgb_property;
> > struct drm_property *force_audio_property;
> > +   struct drm_property *render_comp_property;
> >
> > /* hda/i915 audio component */
> > struct i915_audio_component *audio_component; diff --git
> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 25e229b..da45cc9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5816,6 +5816,28 @@ enum skl_disp_power_wells {
> > _ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
> > _ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
> >
> > +#define PLANE_AUX_DIST_1_A 0x701c0
> > +#define PLANE_AUX_DIST_2_A 0x702c0
> > +#define PLANE_AUX_DIST_1_B 0x711c0
> > +#define PLANE_AUX_DIST_2_B 0x712c0
> > +#define _PLANE_AUX_DIST_1(pipe) \
> > +   _PIPE(pipe, PLANE_AUX_DIST_1_A,
> PLANE_AUX_DIST_1_B) #define
> > +_PLANE_AUX_DIST_2(pipe) \
> > +   _PIPE(pipe, PLANE_AUX_DIST_2_A,
> PLANE_AUX_DIST_2_B)
> > +#define PLANE_AUX_DIST(pipe, plane) \
> > +   _MMIO_PLANE(plane, _PLANE_AUX_DIST_1(pipe),
> _PLANE_AUX_DIST_2(pipe))
> > +
> > +#define PLANE_AUX_OFFSET_1_A   0x701c4
> > +#define PLANE_AUX_OFFSET_2_A   0x702c4
> > +#define PLANE_AUX_OFFSET_1_B   0x711c4
> > +#define 

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Render decompression support for Gen9 and above

[Ville Syrjälä]

On Fri, Apr 29, 2016 at 08:27:00PM +0530, Vandana Kannan wrote:
> This patch includes enabling render decompression (RC) after checking
> all the requirements (format, tiling, rotation etc.).
> 
> TODO:
> 1. Disable stereo 3D when render decomp is enabled (bit 7:6)
> 2. Render decompression must not be used in VTd pass-through mode
> 3. Program hashing select CHICKEN_MISC1 bit 15
> 4. For Gen10, add support for RGB 1010102
> 5. RC-FBC workaround
> 6. RC watermark calculation
> 
> The reason for using a plane property instead of fb modifier:-
> In Android, OGL passes a render compressed buffer to hardware composer
> (HWC), which would then request a flip on that buffer after checking if
> the target can support render compressed buffer. For example, only planes
> 1 and 2 on pipes 1 and 2 can support RC. In case the target cannot
> support it, HWC will revert back to OGL requesting for uncompressed
> buffer.
> Here,
> if plane property is used, OGL would send back the buffer (same ID) after
> decompression, marked uncompressed. If fb modifier was used, a different
> version of the buffer would have to be maintained for different
> combinations - in the simple case of render compressed vs uncompressed
> buffer, there would be 2 fbs with 2 IDs. So, in this case, OGL would give
> a reference to a fb with a different ID.
> To avoid the difficulty of keeping track of multiple fbs and the subsequent
> complexity in debug, the architecture forum decided to go ahead with a
> plane property for RC.
> 
> [Mayuresh] Added the plane check in skl_check_compression()
> 
> v2: Ville's review comments addressed
> - Removed WAs specific to SKL-C and BXT-A
> - Assign capabilities according to pipe and plane during property creation
> - in skl_check_compression(), check for conditions that must be satisifed
> 
> Maintaining the check for pixel format, even though compressed fb of format
> other RGB should not have been created, to be on the safer side.
> Added checks for BGR too.
> Bitmask is being used for the property to accommodate 2 more options
> expected to be added in future.
> 
> This patch depends on Ville's patch series on fb->offsets.
> (Ref: git://github.com/vsyrjala/linux.git fb_offsets_14)
> 
> Testing is in progress for v2 of the patch.
> 
> Signed-off-by: Vandana Kannan 
> Cc: Ville Syrjälä 
> Cc: Smith, Gary K 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   1 +
>  drivers/gpu/drm/i915/i915_reg.h   |  22 +
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  24 +-
>  drivers/gpu/drm/i915/intel_display.c  | 135 
> +-
>  drivers/gpu/drm/i915/intel_drv.h  |  10 +++
>  drivers/gpu/drm/i915/intel_sprite.c   |  27 +-
>  6 files changed, 213 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 32f0597..ba32e7c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1901,6 +1901,7 @@ struct drm_i915_private {
>  
>   struct drm_property *broadcast_rgb_property;
>   struct drm_property *force_audio_property;
> + struct drm_property *render_comp_property;
>  
>   /* hda/i915 audio component */
>   struct i915_audio_component *audio_component;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 25e229b..da45cc9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5816,6 +5816,28 @@ enum skl_disp_power_wells {
>   _ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
>   _ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
>  
> +#define PLANE_AUX_DIST_1_A   0x701c0
> +#define PLANE_AUX_DIST_2_A   0x702c0
> +#define PLANE_AUX_DIST_1_B   0x711c0
> +#define PLANE_AUX_DIST_2_B   0x712c0
> +#define _PLANE_AUX_DIST_1(pipe) \
> + _PIPE(pipe, PLANE_AUX_DIST_1_A, PLANE_AUX_DIST_1_B)
> +#define _PLANE_AUX_DIST_2(pipe) \
> + _PIPE(pipe, PLANE_AUX_DIST_2_A, PLANE_AUX_DIST_2_B)
> +#define PLANE_AUX_DIST(pipe, plane) \
> + _MMIO_PLANE(plane, _PLANE_AUX_DIST_1(pipe), _PLANE_AUX_DIST_2(pipe))
> +
> +#define PLANE_AUX_OFFSET_1_A 0x701c4
> +#define PLANE_AUX_OFFSET_2_A 0x702c4
> +#define PLANE_AUX_OFFSET_1_B 0x711c4
> +#define PLANE_AUX_OFFSET_2_B 0x712c4
> +#define _PLANE_AUX_OFFSET_1(pipe)   \
> + _PIPE(pipe, PLANE_AUX_OFFSET_1_A, PLANE_AUX_OFFSET_1_B)
> +#define _PLANE_AUX_OFFSET_2(pipe)   \
> + _PIPE(pipe, PLANE_AUX_OFFSET_2_A, PLANE_AUX_OFFSET_2_B)
> +#define PLANE_AUX_OFFSET(pipe, plane)   \
> + _MMIO_PLANE(plane, _PLANE_AUX_OFFSET_1(pipe), _PLANE_AUX_OFFSET_2(pipe))
> +
>  /* legacy palette */
>  #define _LGC_PALETTE_A   0x4a000
>  #define _LGC_PALETTE_B   0x4a800
> diff --git