On Fri, Jan 22, 2016 at 03:29:08PM +0530, Mayuresh Gharpure wrote:
> BXT/SKL Bspec specifies a workaround of setting a bit of CHICKEN_DCPR_1
> register when one of the plane is enabled with a Y or Yf tiled buffer.
> This patch implements this workaround
>
> Signed-off-by: Mayuresh Gharpure <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++++++
> drivers/gpu/drm/i915/intel_display.c | 42
> ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> drivers/gpu/drm/i915/intel_sprite.c | 5 +++++
> 4 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 204661f..a854690 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1958,6 +1958,12 @@ struct drm_i915_private {
> struct intel_encoder *dig_port_map[I915_MAX_PORTS];
>
> /*
> + * Mask to keep track of planes enabled with y/yf tiled buffers
> + * This is required because of a bspec workaround in SKL/BXT
> + */
> + uint32_t tile_y_mask;
Global state is a horror show for atomic (due to locking and all that) and
hence verboten. I think the correct way would be to compute this in the
crtc atomic_check functions and store in in crtc_state. Then use that to
set it in pre_commit functions when we'll need it, and clear it in
post-commit when it's no longer needed.
If you want to check your logic (which I highly recommend for these kind
of state-driven w/a) check that the w/a bit is set when enabling a Y or Yf
tiled buffer, and check that it's clear when everything is off (when we
e.g. drop the main display power well).
Also please extend the commit message a bit on when exactly this is needed
(is it harmful to enable the chicken bit when no Y/Yf plane is in use,
what happens if we don't?). Without this I can't really review the logic
you have here in detail.
-Daniel
> +
> + /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> */
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8104511..f78fe65 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3092,6 +3092,7 @@ 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 int plane_index = drm_plane_index(plane);
>
> plane_ctl = PLANE_CTL_ENABLE |
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -3128,6 +3129,8 @@ static void skylake_update_primary_plane(struct
> drm_plane *plane,
> intel_crtc->adjusted_x = x_offset;
> intel_crtc->adjusted_y = y_offset;
>
> + skl_handle_ytile_wa(dev_priv, fb->modifier[0], plane_index, true);
> +
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
> I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> @@ -3159,10 +3162,12 @@ static void skylake_disable_primary_plane(struct
> drm_plane *primary,
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> int pipe = to_intel_crtc(crtc)->pipe;
> + unsigned int plane_index = drm_plane_index(primary);
>
> if (dev_priv->fbc.deactivate)
> dev_priv->fbc.deactivate(dev_priv);
>
> + skl_handle_ytile_wa(dev_priv, 0, plane_index, false);
> I915_WRITE(PLANE_CTL(pipe, 0), 0);
> I915_WRITE(PLANE_SURF(pipe, 0), 0);
> POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -16331,3 +16336,40 @@ void intel_modeset_preclose(struct drm_device *dev,
> struct drm_file *file)
> spin_unlock_irq(&dev->event_lock);
> }
> }
> +
> +void skl_handle_ytile_wa(struct drm_i915_private *dev_priv,
> + uint64_t tiling,
> + unsigned int plane_index,
> + bool enable_flag)
> +{
> + /*
> + * When a plane is being disabled, if none of the other planes
> + * is enabled with Y/Yf tiled buffers,reset bit 13 in CHICKEN_DCPR_1
> + */
> + if (!enable_flag) {
> + dev_priv->tile_y_mask &= ~(1<<plane_index);
> + if (dev_priv->tile_y_mask == 0)
> + I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1)
> + & ~IDLE_WAKEMEM_MASK);
> + } else {
> + /*
> + * When a plane is being enabled with Y/Yf tiled buffer,set the
> + * bit 13 in CHICKEN_DCPR_1, if plane is being enabled with a
> + * linear or x-tiled buffer, reset the bit if no other plane is
> + * enabled with Y/Yf tiled buffer
> + */
> + if (tiling == I915_FORMAT_MOD_Y_TILED ||
> + tiling == I915_FORMAT_MOD_Yf_TILED) {
> + dev_priv->tile_y_mask |= (1<<plane_index);
> + I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1) |
> + IDLE_WAKEMEM_MASK);
> + } else{
> + dev_priv->tile_y_mask &= ~(1<<plane_index);
> + if (dev_priv->tile_y_mask == 0)
> + I915_WRITE(CHICKEN_DCPR_1,
> + I915_READ(CHICKEN_DCPR_1) &
> + ~IDLE_WAKEMEM_MASK);
> + }
> + }
> +}
> +
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index bc97012..a8a4da5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1609,6 +1609,10 @@ struct intel_plane_state
> *intel_create_plane_state(struct drm_plane *plane);
> struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane);
> void intel_plane_destroy_state(struct drm_plane *plane,
> struct drm_plane_state *state);
> +void skl_handle_ytile_wa(struct drm_i915_private *dev_priv,
> + uint64_t tiling,
> + unsigned int plane_index,
> + bool enable_flag);
> extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 0875c8e..7f03dcf 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -205,6 +205,7 @@ 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 int plane_index = drm_plane_index(drm_plane);
>
> plane_ctl = PLANE_CTL_ENABLE |
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -255,6 +256,8 @@ skl_update_plane(struct drm_plane *drm_plane,
> }
> plane_offset = y_offset << 16 | x_offset;
>
> + skl_handle_ytile_wa(dev_priv, fb->modifier[0], plane_index, true);
> +
> I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
> I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
> @@ -292,6 +295,8 @@ skl_disable_plane(struct drm_plane *dplane, struct
> drm_crtc *crtc)
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
>
> + skl_handle_ytile_wa(dev_priv, 0, drm_plane_index(dplane), false);
> +
> I915_WRITE(PLANE_CTL(pipe, plane), 0);
>
> I915_WRITE(PLANE_SURF(pipe, plane), 0);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
