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 <mayuresh.s.gharpure@xxxxxxxxx> > --- > 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 > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx