Hi Ville,

Did you get a chance to review this patch?

- Vandana

> -----Original Message-----
> From: Kannan, Vandana
> Sent: Friday, May 13, 2016 7:35 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Kannan, Vandana <vandana.kan...@intel.com>; Ville Syrjälä
> <ville.syrj...@linux.intel.com>; Smith, Gary K <gary.k.sm...@intel.com>
> Subject: [PATCH v3 2/2] drm/i915: Render decompression support for
> Gen9 and above
> 
> 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 RGB8888 should not have been created, to be on the safer
> side.
> Added checks for BGR8888 too.
> Bitmask is being used for the property to accommodate 2 more options
> expected to be added in future.
> 
> v3: This patch has been implemented on top of Ville's patch series on
> fb->offsets.
> (Ref: git://github.com/vsyrjala/linux.git fb_offsets_15)
> - Userspace is expected to pass aux offset through fb->offsets[1].
> 
> Testing is in progress for v3 of the patch.
> 
> Signed-off-by: Vandana Kannan <vandana.kan...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Cc: Smith, Gary K <gary.k.sm...@intel.com>
> ---
>  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      | 111
> ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h          |  10 +++
>  drivers/gpu/drm/i915/intel_sprite.c       |  13 ++++
>  6 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 7a0b513..0465e0f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1915,6 +1915,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 54ce0b1..9d008e1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5818,6 +5818,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 a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7de7721..2617b75 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -228,8 +228,16 @@ intel_plane_atomic_get_property(struct
> drm_plane *plane,
>                               struct drm_property *property,
>                               uint64_t *val)
>  {
> -     DRM_DEBUG_KMS("Unknown plane property '%s'\n", property-
> >name);
> -     return -EINVAL;
> +     struct drm_i915_private *dev_priv = state->plane->dev-
> >dev_private;
> +     struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> +     if (property == dev_priv->render_comp_property) {
> +             *val = intel_state->render_comp_enable;
> +     } else {
> +             DRM_DEBUG_KMS("Unknown plane property '%s'\n",
> property->name);
> +             return -EINVAL;
> +     }
> +     return 0;
>  }
> 
>  /**
> @@ -250,6 +258,14 @@ intel_plane_atomic_set_property(struct drm_plane
> *plane,
>                               struct drm_property *property,
>                               uint64_t val)
>  {
> -     DRM_DEBUG_KMS("Unknown plane property '%s'\n", property-
> >name);
> -     return -EINVAL;
> +     struct drm_i915_private *dev_priv = state->plane->dev-
> >dev_private;
> +     struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> +     if (property == dev_priv->render_comp_property) {
> +             intel_state->render_comp_enable = val;
> +     } else {
> +             DRM_DEBUG_KMS("Unknown plane property '%s'\n",
> property->name);
> +             return -EINVAL;
> +     }
> +     return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 33dad36..f006724 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -117,6 +117,7 @@ static void ironlake_pfit_disable(struct intel_crtc
> *crtc, bool force);  static void ironlake_pfit_enable(struct intel_crtc 
> *crtc);
> static void intel_modeset_setup_hw_state(struct drm_device *dev);  static
> void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
> +static int skl_check_compression(struct intel_plane_state
> +*plane_state);
> 
>  typedef struct {
>       int     min, max;
> @@ -3032,6 +3033,15 @@ int skl_check_plane_surface(struct
> intel_plane_state *plane_state)
>               ret = skl_check_nv12_aux_surface(plane_state);
>               if (ret)
>                       return ret;
> +     } else if (fb && plane_state->render_comp_enable) {
> +             ret = skl_check_compression(plane_state);
> +             if (ret) {
> +                     DRM_DEBUG_KMS("Render compr checks failed\n");
> +                     return ret;
> +             }
> +             plane_state->aux.offset = fb->offsets[1];
> +             plane_state->aux.x = 0;
> +             plane_state->aux.y = 0;
>       } else {
>               plane_state->aux.offset = ~0xfff;
>               plane_state->aux.x = 0;
> @@ -3424,6 +3434,7 @@ static void skylake_update_primary_plane(struct
> drm_plane *plane,
>       u32 plane_ctl;
>       unsigned int rotation = plane_state->base.rotation;
>       u32 stride = skl_plane_stride(fb, 0, rotation);
> +     u32 aux_stride = skl_plane_stride(fb, 1, rotation);
>       u32 surf_addr = plane_state->main.offset;
>       int scaler_id = plane_state->scaler_id;
>       int src_x = plane_state->main.x;
> @@ -3453,10 +3464,19 @@ static void
> skylake_update_primary_plane(struct drm_plane *plane,
>       intel_crtc->adjusted_x = src_x;
>       intel_crtc->adjusted_y = src_y;
> 
> +     if (plane_state->render_comp_enable)
> +             plane_ctl |= PLANE_CTL_DECOMPRESSION_ENABLE;
> +     else
> +             plane_ctl &= ~PLANE_CTL_DECOMPRESSION_ENABLE;
> +
>       I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>       I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
>       I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
>       I915_WRITE(PLANE_SIZE(pipe, 0), (src_h << 16) | src_w);
> +     I915_WRITE(PLANE_AUX_DIST(pipe, 0),
> +                             plane_state->aux.offset | aux_stride);
> +     I915_WRITE(PLANE_AUX_OFFSET(pipe, 0),
> +                             plane_state->aux.y <<16 | plane_state-
> >aux.x);
> 
>       if (scaler_id >= 0) {
>               uint32_t ps_ctrl = 0;
> @@ -12189,6 +12209,8 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>                       return ret;
>       }
> 
> +
> +
>       was_visible = old_plane_state->visible;
>       visible = to_intel_plane_state(plane_state)->visible;
> 
> @@ -14318,6 +14340,54 @@ skl_max_scale(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state
>       return max_scale;
>  }
> 
> +int skl_check_compression(struct intel_plane_state *plane_state) {
> +     struct drm_framebuffer *fb = plane_state->base.fb;
> +     int x_offset = plane_state->src.x1 >> 17;
> +     int src_w = drm_rect_width(&plane_state->src) >> 16;
> +
> +     /*
> +      * 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
> +      */
> +
> +     if (plane_state->base.rotation &
> +                     ~(BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180))) {
> +             DRM_DEBUG_KMS("RC support only with 0/180 degree
> rotation\n");
> +             return -EINVAL;
> +     }
> +
> +     if ((fb->modifier[0] != I915_FORMAT_MOD_Y_TILED) &&
> +                     (fb->modifier[0] != I915_FORMAT_MOD_Yf_TILED)) {
> +             DRM_DEBUG_KMS("RC supported only with Y-tile\n");
> +             return -EINVAL;
> +     }
> +
> +     if ((fb->pixel_format != DRM_FORMAT_XBGR8888) &&
> +                     (fb->pixel_format != DRM_FORMAT_ABGR8888) &&
> +                     (fb->pixel_format != DRM_FORMAT_XRGB8888) &&
> +                     (fb->pixel_format != DRM_FORMAT_ARGB8888)) {
> +             DRM_DEBUG_KMS("RC supported only with RGB8888
> formats\n");
> +             return -EINVAL;
> +     }
> +
> +     /*
> +      * For SKL & BXT,
> +      * When the render compression is enabled with plane
> +      * width greater than 3840 and horizontal panning,
> +      * the stride programmed in the PLANE_STRIDE register
> +      * must be multiple of 4.
> +      */
> +     if (src_w > 3840 && x_offset != 0) {
> +             DRM_DEBUG_KMS("RC: width > 3840, horizontal
> panning\n");
> +             return -EINVAL;
> +     }
> +
> +     return 0;
> +}
> +
>  static int
>  intel_check_primary_plane(struct drm_plane *plane,
>                         struct intel_crtc_state *crtc_state, @@ -14487,6
> +14557,9 @@ static struct drm_plane *intel_primary_plane_create(struct
> drm_device *dev,
>       if (INTEL_INFO(dev)->gen >= 4)
>               intel_create_rotation_property(dev, primary);
> 
> +     if (INTEL_INFO(dev)->gen >= 9)
> +             intel_create_render_comp_property(dev, primary);
> +
>       drm_plane_helper_add(&primary->base,
> &intel_plane_helper_funcs);
> 
>       return &primary->base;
> @@ -14516,6 +14589,44 @@ void intel_create_rotation_property(struct
> drm_device *dev, struct intel_plane *
>                               plane->base.state->rotation);
>  }
> 
> +void intel_create_render_comp_property(struct drm_device *dev,
> +             struct intel_plane *plane)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct drm_plane *drm_plane = &plane->base;
> +     unsigned long flags = BIT(COMP_UNCOMPRESSED);
> +
> +     static const struct drm_prop_enum_list rc_status[] = {
> +             { COMP_UNCOMPRESSED,   "Uncompressed/not capable" },
> +             { COMP_RENDER,  "Only render decompression" },
> +     };
> +
> +     if (plane->pipe != PIPE_C ||
> +                     (drm_plane->type == DRM_PLANE_TYPE_PRIMARY ||
> +                      (drm_plane->type == DRM_PLANE_TYPE_OVERLAY
> &&
> +                       plane->plane == PLANE_A))) {
> +             flags |= BIT(COMP_RENDER);
> +     }
> +
> +     if (!dev_priv->render_comp_property) {
> +             dev_priv->render_comp_property =
> +                     drm_property_create_bitmask(dev, 0,
> +                                     "render compression",
> +                                     rc_status, ARRAY_SIZE(rc_status),
> +                                     flags);
> +             if (!dev_priv->render_comp_property) {
> +                     DRM_ERROR("RC: Failed to create property\n");
> +                     return;
> +             }
> +     }
> +
> +     if (dev_priv->render_comp_property) {
> +             drm_object_attach_property(&plane->base.base,
> +                             dev_priv->render_comp_property, 0);
> +     }
> +     dev->mode_config.allow_aux_plane = true; }
> +
>  static int
>  intel_check_cursor_plane(struct drm_plane *plane,
>                        struct intel_crtc_state *crtc_state, diff --git
> a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8901ce5..ff8b804 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -314,6 +314,10 @@ struct intel_atomic_state {
>       bool skip_intermediate_wm;
>  };
> 
> +/* render compression property bits */
> +#define COMP_UNCOMPRESSED          0
> +#define COMP_RENDER                1
> +
>  struct intel_plane_state {
>       struct drm_plane_state base;
>       struct drm_rect src;
> @@ -353,6 +357,9 @@ struct intel_plane_state {
> 
>       /* async flip related structures */
>       struct drm_i915_gem_request *wait_req;
> +
> +     /* Render compression */
> +     unsigned int render_comp_enable;
>  };
> 
>  struct intel_initial_plane_config {
> @@ -1225,6 +1232,9 @@ intel_rotation_90_or_270(unsigned int rotation)
> void intel_create_rotation_property(struct drm_device *dev,
>                                       struct intel_plane *plane);
> 
> +void intel_create_render_comp_property(struct drm_device *dev,
> +             struct intel_plane *plane);
> +
>  void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
>                                   enum pipe pipe);
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 02e8401..658751f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -193,6 +193,7 @@ skl_update_plane(struct drm_plane *drm_plane,
>       u32 surf_addr = plane_state->main.offset;
>       unsigned int rotation = plane_state->base.rotation;
>       u32 stride = skl_plane_stride(fb, 0, rotation);
> +     u32 aux_stride = skl_plane_stride(fb, 1, rotation);
>       int crtc_x = plane_state->dst.x1;
>       int crtc_y = plane_state->dst.y1;
>       uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> @@ -213,6 +214,11 @@ skl_update_plane(struct drm_plane *drm_plane,
> 
>       plane_ctl |= skl_plane_ctl_rotation(rotation);
> 
> +     if (plane_state->render_comp_enable)
> +             plane_ctl |= PLANE_CTL_DECOMPRESSION_ENABLE;
> +     else
> +             plane_ctl &= ~PLANE_CTL_DECOMPRESSION_ENABLE;
> +
>       if (key->flags) {
>               I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
>               I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
> @@ -233,6 +239,10 @@ skl_update_plane(struct drm_plane *drm_plane,
>       I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
>       I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
>       I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
> +     I915_WRITE(PLANE_AUX_DIST(pipe, 0),
> +                     plane_state->aux.offset | aux_stride);
> +     I915_WRITE(PLANE_AUX_OFFSET(pipe, 0),
> +                     plane_state->aux.y <<16 | plane_state->aux.x);
> 
>       /* program plane scaler */
>       if (plane_state->scaler_id >= 0) {
> @@ -1095,6 +1105,9 @@ intel_plane_init(struct drm_device *dev, enum
> pipe pipe, int plane)
> 
>       intel_create_rotation_property(dev, intel_plane);
> 
> +     if (INTEL_INFO(dev)->gen >= 9)
> +             intel_create_render_comp_property(dev, intel_plane);
> +
>       drm_plane_helper_add(&intel_plane->base,
> &intel_plane_helper_funcs);
> 
>       return 0;
> --
> 1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to