> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > Sent: Friday, April 29, 2016 8:15 PM > To: Kannan, Vandana <vandana.kannan@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Smith, Gary K > <gary.k.smith@xxxxxxxxx> > 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@xxxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Cc: Smith, Gary K <gary.k.smith@xxxxxxxxx> > > --- > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
