[PATCH] drm/i915: Y/Yf Tiling Workaround for BXT/SKL

[PATCH] drm/i915: Y/Yf Tiling Workaround for BXT/SKL

[Mayuresh Gharpure]

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@intel.com>
---
 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;
+
+	/*
 	 * 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Y/Yf Tiling Workaround for BXT/SKL

[Patchwork]

== Summary ==

drivers/gpu/drm/i915/intel_display.c: In function 'skl_handle_ytile_wa':
drivers/gpu/drm/i915/intel_display.c:16352:50: error: 'CHICKEN_DCPR_1' undeclared (first use in this function)
    I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1)
                                                  ^
drivers/gpu/drm/i915/intel_display.c:16352:50: note: each undeclared identifier is reported only once for each function it appears in
drivers/gpu/drm/i915/intel_display.c:16352:139: error: 'IDLE_WAKEMEM_MASK' undeclared (first use in this function)
    I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1)
                                                                                                                                           ^
  CC [M]  drivers/gpu/drm/i915/intel_dp_mst.o
scripts/Makefile.build:258: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
scripts/Makefile.build:403: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:403: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:403: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:943: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

✗ Fi.CI.BAT: failure for drm/i915: Y/Yf Tiling Workaround for BXT/SKL

[Patchwork]

== Summary ==

  CC [M]  drivers/gpu/drm/i915/dvo_ch7017.o
  CC [M]  drivers/gpu/drm/i915/dvo_ch7xxx.o
  CC [M]  drivers/gpu/drm/i915/dvo_ivch.o
  CC [M]  drivers/gpu/drm/i915/dvo_ns2501.o
  CC [M]  drivers/gpu/drm/i915/dvo_sil164.o
  CC [M]  drivers/gpu/drm/i915/dvo_tfp410.o
  CC [M]  drivers/gpu/drm/i915/intel_ddi.o
  CC [M]  drivers/gpu/drm/i915/intel_crt.o
  CC [M]  drivers/gpu/drm/i915/intel_dp_link_training.o
drivers/gpu/drm/i915/intel_display.c: In function 'skl_handle_ytile_wa':
drivers/gpu/drm/i915/intel_display.c:16352:50: error: 'CHICKEN_DCPR_1' undeclared (first use in this function)
    I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1)
                                                  ^
drivers/gpu/drm/i915/intel_display.c:16352:50: note: each undeclared identifier is reported only once for each function it appears in
drivers/gpu/drm/i915/intel_display.c:16352:139: error: 'IDLE_WAKEMEM_MASK' undeclared (first use in this function)
    I915_WRITE(CHICKEN_DCPR_1, I915_READ(CHICKEN_DCPR_1)
                                                                                                                                           ^
  CC [M]  drivers/gpu/drm/i915/intel_dp_mst.o
scripts/Makefile.build:258: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
scripts/Makefile.build:403: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:403: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:403: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:943: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH] drm/i915: Y/Yf Tiling Workaround for BXT/SKL

[Daniel Vetter]

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@intel.com>
> ---
>  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx