[PATCH 0/7] Explicitly pass crtc_state and plane_state to plane update functions.

[PATCH 0/7] Explicitly pass crtc_state and plane_state to plane update functions.

[Maarten Lankhorst]

Right now there are 3 possible functions, update_plane that's only used by sprites,
disable_plane which may be called during plane disable, and commit_plane which
may call either.

This is a bit messy and unclear. Make update_plane only be called when
the plane is visible, and disable_plane whenever it's not. commit_plane
will be removed to reduce confusion.

This makes it easier to put the vma in plane_state.

Maarten Lankhorst (7):
  drm/i915: Use passed plane state for sprite planes, v4.
  drm/i915: Do not use commit_plane for sprite planes.
  drm/i915: Remove some visibility checks from intel_crtc_update_cursor.
  drm/i915: Make disable_cursor_plane similar to commit_cursor_plane.
  drm/i915: Use the plane state for cursor updates.
  drm/i915: Use plane state for primary plane updates.
  drm/i915: Remove commit_plane function pointer.

 drivers/gpu/drm/i915/i915_drv.h           |   3 -
 drivers/gpu/drm/i915/intel_atomic_plane.c |  10 +-
 drivers/gpu/drm/i915/intel_display.c      | 355 +++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h          |  12 +-
 drivers/gpu/drm/i915/intel_sprite.c       | 129 +++++------
 5 files changed, 224 insertions(+), 285 deletions(-)

-- 
2.1.0

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

[PATCH 1/7] drm/i915: Use passed plane state for sprite planes, v4.

[Maarten Lankhorst]

Don't use plane->state directly, use the pointer from commit_plane.

Changes since v1:
- Fix uses of plane->state->rotation and color key to use the passed state too.
- Only pass crtc_state and plane_state to update_plane.
Changes since v2:
- Rebased.
Changes since v3:
- Small whitespace changes and only assign 1 variable per line.
- Constify plane_state and crtc_state. (vsyrjala)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  10 +--
 drivers/gpu/drm/i915/intel_sprite.c | 124 ++++++++++++++++++++----------------
 2 files changed, 71 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a3b2025da563..068050884353 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -658,16 +658,12 @@ struct intel_plane {
 	/*
 	 * NOTE: Do not place new plane state fields here (e.g., when adding
 	 * new plane properties).  New runtime state should now be placed in
-	 * the intel_plane_state structure and accessed via drm_plane->state.
+	 * the intel_plane_state structure and accessed via plane_state.
 	 */
 
 	void (*update_plane)(struct drm_plane *plane,
-			     struct drm_crtc *crtc,
-			     struct drm_framebuffer *fb,
-			     int crtc_x, int crtc_y,
-			     unsigned int crtc_w, unsigned int crtc_h,
-			     uint32_t x, uint32_t y,
-			     uint32_t src_w, uint32_t src_h);
+			     const struct intel_crtc_state *crtc_state,
+			     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct drm_plane *plane,
 			      struct drm_crtc *crtc);
 	int (*check_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4ff7a1f4183e..9f64289333e8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -178,28 +178,33 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
 }
 
 static void
-skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+skl_update_plane(struct drm_plane *drm_plane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = drm_plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
 	u32 plane_ctl, stride_div, stride;
-	const struct drm_intel_sprite_colorkey *key =
-		&to_intel_plane_state(drm_plane->state)->ckey;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr;
 	u32 tile_height, plane_offset, plane_size;
 	unsigned int rotation;
 	int x_offset, y_offset;
-	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
-	int scaler_id;
+	int crtc_x = plane_state->dst.x1;
+	int crtc_y = plane_state->dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
+	uint32_t x = plane_state->src.x1 >> 16;
+	uint32_t y = plane_state->src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
+	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];
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		PLANE_CTL_PIPE_GAMMA_ENABLE |
@@ -208,14 +213,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
 
-	rotation = drm_plane->state->rotation;
+	rotation = plane_state->base.rotation;
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
 
-	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -256,13 +259,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
 
 	/* program plane scaler */
-	if (scaler_id >= 0) {
+	if (plane_state->scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
+		int scaler_id = plane_state->scaler_id;
 
 		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
 			PS_PLANE_SEL(plane));
-		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
-			crtc_state->scaler_state.scalers[scaler_id].mode;
+		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode;
 		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
 		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
 		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
@@ -334,24 +337,29 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
 }
 
 static void
-vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+vlv_update_plane(struct drm_plane *dplane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int pipe = intel_plane->pipe;
 	int plane = intel_plane->plane;
 	u32 sprctl;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
-	const struct drm_intel_sprite_colorkey *key =
-		&to_intel_plane_state(dplane->state)->ckey;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->dst.x1;
+	int crtc_y = plane_state->dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
+	uint32_t x = plane_state->src.x1 >> 16;
+	uint32_t y = plane_state->src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
 
 	sprctl = SP_ENABLE;
 
@@ -421,7 +429,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 							fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
-	if (dplane->state->rotation == BIT(DRM_ROTATE_180)) {
+	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
 		sprctl |= SP_ROTATE_180;
 
 		x += src_w;
@@ -474,23 +482,28 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 }
 
 static void
-ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+ivb_update_plane(struct drm_plane *plane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	enum pipe pipe = intel_plane->pipe;
 	u32 sprctl, sprscale = 0;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
-	const struct drm_intel_sprite_colorkey *key =
-		&to_intel_plane_state(plane->state)->ckey;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->dst.x1;
+	int crtc_y = plane_state->dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
+	uint32_t x = plane_state->src.x1 >> 16;
+	uint32_t y = plane_state->src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
 
 	sprctl = SPRITE_ENABLE;
 
@@ -550,7 +563,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
-	if (plane->state->rotation == BIT(DRM_ROTATE_180)) {
+	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
 		sprctl |= SPRITE_ROTATE_180;
 
 		/* HSW and BDW does this automagically in hardware */
@@ -612,23 +625,28 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 }
 
 static void
-ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t x, uint32_t y,
-		 uint32_t src_w, uint32_t src_h)
+ilk_update_plane(struct drm_plane *plane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int pipe = intel_plane->pipe;
 	unsigned long dvssurf_offset, linear_offset;
 	u32 dvscntr, dvsscale;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
-	const struct drm_intel_sprite_colorkey *key =
-		&to_intel_plane_state(plane->state)->ckey;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->dst.x1;
+	int crtc_y = plane_state->dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
+	uint32_t x = plane_state->src.x1 >> 16;
+	uint32_t y = plane_state->src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
 
 	dvscntr = DVS_ENABLE;
 
@@ -684,7 +702,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= dvssurf_offset;
 
-	if (plane->state->rotation == BIT(DRM_ROTATE_180)) {
+	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
 		dvscntr |= DVS_ROTATE_180;
 
 		x += src_w;
@@ -917,23 +935,17 @@ static void
 intel_commit_sprite_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
-	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct drm_framebuffer *fb = state->base.fb;
-
-	crtc = crtc ? crtc : plane->crtc;
 
 	if (state->visible) {
-		intel_plane->update_plane(plane, crtc, fb,
-					  state->dst.x1, state->dst.y1,
-					  drm_rect_width(&state->dst),
-					  drm_rect_height(&state->dst),
-					  state->src.x1 >> 16,
-					  state->src.y1 >> 16,
-					  drm_rect_width(&state->src) >> 16,
-					  drm_rect_height(&state->src) >> 16);
+		struct intel_crtc_state *crtc_state =
+			to_intel_crtc(state->base.crtc)->config;
+
+		intel_plane->update_plane(plane, crtc_state, state);
 	} else {
-		intel_plane->disable_plane(plane, crtc);
+		struct drm_crtc *crtc = state->base.crtc;
+
+		intel_plane->disable_plane(plane, crtc ?: plane->crtc);
 	}
 }
 
-- 
2.1.0

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

Re: [PATCH 1/7] drm/i915: Use passed plane state for sprite planes, v4.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:06AM +0100, Maarten Lankhorst wrote:
> Don't use plane->state directly, use the pointer from commit_plane.
> 
> Changes since v1:
> - Fix uses of plane->state->rotation and color key to use the passed state too.
> - Only pass crtc_state and plane_state to update_plane.
> Changes since v2:
> - Rebased.
> Changes since v3:
> - Small whitespace changes and only assign 1 variable per line.
> - Constify plane_state and crtc_state. (vsyrjala)
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  10 +--
>  drivers/gpu/drm/i915/intel_sprite.c | 124 ++++++++++++++++++++----------------
>  2 files changed, 71 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a3b2025da563..068050884353 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -658,16 +658,12 @@ struct intel_plane {
>  	/*
>  	 * NOTE: Do not place new plane state fields here (e.g., when adding
>  	 * new plane properties).  New runtime state should now be placed in
> -	 * the intel_plane_state structure and accessed via drm_plane->state.
> +	 * the intel_plane_state structure and accessed via plane_state.
>  	 */
>  
>  	void (*update_plane)(struct drm_plane *plane,
> -			     struct drm_crtc *crtc,
> -			     struct drm_framebuffer *fb,
> -			     int crtc_x, int crtc_y,
> -			     unsigned int crtc_w, unsigned int crtc_h,
> -			     uint32_t x, uint32_t y,
> -			     uint32_t src_w, uint32_t src_h);
> +			     const struct intel_crtc_state *crtc_state,
> +			     const struct intel_plane_state *plane_state);
>  	void (*disable_plane)(struct drm_plane *plane,
>  			      struct drm_crtc *crtc);
>  	int (*check_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4ff7a1f4183e..9f64289333e8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -178,28 +178,33 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
>  }
>  
>  static void
> -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> -		 int crtc_x, int crtc_y,
> -		 unsigned int crtc_w, unsigned int crtc_h,
> -		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +skl_update_plane(struct drm_plane *drm_plane,
> +		 const struct intel_crtc_state *crtc_state,
> +		 const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = drm_plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	const int pipe = intel_plane->pipe;
>  	const int plane = intel_plane->plane + 1;
>  	u32 plane_ctl, stride_div, stride;
> -	const struct drm_intel_sprite_colorkey *key =
> -		&to_intel_plane_state(drm_plane->state)->ckey;
> +	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
>  	u32 surf_addr;
>  	u32 tile_height, plane_offset, plane_size;
>  	unsigned int rotation;
>  	int x_offset, y_offset;
> -	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
> -	int scaler_id;
> +	int crtc_x = plane_state->dst.x1;
> +	int crtc_y = plane_state->dst.y1;
> +	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> +	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> +	uint32_t x = plane_state->src.x1 >> 16;
> +	uint32_t y = plane_state->src.y1 >> 16;
> +	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
> +	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];
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -208,14 +213,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
>  	plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
>  
> -	rotation = drm_plane->state->rotation;
> +	rotation = plane_state->base.rotation;
>  	plane_ctl |= skl_plane_ctl_rotation(rotation);
>  
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
>  
> -	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
> -
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -256,13 +259,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
>  
>  	/* program plane scaler */
> -	if (scaler_id >= 0) {
> +	if (plane_state->scaler_id >= 0) {
>  		uint32_t ps_ctrl = 0;
> +		int scaler_id = plane_state->scaler_id;
>  
>  		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
>  			PS_PLANE_SEL(plane));
> -		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
> -			crtc_state->scaler_state.scalers[scaler_id].mode;
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode;
>  		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
>  		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
> @@ -334,24 +337,29 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
>  }
>  
>  static void
> -vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> -		 int crtc_x, int crtc_y,
> -		 unsigned int crtc_w, unsigned int crtc_h,
> -		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +vlv_update_plane(struct drm_plane *dplane,
> +		 const struct intel_crtc_state *crtc_state,
> +		 const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = dplane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(dplane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int pipe = intel_plane->pipe;
>  	int plane = intel_plane->plane;
>  	u32 sprctl;
>  	unsigned long sprsurf_offset, linear_offset;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	const struct drm_intel_sprite_colorkey *key =
> -		&to_intel_plane_state(dplane->state)->ckey;
> +	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> +	int crtc_x = plane_state->dst.x1;
> +	int crtc_y = plane_state->dst.y1;
> +	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> +	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> +	uint32_t x = plane_state->src.x1 >> 16;
> +	uint32_t y = plane_state->src.y1 >> 16;
> +	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
> +	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
>  
>  	sprctl = SP_ENABLE;
>  
> @@ -421,7 +429,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  							fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
>  
> -	if (dplane->state->rotation == BIT(DRM_ROTATE_180)) {
> +	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
>  		sprctl |= SP_ROTATE_180;
>  
>  		x += src_w;
> @@ -474,23 +482,28 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>  }
>  
>  static void
> -ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> -		 int crtc_x, int crtc_y,
> -		 unsigned int crtc_w, unsigned int crtc_h,
> -		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +ivb_update_plane(struct drm_plane *plane,
> +		 const struct intel_crtc_state *crtc_state,
> +		 const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	enum pipe pipe = intel_plane->pipe;
>  	u32 sprctl, sprscale = 0;
>  	unsigned long sprsurf_offset, linear_offset;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	const struct drm_intel_sprite_colorkey *key =
> -		&to_intel_plane_state(plane->state)->ckey;
> +	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> +	int crtc_x = plane_state->dst.x1;
> +	int crtc_y = plane_state->dst.y1;
> +	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> +	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> +	uint32_t x = plane_state->src.x1 >> 16;
> +	uint32_t y = plane_state->src.y1 >> 16;
> +	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
> +	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
>  
>  	sprctl = SPRITE_ENABLE;
>  
> @@ -550,7 +563,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
>  
> -	if (plane->state->rotation == BIT(DRM_ROTATE_180)) {
> +	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
>  		sprctl |= SPRITE_ROTATE_180;
>  
>  		/* HSW and BDW does this automagically in hardware */
> @@ -612,23 +625,28 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>  }
>  
>  static void
> -ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> -		 int crtc_x, int crtc_y,
> -		 unsigned int crtc_w, unsigned int crtc_h,
> -		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +ilk_update_plane(struct drm_plane *plane,
> +		 const struct intel_crtc_state *crtc_state,
> +		 const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int pipe = intel_plane->pipe;
>  	unsigned long dvssurf_offset, linear_offset;
>  	u32 dvscntr, dvsscale;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	const struct drm_intel_sprite_colorkey *key =
> -		&to_intel_plane_state(plane->state)->ckey;
> +	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> +	int crtc_x = plane_state->dst.x1;
> +	int crtc_y = plane_state->dst.y1;
> +	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> +	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> +	uint32_t x = plane_state->src.x1 >> 16;
> +	uint32_t y = plane_state->src.y1 >> 16;
> +	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
> +	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
>  
>  	dvscntr = DVS_ENABLE;
>  
> @@ -684,7 +702,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= dvssurf_offset;
>  
> -	if (plane->state->rotation == BIT(DRM_ROTATE_180)) {
> +	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
>  		dvscntr |= DVS_ROTATE_180;
>  
>  		x += src_w;
> @@ -917,23 +935,17 @@ static void
>  intel_commit_sprite_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
>  {
> -	struct drm_crtc *crtc = state->base.crtc;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> -	struct drm_framebuffer *fb = state->base.fb;
> -
> -	crtc = crtc ? crtc : plane->crtc;
>  
>  	if (state->visible) {
> -		intel_plane->update_plane(plane, crtc, fb,
> -					  state->dst.x1, state->dst.y1,
> -					  drm_rect_width(&state->dst),
> -					  drm_rect_height(&state->dst),
> -					  state->src.x1 >> 16,
> -					  state->src.y1 >> 16,
> -					  drm_rect_width(&state->src) >> 16,
> -					  drm_rect_height(&state->src) >> 16);
> +		struct intel_crtc_state *crtc_state =
> +			to_intel_crtc(state->base.crtc)->config;
> +
> +		intel_plane->update_plane(plane, crtc_state, state);
>  	} else {
> -		intel_plane->disable_plane(plane, crtc);
> +		struct drm_crtc *crtc = state->base.crtc;
> +
> +		intel_plane->disable_plane(plane, crtc ?: plane->crtc);
>  	}
>  }
>  
> -- 
> 2.1.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/7] drm/i915: Do not use commit_plane for sprite planes.

[Maarten Lankhorst]

Use update_plane and disable_plane directly.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 12 +++++++++++-
 drivers/gpu/drm/i915/intel_sprite.c       | 19 -------------------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 856c3118bb87..969aa410deaa 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -194,8 +194,18 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state =
 		to_intel_plane_state(plane->state);
+	struct drm_crtc *crtc = plane->state->crtc ?: old_state->crtc;
+	struct drm_crtc_state *crtc_state =
+		drm_atomic_get_existing_crtc_state(old_state->state, crtc);
 
-	intel_plane->commit_plane(plane, intel_state);
+	if (intel_plane->commit_plane)
+		intel_plane->commit_plane(plane, intel_state);
+	else if (intel_state->visible)
+		intel_plane->update_plane(plane,
+					  to_intel_crtc_state(crtc_state),
+					  intel_state);
+	else
+		intel_plane->disable_plane(plane, crtc);
 }
 
 const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9f64289333e8..4d448b990c50 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -931,24 +931,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	return 0;
 }
 
-static void
-intel_commit_sprite_plane(struct drm_plane *plane,
-			  struct intel_plane_state *state)
-{
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-
-	if (state->visible) {
-		struct intel_crtc_state *crtc_state =
-			to_intel_crtc(state->base.crtc)->config;
-
-		intel_plane->update_plane(plane, crtc_state, state);
-	} else {
-		struct drm_crtc *crtc = state->base.crtc;
-
-		intel_plane->disable_plane(plane, crtc ?: plane->crtc);
-	}
-}
-
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv)
 {
@@ -1130,7 +1112,6 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	intel_plane->plane = plane;
 	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
 	intel_plane->check_plane = intel_check_sprite_plane;
-	intel_plane->commit_plane = intel_commit_sprite_plane;
 	possible_crtcs = (1 << pipe);
 	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
 				       &intel_plane_funcs,
-- 
2.1.0

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

Re: [PATCH 2/7] drm/i915: Do not use commit_plane for sprite planes.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:07AM +0100, Maarten Lankhorst wrote:
> Use update_plane and disable_plane directly.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 12 +++++++++++-
>  drivers/gpu/drm/i915/intel_sprite.c       | 19 -------------------
>  2 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 856c3118bb87..969aa410deaa 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -194,8 +194,18 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct intel_plane_state *intel_state =
>  		to_intel_plane_state(plane->state);
> +	struct drm_crtc *crtc = plane->state->crtc ?: old_state->crtc;
> +	struct drm_crtc_state *crtc_state =
> +		drm_atomic_get_existing_crtc_state(old_state->state, crtc);
>  
> -	intel_plane->commit_plane(plane, intel_state);
> +	if (intel_plane->commit_plane)
> +		intel_plane->commit_plane(plane, intel_state);
> +	else if (intel_state->visible)
> +		intel_plane->update_plane(plane,
> +					  to_intel_crtc_state(crtc_state),
> +					  intel_state);
> +	else
> +		intel_plane->disable_plane(plane, crtc);
>  }
>  
>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9f64289333e8..4d448b990c50 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -931,24 +931,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	return 0;
>  }
>  
> -static void
> -intel_commit_sprite_plane(struct drm_plane *plane,
> -			  struct intel_plane_state *state)
> -{
> -	struct intel_plane *intel_plane = to_intel_plane(plane);
> -
> -	if (state->visible) {
> -		struct intel_crtc_state *crtc_state =
> -			to_intel_crtc(state->base.crtc)->config;
> -
> -		intel_plane->update_plane(plane, crtc_state, state);
> -	} else {
> -		struct drm_crtc *crtc = state->base.crtc;
> -
> -		intel_plane->disable_plane(plane, crtc ?: plane->crtc);
> -	}
> -}
> -
>  int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv)
>  {
> @@ -1130,7 +1112,6 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  	intel_plane->plane = plane;
>  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>  	intel_plane->check_plane = intel_check_sprite_plane;
> -	intel_plane->commit_plane = intel_commit_sprite_plane;
>  	possible_crtcs = (1 << pipe);
>  	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
>  				       &intel_plane_funcs,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915: Remove some visibility checks from intel_crtc_update_cursor.

[Maarten Lankhorst]

This is duplicated with intel_check_cursor_plane, and with all
non-atomic paths removed this should be dead code.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 63286f16a9e6..35b881d156b0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -85,8 +85,6 @@ static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
 
-static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
-
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				struct intel_crtc_state *pipe_config);
 static void ironlake_pch_clock_get(struct intel_crtc *crtc,
@@ -10201,25 +10199,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 
 	base = intel_crtc->cursor_addr;
 
-	if (x >= intel_crtc->config->pipe_src_w)
-		on = false;
-
-	if (y >= intel_crtc->config->pipe_src_h)
-		on = false;
-
 	if (x < 0) {
-		if (x + cursor_state->crtc_w <= 0)
-			on = false;
-
 		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
 		x = -x;
 	}
 	pos |= x << CURSOR_X_SHIFT;
 
 	if (y < 0) {
-		if (y + cursor_state->crtc_h <= 0)
-			on = false;
-
 		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
 		y = -y;
 	}
-- 
2.1.0

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

Re: [PATCH 3/7] drm/i915: Remove some visibility checks from intel_crtc_update_cursor.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:08AM +0100, Maarten Lankhorst wrote:
> This is duplicated with intel_check_cursor_plane, and with all
> non-atomic paths removed this should be dead code.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

I have some cursor stuff in a branch somewhere that does some of the
same, but I should be able to rebase that stuff rather easily.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 63286f16a9e6..35b881d156b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -85,8 +85,6 @@ static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
>  };
>  
> -static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
> -
>  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  				struct intel_crtc_state *pipe_config);
>  static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> @@ -10201,25 +10199,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>  
>  	base = intel_crtc->cursor_addr;
>  
> -	if (x >= intel_crtc->config->pipe_src_w)
> -		on = false;
> -
> -	if (y >= intel_crtc->config->pipe_src_h)
> -		on = false;
> -
>  	if (x < 0) {
> -		if (x + cursor_state->crtc_w <= 0)
> -			on = false;
> -
>  		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
>  		x = -x;
>  	}
>  	pos |= x << CURSOR_X_SHIFT;
>  
>  	if (y < 0) {
> -		if (y + cursor_state->crtc_h <= 0)
> -			on = false;
> -
>  		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
>  		y = -y;
>  	}
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915: Make disable_cursor_plane similar to commit_cursor_plane.

[Maarten Lankhorst]

Update cursor_bo and cursor_addr after being called.
This is required to make commit_cursor_plane take a
crtc_state and a plane_state.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35b881d156b0..7ea49d5e2ce0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14177,6 +14177,9 @@ static void
 intel_disable_cursor_plane(struct drm_plane *plane,
 			   struct drm_crtc *crtc)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	intel_crtc->cursor_addr = 0;
 	intel_crtc_update_cursor(crtc, false);
 }
 
-- 
2.1.0

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

Re: [PATCH 4/7] drm/i915: Make disable_cursor_plane similar to commit_cursor_plane.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:09AM +0100, Maarten Lankhorst wrote:
> Update cursor_bo and cursor_addr after being called.

cursor_bo no longer exists. My cursor stuff sitting in branch also gets
rid of cursor_addr, but until I land that we can go with this.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> This is required to make commit_cursor_plane take a
> crtc_state and a plane_state.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35b881d156b0..7ea49d5e2ce0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14177,6 +14177,9 @@ static void
>  intel_disable_cursor_plane(struct drm_plane *plane,
>  			   struct drm_crtc *crtc)
>  {
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	intel_crtc->cursor_addr = 0;
>  	intel_crtc_update_cursor(crtc, false);
>  }
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: Use the plane state for cursor updates.

[Maarten Lankhorst]

Cursor planes grab the state from plane->state instead of the state
that was passed. The only updates are atomic now, so use the plane_state
that's passed in.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++------------------
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7ea49d5e2ce0..f7c293638df4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10076,16 +10076,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	return true;
 }
 
-static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
+static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
+			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t cntl = 0, size = 0;
 
-	if (on) {
-		unsigned int width = intel_crtc->base.cursor->state->crtc_w;
-		unsigned int height = intel_crtc->base.cursor->state->crtc_h;
+	if (plane_state && plane_state->visible) {
+		unsigned int width = plane_state->base.crtc_w;
+		unsigned int height = plane_state->base.crtc_h;
 		unsigned int stride = roundup_pow_of_two(width) * 4;
 
 		switch (stride) {
@@ -10138,7 +10139,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
 	}
 }
 
-static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
+static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
+			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -10146,9 +10148,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
-	if (on) {
+	if (plane_state && plane_state->visible) {
 		cntl = MCURSOR_GAMMA_ENABLE;
-		switch (intel_crtc->base.cursor->state->crtc_w) {
+		switch (plane_state->base.crtc_w) {
 			case 64:
 				cntl |= CURSOR_MODE_64_ARGB_AX;
 				break;
@@ -10159,17 +10161,17 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
 				cntl |= CURSOR_MODE_256_ARGB_AX;
 				break;
 			default:
-				MISSING_CASE(intel_crtc->base.cursor->state->crtc_w);
+				MISSING_CASE(plane_state->base.crtc_w);
 				return;
 		}
 		cntl |= pipe << 28; /* Connect to correct pipe */
 
 		if (HAS_DDI(dev))
 			cntl |= CURSOR_PIPE_CSC_ENABLE;
-	}
 
-	if (crtc->cursor->state->rotation == BIT(DRM_ROTATE_180))
-		cntl |= CURSOR_ROTATE_180;
+		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
+			cntl |= CURSOR_ROTATE_180;
+	}
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE(CURCNTR(pipe), cntl);
@@ -10186,44 +10188,45 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
 
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc,
-				     bool on)
+				     const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
-	struct drm_plane_state *cursor_state = crtc->cursor->state;
-	int x = cursor_state->crtc_x;
-	int y = cursor_state->crtc_y;
-	u32 base = 0, pos = 0;
+	u32 base = intel_crtc->cursor_addr;
+	u32 pos = 0;
 
-	base = intel_crtc->cursor_addr;
+	if (plane_state) {
+		int x = plane_state->base.crtc_x;
+		int y = plane_state->base.crtc_y;
 
-	if (x < 0) {
-		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
-		x = -x;
-	}
-	pos |= x << CURSOR_X_SHIFT;
+		if (x < 0) {
+			pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
+			x = -x;
+		}
+		pos |= x << CURSOR_X_SHIFT;
 
-	if (y < 0) {
-		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
-		y = -y;
+		if (y < 0) {
+			pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
+			y = -y;
+		}
+		pos |= y << CURSOR_Y_SHIFT;
+
+		/* ILK+ do this automagically */
+		if (HAS_GMCH_DISPLAY(dev) &&
+		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
+			base += (plane_state->base.crtc_h *
+				 plane_state->base.crtc_w - 1) * 4;
+		}
 	}
-	pos |= y << CURSOR_Y_SHIFT;
 
 	I915_WRITE(CURPOS(pipe), pos);
 
-	/* ILK+ do this automagically */
-	if (HAS_GMCH_DISPLAY(dev) &&
-	    crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) {
-		base += (cursor_state->crtc_h *
-			 cursor_state->crtc_w - 1) * 4;
-	}
-
 	if (IS_845G(dev) || IS_I865G(dev))
-		i845_update_cursor(crtc, base, on);
+		i845_update_cursor(crtc, base, plane_state);
 	else
-		i9xx_update_cursor(crtc, base, on);
+		i9xx_update_cursor(crtc, base, plane_state);
 }
 
 static bool cursor_size_ok(struct drm_device *dev,
@@ -14180,22 +14183,20 @@ intel_disable_cursor_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc->cursor_addr = 0;
-	intel_crtc_update_cursor(crtc, false);
+	intel_crtc_update_cursor(crtc, NULL);
 }
 
 static void
-intel_commit_cursor_plane(struct drm_plane *plane,
-			  struct intel_plane_state *state)
+intel_update_cursor_plane(struct drm_plane *plane,
+			  const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *state)
 {
-	struct drm_crtc *crtc = state->base.crtc;
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = plane->dev;
-	struct intel_crtc *intel_crtc;
 	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
 	uint32_t addr;
 
-	crtc = crtc ? crtc : plane->crtc;
-	intel_crtc = to_intel_crtc(crtc);
-
 	if (!obj)
 		addr = 0;
 	else if (!INTEL_INFO(dev)->cursor_needs_physical)
@@ -14204,8 +14205,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 		addr = obj->phys_handle->busaddr;
 
 	intel_crtc->cursor_addr = addr;
-
-	intel_crtc_update_cursor(crtc, state->visible);
+	intel_crtc_update_cursor(crtc, state);
 }
 
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -14231,7 +14231,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 	cursor->plane = pipe;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 	cursor->check_plane = intel_check_cursor_plane;
-	cursor->commit_plane = intel_commit_cursor_plane;
+	cursor->update_plane = intel_update_cursor_plane;
 	cursor->disable_plane = intel_disable_cursor_plane;
 
 	drm_universal_plane_init(dev, &cursor->base, 0,
-- 
2.1.0

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

Re: [PATCH 5/7] drm/i915: Use the plane state for cursor updates.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:10AM +0100, Maarten Lankhorst wrote:
> Cursor planes grab the state from plane->state instead of the state
> that was passed. The only updates are atomic now, so use the plane_state
> that's passed in.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Again clashes a bit with my pending stuff. But like said, I can
rebase.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++------------------
>  1 file changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7ea49d5e2ce0..f7c293638df4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10076,16 +10076,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	return true;
>  }
>  
> -static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
> +static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> +			       const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint32_t cntl = 0, size = 0;
>  
> -	if (on) {
> -		unsigned int width = intel_crtc->base.cursor->state->crtc_w;
> -		unsigned int height = intel_crtc->base.cursor->state->crtc_h;
> +	if (plane_state && plane_state->visible) {
> +		unsigned int width = plane_state->base.crtc_w;
> +		unsigned int height = plane_state->base.crtc_h;
>  		unsigned int stride = roundup_pow_of_two(width) * 4;
>  
>  		switch (stride) {
> @@ -10138,7 +10139,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  	}
>  }
>  
> -static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
> +static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
> +			       const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -10146,9 +10148,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  	int pipe = intel_crtc->pipe;
>  	uint32_t cntl = 0;
>  
> -	if (on) {
> +	if (plane_state && plane_state->visible) {
>  		cntl = MCURSOR_GAMMA_ENABLE;
> -		switch (intel_crtc->base.cursor->state->crtc_w) {
> +		switch (plane_state->base.crtc_w) {
>  			case 64:
>  				cntl |= CURSOR_MODE_64_ARGB_AX;
>  				break;
> @@ -10159,17 +10161,17 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  				cntl |= CURSOR_MODE_256_ARGB_AX;
>  				break;
>  			default:
> -				MISSING_CASE(intel_crtc->base.cursor->state->crtc_w);
> +				MISSING_CASE(plane_state->base.crtc_w);
>  				return;
>  		}
>  		cntl |= pipe << 28; /* Connect to correct pipe */
>  
>  		if (HAS_DDI(dev))
>  			cntl |= CURSOR_PIPE_CSC_ENABLE;
> -	}
>  
> -	if (crtc->cursor->state->rotation == BIT(DRM_ROTATE_180))
> -		cntl |= CURSOR_ROTATE_180;
> +		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
> +			cntl |= CURSOR_ROTATE_180;
> +	}
>  
>  	if (intel_crtc->cursor_cntl != cntl) {
>  		I915_WRITE(CURCNTR(pipe), cntl);
> @@ -10186,44 +10188,45 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  
>  /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
>  static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> -				     bool on)
> +				     const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
> -	struct drm_plane_state *cursor_state = crtc->cursor->state;
> -	int x = cursor_state->crtc_x;
> -	int y = cursor_state->crtc_y;
> -	u32 base = 0, pos = 0;
> +	u32 base = intel_crtc->cursor_addr;
> +	u32 pos = 0;
>  
> -	base = intel_crtc->cursor_addr;
> +	if (plane_state) {
> +		int x = plane_state->base.crtc_x;
> +		int y = plane_state->base.crtc_y;
>  
> -	if (x < 0) {
> -		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
> -		x = -x;
> -	}
> -	pos |= x << CURSOR_X_SHIFT;
> +		if (x < 0) {
> +			pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
> +			x = -x;
> +		}
> +		pos |= x << CURSOR_X_SHIFT;
>  
> -	if (y < 0) {
> -		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
> -		y = -y;
> +		if (y < 0) {
> +			pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
> +			y = -y;
> +		}
> +		pos |= y << CURSOR_Y_SHIFT;
> +
> +		/* ILK+ do this automagically */
> +		if (HAS_GMCH_DISPLAY(dev) &&
> +		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> +			base += (plane_state->base.crtc_h *
> +				 plane_state->base.crtc_w - 1) * 4;
> +		}
>  	}
> -	pos |= y << CURSOR_Y_SHIFT;
>  
>  	I915_WRITE(CURPOS(pipe), pos);
>  
> -	/* ILK+ do this automagically */
> -	if (HAS_GMCH_DISPLAY(dev) &&
> -	    crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) {
> -		base += (cursor_state->crtc_h *
> -			 cursor_state->crtc_w - 1) * 4;
> -	}
> -
>  	if (IS_845G(dev) || IS_I865G(dev))
> -		i845_update_cursor(crtc, base, on);
> +		i845_update_cursor(crtc, base, plane_state);
>  	else
> -		i9xx_update_cursor(crtc, base, on);
> +		i9xx_update_cursor(crtc, base, plane_state);
>  }
>  
>  static bool cursor_size_ok(struct drm_device *dev,
> @@ -14180,22 +14183,20 @@ intel_disable_cursor_plane(struct drm_plane *plane,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
>  	intel_crtc->cursor_addr = 0;
> -	intel_crtc_update_cursor(crtc, false);
> +	intel_crtc_update_cursor(crtc, NULL);
>  }
>  
>  static void
> -intel_commit_cursor_plane(struct drm_plane *plane,
> -			  struct intel_plane_state *state)
> +intel_update_cursor_plane(struct drm_plane *plane,
> +			  const struct intel_crtc_state *crtc_state,
> +			  const struct intel_plane_state *state)
>  {
> -	struct drm_crtc *crtc = state->base.crtc;
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = plane->dev;
> -	struct intel_crtc *intel_crtc;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>  	uint32_t addr;
>  
> -	crtc = crtc ? crtc : plane->crtc;
> -	intel_crtc = to_intel_crtc(crtc);
> -
>  	if (!obj)
>  		addr = 0;
>  	else if (!INTEL_INFO(dev)->cursor_needs_physical)
> @@ -14204,8 +14205,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  		addr = obj->phys_handle->busaddr;
>  
>  	intel_crtc->cursor_addr = addr;
> -
> -	intel_crtc_update_cursor(crtc, state->visible);
> +	intel_crtc_update_cursor(crtc, state);
>  }
>  
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> @@ -14231,7 +14231,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->plane = pipe;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  	cursor->check_plane = intel_check_cursor_plane;
> -	cursor->commit_plane = intel_commit_cursor_plane;
> +	cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>  
>  	drm_universal_plane_init(dev, &cursor->base, 0,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915: Use plane state for primary plane updates.

[Maarten Lankhorst]

Pass in the atomic states to allow for proper updates.
This removes uses of intel_crtc->config and direct access
to plane->state.

This breaks the last bit of kgdboc, but that appears to be dead code.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   3 -
 drivers/gpu/drm/i915/intel_display.c | 248 +++++++++++++++--------------------
 2 files changed, 103 insertions(+), 148 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bde9c764b415..75d8f89088aa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -648,9 +648,6 @@ struct drm_i915_display_funcs {
 			  struct drm_i915_gem_object *obj,
 			  struct drm_i915_gem_request *req,
 			  uint32_t flags);
-	void (*update_primary_plane)(struct drm_crtc *crtc,
-				     struct drm_framebuffer *fb,
-				     int x, int y);
 	void (*hpd_irq_setup)(struct drm_device *dev);
 	/* clock updates for mode set */
 	/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f7c293638df4..f276caf65a68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2679,36 +2679,23 @@ valid_fb:
 	obj->frontbuffer_bits |= to_intel_plane(primary)->frontbuffer_bit;
 }
 
-static void i9xx_update_primary_plane(struct drm_crtc *crtc,
-				      struct drm_framebuffer *fb,
-				      int x, int y)
+static void i9xx_update_primary_plane(struct drm_plane *primary,
+				      const struct intel_crtc_state *crtc_state,
+				      const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = primary->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_plane *primary = crtc->primary;
-	bool visible = to_intel_plane_state(primary->state)->visible;
-	struct drm_i915_gem_object *obj;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
+	int x = plane_state->src.x1 >> 16;
+	int y = plane_state->src.y1 >> 16;
 	u32 dspcntr;
 	i915_reg_t reg = DSPCNTR(plane);
 	int pixel_size;
 
-	if (!visible || !fb) {
-		I915_WRITE(reg, 0);
-		if (INTEL_INFO(dev)->gen >= 4)
-			I915_WRITE(DSPSURF(plane), 0);
-		else
-			I915_WRITE(DSPADDR(plane), 0);
-		POSTING_READ(reg);
-		return;
-	}
-
-	obj = intel_fb_obj(fb);
-	if (WARN_ON(obj == NULL))
-		return;
-
 	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
@@ -2723,13 +2710,13 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		 * which should always be the user's requested size.
 		 */
 		I915_WRITE(DSPSIZE(plane),
-			   ((intel_crtc->config->pipe_src_h - 1) << 16) |
-			   (intel_crtc->config->pipe_src_w - 1));
+			   ((crtc_state->pipe_src_h - 1) << 16) |
+			   (crtc_state->pipe_src_w - 1));
 		I915_WRITE(DSPPOS(plane), 0);
 	} else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
 		I915_WRITE(PRIMSIZE(plane),
-			   ((intel_crtc->config->pipe_src_h - 1) << 16) |
-			   (intel_crtc->config->pipe_src_w - 1));
+			   ((crtc_state->pipe_src_h - 1) << 16) |
+			   (crtc_state->pipe_src_w - 1));
 		I915_WRITE(PRIMPOS(plane), 0);
 		I915_WRITE(PRIMCNSTALPHA(plane), 0);
 	}
@@ -2780,17 +2767,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
-	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
+	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
 		dspcntr |= DISPPLANE_ROTATE_180;
 
-		x += (intel_crtc->config->pipe_src_w - 1);
-		y += (intel_crtc->config->pipe_src_h - 1);
+		x += (crtc_state->pipe_src_w - 1);
+		y += (crtc_state->pipe_src_h - 1);
 
 		/* Finding the last pixel of the last line of the display
 		data and adding to linear_offset*/
 		linear_offset +=
-			(intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
-			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
+			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
+			(crtc_state->pipe_src_w - 1) * pixel_size;
 	}
 
 	intel_crtc->adjusted_x = x;
@@ -2809,37 +2796,40 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	POSTING_READ(reg);
 }
 
-static void ironlake_update_primary_plane(struct drm_crtc *crtc,
-					  struct drm_framebuffer *fb,
-					  int x, int y)
+static void i9xx_disable_primary_plane(struct drm_plane *primary,
+				       struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_plane *primary = crtc->primary;
-	bool visible = to_intel_plane_state(primary->state)->visible;
-	struct drm_i915_gem_object *obj;
 	int plane = intel_crtc->plane;
-	unsigned long linear_offset;
-	u32 dspcntr;
-	i915_reg_t reg = DSPCNTR(plane);
-	int pixel_size;
 
-	if (!visible || !fb) {
-		I915_WRITE(reg, 0);
+	I915_WRITE(DSPCNTR(plane), 0);
+	if (INTEL_INFO(dev_priv)->gen >= 4)
 		I915_WRITE(DSPSURF(plane), 0);
-		POSTING_READ(reg);
-		return;
-	}
-
-	obj = intel_fb_obj(fb);
-	if (WARN_ON(obj == NULL))
-		return;
+	else
+		I915_WRITE(DSPADDR(plane), 0);
+	POSTING_READ(DSPCNTR(plane));
+}
 
-	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+static void ironlake_update_primary_plane(struct drm_plane *primary,
+					  const struct intel_crtc_state *crtc_state,
+					  const struct intel_plane_state *plane_state)
+{
+	struct drm_device *dev = primary->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	int plane = intel_crtc->plane;
+	unsigned long linear_offset;
+	u32 dspcntr;
+	i915_reg_t reg = DSPCNTR(plane);
+	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+	int x = plane_state->src.x1 >> 16;
+	int y = plane_state->src.y1 >> 16;
 
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
-
 	dspcntr |= DISPLAY_PLANE_ENABLE;
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -2881,18 +2871,18 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 					       pixel_size,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
-	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
+	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
 		dspcntr |= DISPPLANE_ROTATE_180;
 
 		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
-			x += (intel_crtc->config->pipe_src_w - 1);
-			y += (intel_crtc->config->pipe_src_h - 1);
+			x += (crtc_state->pipe_src_w - 1);
+			y += (crtc_state->pipe_src_h - 1);
 
 			/* Finding the last pixel of the last line of the display
 			data and adding to linear_offset*/
 			linear_offset +=
-				(intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
-				(intel_crtc->config->pipe_src_w - 1) * pixel_size;
+				(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
+				(crtc_state->pipe_src_w - 1) * pixel_size;
 		}
 	}
 
@@ -3083,36 +3073,30 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)
 	return 0;
 }
 
-static void skylake_update_primary_plane(struct drm_crtc *crtc,
-					 struct drm_framebuffer *fb,
-					 int x, int y)
+static void skylake_update_primary_plane(struct drm_plane *plane,
+					 const struct intel_crtc_state *crtc_state,
+					 const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_plane *plane = crtc->primary;
-	bool visible = to_intel_plane_state(plane->state)->visible;
-	struct drm_i915_gem_object *obj;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl, stride_div, stride;
 	u32 tile_height, plane_offset, plane_size;
-	unsigned int rotation;
+	unsigned int rotation = plane_state->base.rotation;
 	int x_offset, y_offset;
 	u32 surf_addr;
-	struct intel_crtc_state *crtc_state = intel_crtc->config;
-	struct intel_plane_state *plane_state;
-	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
-	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
-	int scaler_id = -1;
-
-	plane_state = to_intel_plane_state(plane->state);
-
-	if (!visible || !fb) {
-		I915_WRITE(PLANE_CTL(pipe, 0), 0);
-		I915_WRITE(PLANE_SURF(pipe, 0), 0);
-		POSTING_READ(PLANE_CTL(pipe, 0));
-		return;
-	}
+	int scaler_id = plane_state->scaler_id;
+	int src_x = plane_state->src.x1 >> 16;
+	int src_y = plane_state->src.y1 >> 16;
+	int src_w = drm_rect_width(&plane_state->src) >> 16;
+	int src_h = drm_rect_height(&plane_state->src) >> 16;
+	int dst_x = plane_state->dst.x1;
+	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);
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		    PLANE_CTL_PIPE_GAMMA_ENABLE |
@@ -3121,41 +3105,26 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
 	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-
-	rotation = plane->state->rotation;
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
-	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
 	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
 
 	WARN_ON(drm_rect_width(&plane_state->src) == 0);
 
-	scaler_id = plane_state->scaler_id;
-	src_x = plane_state->src.x1 >> 16;
-	src_y = plane_state->src.y1 >> 16;
-	src_w = drm_rect_width(&plane_state->src) >> 16;
-	src_h = drm_rect_height(&plane_state->src) >> 16;
-	dst_x = plane_state->dst.x1;
-	dst_y = plane_state->dst.y1;
-	dst_w = drm_rect_width(&plane_state->dst);
-	dst_h = drm_rect_height(&plane_state->dst);
-
-	WARN_ON(x != src_x || y != src_y);
-
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride = Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->pixel_format,
 						fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
-		x_offset = stride * tile_height - y - src_h;
-		y_offset = x;
+		x_offset = stride * tile_height - src_y - src_h;
+		y_offset = src_x;
 		plane_size = (src_w - 1) << 16 | (src_h - 1);
 	} else {
 		stride = fb->pitches[0] / stride_div;
-		x_offset = x;
-		y_offset = y;
+		x_offset = src_x;
+		y_offset = src_y;
 		plane_size = (src_h - 1) << 16 | (src_w - 1);
 	}
 	plane_offset = y_offset << 16 | x_offset;
@@ -3188,20 +3157,30 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	POSTING_READ(PLANE_SURF(pipe, 0));
 }
 
-/* Assume fb object is pinned & idle & fenced and just update base pointers */
-static int
-intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-			   int x, int y, enum mode_set_atomic state)
+static void skylake_disable_primary_plane(struct drm_plane *primary,
+					  struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = to_intel_crtc(crtc)->pipe;
 
 	if (dev_priv->fbc.deactivate)
 		dev_priv->fbc.deactivate(dev_priv);
 
-	dev_priv->display.update_primary_plane(crtc, fb, x, y);
+	I915_WRITE(PLANE_CTL(pipe, 0), 0);
+	I915_WRITE(PLANE_SURF(pipe, 0), 0);
+	POSTING_READ(PLANE_SURF(pipe, 0));
+}
 
-	return 0;
+/* Assume fb object is pinned & idle & fenced and just update base pointers */
+static int
+intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
+			   int x, int y, enum mode_set_atomic state)
+{
+	/* Support for kgdboc is disabled, this needs a major rework. */
+	DRM_ERROR("legacy panic handler not supported any more.\n");
+
+	return -ENODEV;
 }
 
 static void intel_complete_page_flips(struct drm_device *dev)
@@ -3228,8 +3207,10 @@ static void intel_update_primary_planes(struct drm_device *dev)
 		drm_modeset_lock_crtc(crtc, &plane->base);
 		plane_state = to_intel_plane_state(plane->base.state);
 
-		if (crtc->state->active && plane_state->base.fb)
-			plane->commit_plane(&plane->base, plane_state);
+		if (plane_state->visible)
+			plane->update_plane(&plane->base,
+					    to_intel_crtc_state(crtc->state),
+					    plane_state);
 
 		drm_modeset_unlock_crtc(crtc);
 	}
@@ -13958,32 +13939,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 					     &state->visible);
 }
 
-static void
-intel_commit_primary_plane(struct drm_plane *plane,
-			   struct intel_plane_state *state)
-{
-	struct drm_crtc *crtc = state->base.crtc;
-	struct drm_framebuffer *fb = state->base.fb;
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	crtc = crtc ? crtc : plane->crtc;
-
-	dev_priv->display.update_primary_plane(crtc, fb,
-					       state->src.x1 >> 16,
-					       state->src.y1 >> 16);
-}
-
-static void
-intel_disable_primary_plane(struct drm_plane *plane,
-			    struct drm_crtc *crtc)
-{
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
-}
-
 static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_crtc_state)
 {
@@ -14068,20 +14023,33 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->plane = pipe;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
-	primary->commit_plane = intel_commit_primary_plane;
-	primary->disable_plane = intel_disable_primary_plane;
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
 	if (INTEL_INFO(dev)->gen >= 9) {
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
+
+		primary->update_plane = skylake_update_primary_plane;
+		primary->disable_plane = skylake_disable_primary_plane;
+	} else if (HAS_PCH_SPLIT(dev)) {
+		intel_primary_formats = i965_primary_formats;
+		num_formats = ARRAY_SIZE(i965_primary_formats);
+
+		primary->update_plane = ironlake_update_primary_plane;
+		primary->disable_plane = i9xx_disable_primary_plane;
 	} else if (INTEL_INFO(dev)->gen >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
+
+		primary->update_plane = i9xx_update_primary_plane;
+		primary->disable_plane = i9xx_disable_primary_plane;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
+
+		primary->update_plane = i9xx_update_primary_plane;
+		primary->disable_plane = i9xx_disable_primary_plane;
 	}
 
 	drm_universal_plane_init(dev, &primary->base, 0,
@@ -14909,8 +14877,6 @@ static void intel_init_display(struct drm_device *dev)
 			haswell_crtc_compute_clock;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
-		dev_priv->display.update_primary_plane =
-			skylake_update_primary_plane;
 	} else if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
@@ -14919,8 +14885,6 @@ static void intel_init_display(struct drm_device *dev)
 			haswell_crtc_compute_clock;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
-		dev_priv->display.update_primary_plane =
-			ironlake_update_primary_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
@@ -14929,8 +14893,6 @@ static void intel_init_display(struct drm_device *dev)
 			ironlake_crtc_compute_clock;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
-		dev_priv->display.update_primary_plane =
-			ironlake_update_primary_plane;
 	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
@@ -14938,8 +14900,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
-		dev_priv->display.update_primary_plane =
-			i9xx_update_primary_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
@@ -14947,8 +14907,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
-		dev_priv->display.update_primary_plane =
-			i9xx_update_primary_plane;
 	}
 
 	/* Returns the core display clock speed */
-- 
2.1.0

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

Re: [PATCH 6/7] drm/i915: Use plane state for primary plane updates.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:11AM +0100, Maarten Lankhorst wrote:
> Pass in the atomic states to allow for proper updates.
> This removes uses of intel_crtc->config and direct access
> to plane->state.
> 
> This breaks the last bit of kgdboc, but that appears to be dead code.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

lgtm.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   3 -
>  drivers/gpu/drm/i915/intel_display.c | 248 +++++++++++++++--------------------
>  2 files changed, 103 insertions(+), 148 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bde9c764b415..75d8f89088aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -648,9 +648,6 @@ struct drm_i915_display_funcs {
>  			  struct drm_i915_gem_object *obj,
>  			  struct drm_i915_gem_request *req,
>  			  uint32_t flags);
> -	void (*update_primary_plane)(struct drm_crtc *crtc,
> -				     struct drm_framebuffer *fb,
> -				     int x, int y);
>  	void (*hpd_irq_setup)(struct drm_device *dev);
>  	/* clock updates for mode set */
>  	/* cursor updates */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f7c293638df4..f276caf65a68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2679,36 +2679,23 @@ valid_fb:
>  	obj->frontbuffer_bits |= to_intel_plane(primary)->frontbuffer_bit;
>  }
>  
> -static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> -				      struct drm_framebuffer *fb,
> -				      int x, int y)
> +static void i9xx_update_primary_plane(struct drm_plane *primary,
> +				      const struct intel_crtc_state *crtc_state,
> +				      const struct intel_plane_state *plane_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = primary->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_plane *primary = crtc->primary;
> -	bool visible = to_intel_plane_state(primary->state)->visible;
> -	struct drm_i915_gem_object *obj;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
> +	int x = plane_state->src.x1 >> 16;
> +	int y = plane_state->src.y1 >> 16;
>  	u32 dspcntr;
>  	i915_reg_t reg = DSPCNTR(plane);
>  	int pixel_size;
>  
> -	if (!visible || !fb) {
> -		I915_WRITE(reg, 0);
> -		if (INTEL_INFO(dev)->gen >= 4)
> -			I915_WRITE(DSPSURF(plane), 0);
> -		else
> -			I915_WRITE(DSPADDR(plane), 0);
> -		POSTING_READ(reg);
> -		return;
> -	}
> -
> -	obj = intel_fb_obj(fb);
> -	if (WARN_ON(obj == NULL))
> -		return;
> -
>  	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
> @@ -2723,13 +2710,13 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		 * which should always be the user's requested size.
>  		 */
>  		I915_WRITE(DSPSIZE(plane),
> -			   ((intel_crtc->config->pipe_src_h - 1) << 16) |
> -			   (intel_crtc->config->pipe_src_w - 1));
> +			   ((crtc_state->pipe_src_h - 1) << 16) |
> +			   (crtc_state->pipe_src_w - 1));
>  		I915_WRITE(DSPPOS(plane), 0);
>  	} else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
>  		I915_WRITE(PRIMSIZE(plane),
> -			   ((intel_crtc->config->pipe_src_h - 1) << 16) |
> -			   (intel_crtc->config->pipe_src_w - 1));
> +			   ((crtc_state->pipe_src_h - 1) << 16) |
> +			   (crtc_state->pipe_src_w - 1));
>  		I915_WRITE(PRIMPOS(plane), 0);
>  		I915_WRITE(PRIMCNSTALPHA(plane), 0);
>  	}
> @@ -2780,17 +2767,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> -	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
> +	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
>  		dspcntr |= DISPPLANE_ROTATE_180;
>  
> -		x += (intel_crtc->config->pipe_src_w - 1);
> -		y += (intel_crtc->config->pipe_src_h - 1);
> +		x += (crtc_state->pipe_src_w - 1);
> +		y += (crtc_state->pipe_src_h - 1);
>  
>  		/* Finding the last pixel of the last line of the display
>  		data and adding to linear_offset*/
>  		linear_offset +=
> -			(intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> -			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
> +			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> +			(crtc_state->pipe_src_w - 1) * pixel_size;
>  	}
>  
>  	intel_crtc->adjusted_x = x;
> @@ -2809,37 +2796,40 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	POSTING_READ(reg);
>  }
>  
> -static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> -					  struct drm_framebuffer *fb,
> -					  int x, int y)
> +static void i9xx_disable_primary_plane(struct drm_plane *primary,
> +				       struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_plane *primary = crtc->primary;
> -	bool visible = to_intel_plane_state(primary->state)->visible;
> -	struct drm_i915_gem_object *obj;
>  	int plane = intel_crtc->plane;
> -	unsigned long linear_offset;
> -	u32 dspcntr;
> -	i915_reg_t reg = DSPCNTR(plane);
> -	int pixel_size;
>  
> -	if (!visible || !fb) {
> -		I915_WRITE(reg, 0);
> +	I915_WRITE(DSPCNTR(plane), 0);
> +	if (INTEL_INFO(dev_priv)->gen >= 4)
>  		I915_WRITE(DSPSURF(plane), 0);
> -		POSTING_READ(reg);
> -		return;
> -	}
> -
> -	obj = intel_fb_obj(fb);
> -	if (WARN_ON(obj == NULL))
> -		return;
> +	else
> +		I915_WRITE(DSPADDR(plane), 0);
> +	POSTING_READ(DSPCNTR(plane));
> +}
>  
> -	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> +static void ironlake_update_primary_plane(struct drm_plane *primary,
> +					  const struct intel_crtc_state *crtc_state,
> +					  const struct intel_plane_state *plane_state)
> +{
> +	struct drm_device *dev = primary->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +	int plane = intel_crtc->plane;
> +	unsigned long linear_offset;
> +	u32 dspcntr;
> +	i915_reg_t reg = DSPCNTR(plane);
> +	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> +	int x = plane_state->src.x1 >> 16;
> +	int y = plane_state->src.y1 >> 16;
>  
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
> -
>  	dspcntr |= DISPLAY_PLANE_ENABLE;
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> @@ -2881,18 +2871,18 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  					       pixel_size,
>  					       fb->pitches[0]);
>  	linear_offset -= intel_crtc->dspaddr_offset;
> -	if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
> +	if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
>  		dspcntr |= DISPPLANE_ROTATE_180;
>  
>  		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> -			x += (intel_crtc->config->pipe_src_w - 1);
> -			y += (intel_crtc->config->pipe_src_h - 1);
> +			x += (crtc_state->pipe_src_w - 1);
> +			y += (crtc_state->pipe_src_h - 1);
>  
>  			/* Finding the last pixel of the last line of the display
>  			data and adding to linear_offset*/
>  			linear_offset +=
> -				(intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> -				(intel_crtc->config->pipe_src_w - 1) * pixel_size;
> +				(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> +				(crtc_state->pipe_src_w - 1) * pixel_size;
>  		}
>  	}
>  
> @@ -3083,36 +3073,30 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)
>  	return 0;
>  }
>  
> -static void skylake_update_primary_plane(struct drm_crtc *crtc,
> -					 struct drm_framebuffer *fb,
> -					 int x, int y)
> +static void skylake_update_primary_plane(struct drm_plane *plane,
> +					 const struct intel_crtc_state *crtc_state,
> +					 const struct intel_plane_state *plane_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_plane *plane = crtc->primary;
> -	bool visible = to_intel_plane_state(plane->state)->visible;
> -	struct drm_i915_gem_object *obj;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl, stride_div, stride;
>  	u32 tile_height, plane_offset, plane_size;
> -	unsigned int rotation;
> +	unsigned int rotation = plane_state->base.rotation;
>  	int x_offset, y_offset;
>  	u32 surf_addr;
> -	struct intel_crtc_state *crtc_state = intel_crtc->config;
> -	struct intel_plane_state *plane_state;
> -	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> -	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> -	int scaler_id = -1;
> -
> -	plane_state = to_intel_plane_state(plane->state);
> -
> -	if (!visible || !fb) {
> -		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -		POSTING_READ(PLANE_CTL(pipe, 0));
> -		return;
> -	}
> +	int scaler_id = plane_state->scaler_id;
> +	int src_x = plane_state->src.x1 >> 16;
> +	int src_y = plane_state->src.y1 >> 16;
> +	int src_w = drm_rect_width(&plane_state->src) >> 16;
> +	int src_h = drm_rect_height(&plane_state->src) >> 16;
> +	int dst_x = plane_state->dst.x1;
> +	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);
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -3121,41 +3105,26 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
>  	plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
>  	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -
> -	rotation = plane->state->rotation;
>  	plane_ctl |= skl_plane_ctl_rotation(rotation);
>  
> -	obj = intel_fb_obj(fb);
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
>  	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
>  
>  	WARN_ON(drm_rect_width(&plane_state->src) == 0);
>  
> -	scaler_id = plane_state->scaler_id;
> -	src_x = plane_state->src.x1 >> 16;
> -	src_y = plane_state->src.y1 >> 16;
> -	src_w = drm_rect_width(&plane_state->src) >> 16;
> -	src_h = drm_rect_height(&plane_state->src) >> 16;
> -	dst_x = plane_state->dst.x1;
> -	dst_y = plane_state->dst.y1;
> -	dst_w = drm_rect_width(&plane_state->dst);
> -	dst_h = drm_rect_height(&plane_state->dst);
> -
> -	WARN_ON(x != src_x || y != src_y);
> -
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb->pixel_format,
>  						fb->modifier[0], 0);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
> -		x_offset = stride * tile_height - y - src_h;
> -		y_offset = x;
> +		x_offset = stride * tile_height - src_y - src_h;
> +		y_offset = src_x;
>  		plane_size = (src_w - 1) << 16 | (src_h - 1);
>  	} else {
>  		stride = fb->pitches[0] / stride_div;
> -		x_offset = x;
> -		y_offset = y;
> +		x_offset = src_x;
> +		y_offset = src_y;
>  		plane_size = (src_h - 1) << 16 | (src_w - 1);
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
> @@ -3188,20 +3157,30 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	POSTING_READ(PLANE_SURF(pipe, 0));
>  }
>  
> -/* Assume fb object is pinned & idle & fenced and just update base pointers */
> -static int
> -intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> -			   int x, int y, enum mode_set_atomic state)
> +static void skylake_disable_primary_plane(struct drm_plane *primary,
> +					  struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe = to_intel_crtc(crtc)->pipe;
>  
>  	if (dev_priv->fbc.deactivate)
>  		dev_priv->fbc.deactivate(dev_priv);
>  
> -	dev_priv->display.update_primary_plane(crtc, fb, x, y);
> +	I915_WRITE(PLANE_CTL(pipe, 0), 0);
> +	I915_WRITE(PLANE_SURF(pipe, 0), 0);
> +	POSTING_READ(PLANE_SURF(pipe, 0));
> +}
>  
> -	return 0;
> +/* Assume fb object is pinned & idle & fenced and just update base pointers */
> +static int
> +intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> +			   int x, int y, enum mode_set_atomic state)
> +{
> +	/* Support for kgdboc is disabled, this needs a major rework. */
> +	DRM_ERROR("legacy panic handler not supported any more.\n");
> +
> +	return -ENODEV;
>  }
>  
>  static void intel_complete_page_flips(struct drm_device *dev)
> @@ -3228,8 +3207,10 @@ static void intel_update_primary_planes(struct drm_device *dev)
>  		drm_modeset_lock_crtc(crtc, &plane->base);
>  		plane_state = to_intel_plane_state(plane->base.state);
>  
> -		if (crtc->state->active && plane_state->base.fb)
> -			plane->commit_plane(&plane->base, plane_state);
> +		if (plane_state->visible)
> +			plane->update_plane(&plane->base,
> +					    to_intel_crtc_state(crtc->state),
> +					    plane_state);
>  
>  		drm_modeset_unlock_crtc(crtc);
>  	}
> @@ -13958,32 +13939,6 @@ intel_check_primary_plane(struct drm_plane *plane,
>  					     &state->visible);
>  }
>  
> -static void
> -intel_commit_primary_plane(struct drm_plane *plane,
> -			   struct intel_plane_state *state)
> -{
> -	struct drm_crtc *crtc = state->base.crtc;
> -	struct drm_framebuffer *fb = state->base.fb;
> -	struct drm_device *dev = plane->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	crtc = crtc ? crtc : plane->crtc;
> -
> -	dev_priv->display.update_primary_plane(crtc, fb,
> -					       state->src.x1 >> 16,
> -					       state->src.y1 >> 16);
> -}
> -
> -static void
> -intel_disable_primary_plane(struct drm_plane *plane,
> -			    struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev = plane->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
> -}
> -
>  static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  				    struct drm_crtc_state *old_crtc_state)
>  {
> @@ -14068,20 +14023,33 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	primary->plane = pipe;
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
> -	primary->commit_plane = intel_commit_primary_plane;
> -	primary->disable_plane = intel_disable_primary_plane;
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>  		primary->plane = !pipe;
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
>  		intel_primary_formats = skl_primary_formats;
>  		num_formats = ARRAY_SIZE(skl_primary_formats);
> +
> +		primary->update_plane = skylake_update_primary_plane;
> +		primary->disable_plane = skylake_disable_primary_plane;
> +	} else if (HAS_PCH_SPLIT(dev)) {
> +		intel_primary_formats = i965_primary_formats;
> +		num_formats = ARRAY_SIZE(i965_primary_formats);
> +
> +		primary->update_plane = ironlake_update_primary_plane;
> +		primary->disable_plane = i9xx_disable_primary_plane;
>  	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = i965_primary_formats;
>  		num_formats = ARRAY_SIZE(i965_primary_formats);
> +
> +		primary->update_plane = i9xx_update_primary_plane;
> +		primary->disable_plane = i9xx_disable_primary_plane;
>  	} else {
>  		intel_primary_formats = i8xx_primary_formats;
>  		num_formats = ARRAY_SIZE(i8xx_primary_formats);
> +
> +		primary->update_plane = i9xx_update_primary_plane;
> +		primary->disable_plane = i9xx_disable_primary_plane;
>  	}
>  
>  	drm_universal_plane_init(dev, &primary->base, 0,
> @@ -14909,8 +14877,6 @@ static void intel_init_display(struct drm_device *dev)
>  			haswell_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = haswell_crtc_enable;
>  		dev_priv->display.crtc_disable = haswell_crtc_disable;
> -		dev_priv->display.update_primary_plane =
> -			skylake_update_primary_plane;
>  	} else if (HAS_DDI(dev)) {
>  		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14919,8 +14885,6 @@ static void intel_init_display(struct drm_device *dev)
>  			haswell_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = haswell_crtc_enable;
>  		dev_priv->display.crtc_disable = haswell_crtc_disable;
> -		dev_priv->display.update_primary_plane =
> -			ironlake_update_primary_plane;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14929,8 +14893,6 @@ static void intel_init_display(struct drm_device *dev)
>  			ironlake_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
> -		dev_priv->display.update_primary_plane =
> -			ironlake_update_primary_plane;
>  	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14938,8 +14900,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> -		dev_priv->display.update_primary_plane =
> -			i9xx_update_primary_plane;
>  	} else {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14947,8 +14907,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> -		dev_priv->display.update_primary_plane =
> -			i9xx_update_primary_plane;
>  	}
>  
>  	/* Returns the core display clock speed */
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915: Remove commit_plane function pointer.

[Maarten Lankhorst]

With sprites, cursors and primary planes taking the atomic state
this is now unused. It's removed in a separate commit to allow
a revert.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +---
 drivers/gpu/drm/i915/intel_drv.h          | 2 --
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 969aa410deaa..e0b851a0004a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -198,9 +198,7 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 	struct drm_crtc_state *crtc_state =
 		drm_atomic_get_existing_crtc_state(old_state->state, crtc);
 
-	if (intel_plane->commit_plane)
-		intel_plane->commit_plane(plane, intel_state);
-	else if (intel_state->visible)
+	if (intel_state->visible)
 		intel_plane->update_plane(plane,
 					  to_intel_crtc_state(crtc_state),
 					  intel_state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 068050884353..5c41d2030594 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -669,8 +669,6 @@ struct intel_plane {
 	int (*check_plane)(struct drm_plane *plane,
 			   struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *state);
-	void (*commit_plane)(struct drm_plane *plane,
-			     struct intel_plane_state *state);
 };
 
 struct intel_watermark_params {
-- 
2.1.0

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

Re: [PATCH 7/7] drm/i915: Remove commit_plane function pointer.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 11:54:12AM +0100, Maarten Lankhorst wrote:
> With sprites, cursors and primary planes taking the atomic state
> this is now unused. It's removed in a separate commit to allow
> a revert.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

One less weird difference between the planes. Yay.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +---
>  drivers/gpu/drm/i915/intel_drv.h          | 2 --
>  2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 969aa410deaa..e0b851a0004a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -198,9 +198,7 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
>  	struct drm_crtc_state *crtc_state =
>  		drm_atomic_get_existing_crtc_state(old_state->state, crtc);
>  
> -	if (intel_plane->commit_plane)
> -		intel_plane->commit_plane(plane, intel_state);
> -	else if (intel_state->visible)
> +	if (intel_state->visible)
>  		intel_plane->update_plane(plane,
>  					  to_intel_crtc_state(crtc_state),
>  					  intel_state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 068050884353..5c41d2030594 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -669,8 +669,6 @@ struct intel_plane {
>  	int (*check_plane)(struct drm_plane *plane,
>  			   struct intel_crtc_state *crtc_state,
>  			   struct intel_plane_state *state);
> -	void (*commit_plane)(struct drm_plane *plane,
> -			     struct intel_plane_state *state);
>  };
>  
>  struct intel_watermark_params {
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ failure: Fi.CI.BAT

[Patchwork]

== Summary ==

HEAD is now at ff88655 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest
Applying: drm/i915: Use passed plane state for sprite planes, v4.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_drv.h
M	drivers/gpu/drm/i915/intel_sprite.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_sprite.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_sprite.c
Patch failed at 0001 drm/i915: Use passed plane state for sprite planes, v4.

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