[Intel-gfx] [PATCH v2 2/2] drm/i915: Render decompression support for Gen9 and above
Kannan, Vandana
vandana.kannan at intel.com
Mon May 9 06:02:46 UTC 2016
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Friday, April 29, 2016 8:15 PM
> To: Kannan, Vandana <vandana.kannan at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Smith, Gary K
> <gary.k.smith at intel.com>
> 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 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.
> >
> > 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 <vandana.kannan at intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Smith, Gary K <gary.k.smith at 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 | 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 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 bfcc2eb..9b2ff21 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -117,6 +117,9 @@ 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 drm_device *dev,
> > + struct intel_plane_state *plane_state,
> > + enum pipe pipe, int x, int y);
> >
> > typedef struct {
> > int min, max;
> > @@ -3045,7 +3048,7 @@ static void
> skylake_update_primary_plane(struct drm_plane *plane,
> > int pipe = intel_crtc->pipe;
> > u32 plane_ctl, stride_div, stride;
> > u32 tile_height, plane_offset, plane_size;
> > - unsigned int rotation = plane_state->base.rotation;
> > + unsigned int rotation = plane_state->base.rotation, render_comp;
> > int x_offset, y_offset;
> > u32 surf_addr;
> > int scaler_id = plane_state->scaler_id; @@ -3057,6 +3060,9 @@
> static
> > void skylake_update_primary_plane(struct drm_plane *plane,
> > int dst_y = plane_state->dst.y1;
> > int dst_w = drm_rect_width(&plane_state->dst);
> > int dst_h = drm_rect_height(&plane_state->dst);
> > + unsigned long aux_dist = 0;
> > + u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > + u32 tile_row_adjustment = 0, height_in_mem = 0;
> >
> > plane_ctl = PLANE_CTL_ENABLE |
> > PLANE_CTL_PIPE_GAMMA_ENABLE |
> > @@ -3093,10 +3099,28 @@ static void
> skylake_update_primary_plane(struct drm_plane *plane,
> > intel_crtc->adjusted_x = x_offset;
> > intel_crtc->adjusted_y = y_offset;
> >
> > + render_comp = plane_state->render_comp_enable;
> > + if (render_comp) {
> > + int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> > +
> > + tile_height = intel_tile_height(dev_priv, fb->modifier[0],
> cpp);
> > +
> > + height_in_mem = (fb->offsets[1]/fb->pitches[0]);
> > + tile_row_adjustment = height_in_mem % tile_height;
> > + aux_dist = (fb->pitches[0] *
> > + (height_in_mem - tile_row_adjustment));
> > + aux_stride = skl_plane_stride(fb, 1, rotation);
>
> This stuff shouldn't be here if my stuff is used as a base. Like for nv12, it
> should all be computed upfront.
>
> I should probably go read up on render compression so I'd actually know
> what the AUX offsets actually mean for it.
>
[Vandana] you mean skl_check_nv12_aux_surface() ?
The dependency this patch has on the fb_offset series is in terms of skl_plane_stride() and related definitions.
I will go through the nv12 implementation in the fb_offset series and get back.
> > + 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), plane_offset);
> > I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> > I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> > + I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
> > + I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset<<16 |
> > +aux_x_offset);
> >
> > if (scaler_id >= 0) {
> > uint32_t ps_ctrl = 0;
> > @@ -11856,6 +11880,17 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
> > return ret;
> > }
> >
> > + if (fb && to_intel_plane_state(plane_state)->
> > + render_comp_enable) {
> > + ret = skl_check_compression(dev,
> > + to_intel_plane_state(plane_state),
> > + intel_crtc->pipe, crtc->x, crtc->y);
> > + if (ret) {
> > + DRM_DEBUG_KMS("Render compr checks failed\n");
> > + return ret;
> > + }
> > + }
> > +
> > was_visible = old_plane_state->visible;
> > visible = to_intel_plane_state(plane_state)->visible;
> >
> > @@ -13972,6 +14007,63 @@ skl_max_scale(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state
> > return max_scale;
> > }
> >
> > +static int skl_check_compression(struct drm_device *dev,
> > + struct intel_plane_state *plane_state,
> > + enum pipe pipe, int x, int y)
> > +{
> > + struct drm_framebuffer *fb = plane_state->base.fb;
> > + int x_offset;
> > + int src_w = drm_rect_width(&plane_state->src) >> 16;
> > +
> > + if (!IS_SKYLAKE(dev) && !IS_BROXTON(dev)) {
> > + DRM_DEBUG_KMS("RC support on CNL+ needs to be
> revisited\n");
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * 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.
> > + */
> > + x_offset = x;
> > +
> > + 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, @@ -14126,6
> +14218,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;
> > @@ -14155,6 +14250,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 cf27989..38006e7 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -303,6 +303,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;
> > @@ -334,6 +338,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 {
> > @@ -1196,6 +1203,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 0f3e230..2593bcf 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -193,7 +193,7 @@ skl_update_plane(struct drm_plane *drm_plane,
> > const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> > u32 surf_addr;
> > u32 tile_height, plane_offset, plane_size;
> > - unsigned int rotation = plane_state->base.rotation;
> > + unsigned int rotation = plane_state->base.rotation, render_comp;
> > int x_offset, y_offset;
> > int crtc_x = plane_state->dst.x1;
> > int crtc_y = plane_state->dst.y1;
> > @@ -205,6 +205,9 @@ skl_update_plane(struct drm_plane *drm_plane,
> > uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
> > const struct intel_scaler *scaler =
> > &crtc_state->scaler_state.scalers[plane_state->scaler_id];
> > + unsigned long aux_dist = 0;
> > + u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > + u32 tile_row_adjustment = 0, height_in_mem = 0;
> >
> > plane_ctl = PLANE_CTL_ENABLE |
> > PLANE_CTL_PIPE_GAMMA_ENABLE |
> > @@ -254,9 +257,28 @@ skl_update_plane(struct drm_plane *drm_plane,
> > }
> > plane_offset = y_offset << 16 | x_offset;
> >
> > + render_comp = plane_state->render_comp_enable;
> > + if (render_comp) {
> > + int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> > +
> > + tile_height = intel_tile_height(dev_priv, fb->modifier[0],
> cpp);
> > +
> > + height_in_mem = (fb->offsets[1]/fb->pitches[0]);
> > + tile_row_adjustment = height_in_mem % tile_height;
> > + aux_dist = (fb->pitches[0] *
> > + (height_in_mem - tile_row_adjustment));
> > + aux_stride = skl_plane_stride(fb, 1, rotation);
> > + plane_ctl |= PLANE_CTL_DECOMPRESSION_ENABLE;
> > + } else {
> > + plane_ctl &= ~PLANE_CTL_DECOMPRESSION_ENABLE;
> > + }
> > +
> > I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
> > I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> > I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
> > + I915_WRITE(PLANE_AUX_DIST(pipe, plane), aux_dist | aux_stride);
> > + I915_WRITE(PLANE_AUX_OFFSET(pipe, plane),
> > + aux_y_offset<<16 | aux_x_offset);
> >
> > /* program plane scaler */
> > if (plane_state->scaler_id >= 0) {
> > @@ -1120,6 +1142,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
>
> --
> Ville Syrjälä
> Intel OTC
More information about the Intel-gfx
mailing list