[13/14] drm/i915: skylake primary plane scaling using shared scalers

Commit Message

Chandra Konduru April 15, 2015, 10:14 p.m. UTC

This patch enables skylake primary plane scaling using shared
scalers atomic desgin.

v2:
-use single copy of scaler limits (Matt)

v3:
-move detach_scalers to crtc commit path (Matt)
-use values in plane_state->src as regular integers (me)

v4:
-changes to align with updated scaler structures (Matt, me)
-keep plane src rect in 16.16 format (Matt, Daniel)

v5:
-Rebased on top of 90/270 rotation changes (me)
-Fixed an issue introduced by 90/270 changes where plane programming
 is using drm_plane->state rect instead of intel_plane->state rect.
 This change also required for scaling to work properly. (me)
-With 90/270, updated limits to cover both portrait and landscape usages (me)
-Refactored skylake_update_primary_plane to reduce its size (Daniel)
 Added helper functions for refactoring are comprehended enough to be
 used for skylake_update_plane (for sprite) too. One stop towards
 having single function for all planes.

Signed-off-by: Chandra Konduru <[e-mail]>
Reviewed-by: Matt Roper <[e-mail]>
Testcase: kms_plane_scaling
---
 drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
 drivers/gpu/drm/i915/intel_display.c |  261 +++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h     |    8 +-
 drivers/gpu/drm/i915/intel_sprite.c  |    9 ++
 4 files changed, 213 insertions(+), 70 deletions(-)

Comments

Chandra Konduru April 15, 2015, 11:08 p.m. UTC | #1

Sonika,
I have rebased 13/14 and 14/14 of scaler series on top of 90/270
and did some refactoring to reduce function size. 
Previous versions are already reviewed and Matt gave r-b
for those changes.
Can you review v5 changes and give r-b or ack for them?
You can see below (v5) list of changes triggered by 90/270.


-Chandra

> -----Original Message-----
> From: Konduru, Chandra
> Sent: Wednesday, April 15, 2015 3:15 PM
> To: [e-mail]
> Cc: Konduru, Chandra; Jindal, Sonika; Roper, Matthew D; Vetter, Daniel;
> Conselvan De Oliveira, Ander
> Subject: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared
> scalers
> 
> This patch enables skylake primary plane scaling using shared
> scalers atomic desgin.
> 
> v2:
> -use single copy of scaler limits (Matt)
> 
> v3:
> -move detach_scalers to crtc commit path (Matt)
> -use values in plane_state->src as regular integers (me)
> 
> v4:
> -changes to align with updated scaler structures (Matt, me)
> -keep plane src rect in 16.16 format (Matt, Daniel)
> 
> v5:
> -Rebased on top of 90/270 rotation changes (me)
> -Fixed an issue introduced by 90/270 changes where plane programming
>  is using drm_plane->state rect instead of intel_plane->state rect.
>  This change also required for scaling to work properly. (me)
> -With 90/270, updated limits to cover both portrait and landscape usages (me)
> -Refactored skylake_update_primary_plane to reduce its size (Daniel)
>  Added helper functions for refactoring are comprehended enough to be
>  used for skylake_update_plane (for sprite) too. One stop towards
>  having single function for all planes.
> 
> Signed-off-by: Chandra Konduru <[e-mail]>
> Reviewed-by: Matt Roper <[e-mail]>
> Testcase: kms_plane_scaling
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |  261 +++++++++++++++++++++++++------
> ---
>  drivers/gpu/drm/i915/intel_drv.h     |    8 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++
>  4 files changed, 213 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 3c4b7cd..cb6d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev,
>  		plane->state->state = NULL;
>  	}
> 
> -	/* swap crtc_state */
> +	/* swap crtc_scaler_state */
>  	for (i = 0; i < dev->mode_config.num_crtc; i++) {
>  		struct drm_crtc *crtc = state->crtcs[i];
>  		if (!crtc) {
> @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev,
> 
>  		to_intel_crtc(crtc)->config->scaler_state =
>  			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> +
> +		if (INTEL_INFO(dev)->gen >= 9)
> +			skl_detach_scalers(to_intel_crtc(crtc));
>  	}
> 
>  	drm_atomic_helper_commit_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5f7a40f..7f3ae8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc
> *intel_crtc)
>  	}
>  }
> 
> -static void skylake_update_primary_plane(struct drm_crtc *crtc,
> -					 struct drm_framebuffer *fb,
> -					 int x, int y)
> +u32 skl_plane_ctl_format(uint32_t pixel_format)
>  {
> -	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_i915_gem_object *obj;
> -	int pipe = intel_crtc->pipe;
> -	u32 plane_ctl, stride_div, stride;
> -	u32 tile_height, plane_offset, plane_size;
> -	unsigned int rotation;
> -	int x_offset, y_offset;
> -	unsigned long surf_addr;
> -	struct drm_plane *plane;
> -
> -	if (!intel_crtc->primary_enabled) {
> -		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -		POSTING_READ(PLANE_CTL(pipe, 0));
> -		return;
> -	}
> -
> -	plane_ctl = PLANE_CTL_ENABLE |
> -		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> -		    PLANE_CTL_PIPE_CSC_ENABLE;
> -
> -	switch (fb->pixel_format) {
> +	u32 plane_ctl_format = 0;
> +	switch (pixel_format) {
>  	case DRM_FORMAT_RGB565:
> -		plane_ctl |= PLANE_CTL_FORMAT_RGB_565;
> -		break;
> -	case DRM_FORMAT_XRGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		break;
> -	case DRM_FORMAT_ARGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_RGB_565;
>  		break;
>  	case DRM_FORMAT_XBGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> PLANE_CTL_ORDER_RGBX;
>  		break;
> +	case DRM_FORMAT_XRGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888;
> +		break;
> +	/*
> +	 * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> +	 * to be already pre-multiplied. We need to add a knob (or a different
> +	 * DRM_FORMAT) for user-space to configure that.
> +	 */
>  	case DRM_FORMAT_ABGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> PLANE_CTL_ORDER_RGBX |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>  		break;
>  	case DRM_FORMAT_XRGB2101010:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_2101010;
>  		break;
>  	case DRM_FORMAT_XBGR2101010:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_ORDER_RGBX |
> PLANE_CTL_FORMAT_XRGB_2101010;
> +		break;
> +	case DRM_FORMAT_YUYV:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 |
> PLANE_CTL_YUV422_YUYV;
> +		break;
> +	case DRM_FORMAT_YVYU:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 |
> PLANE_CTL_YUV422_YVYU;
> +		break;
> +	case DRM_FORMAT_UYVY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 |
> PLANE_CTL_YUV422_UYVY;
> +		break;
> +	case DRM_FORMAT_VYUY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 |
> PLANE_CTL_YUV422_VYUY;
>  		break;
>  	default:
>  		BUG();
>  	}
> +	return plane_ctl_format;
> +}
> 
> -	switch (fb->modifier[0]) {
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> +{
> +	u32 plane_ctl_tiling = 0;
> +	switch (fb_modifier) {
>  	case DRM_FORMAT_MOD_NONE:
>  		break;
>  	case I915_FORMAT_MOD_X_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_X;
> +		plane_ctl_tiling = PLANE_CTL_TILED_X;
>  		break;
>  	case I915_FORMAT_MOD_Y_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_Y;
> +		plane_ctl_tiling = PLANE_CTL_TILED_Y;
>  		break;
>  	case I915_FORMAT_MOD_Yf_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_YF;
> +		plane_ctl_tiling = PLANE_CTL_TILED_YF;
>  		break;
>  	default:
> -		MISSING_CASE(fb->modifier[0]);
> +		MISSING_CASE(fb_modifier);
>  	}
> +	return plane_ctl_tiling;
> +}
> 
> -	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -
> -	plane = crtc->primary;
> -	rotation = plane->state->rotation;
> +u32 skl_plane_ctl_rotation(unsigned int rotation)
> +{
> +	u32 plane_ctl_rotation = 0;
>  	switch (rotation) {
> +	case BIT(DRM_ROTATE_0):
> +		break;
>  	case BIT(DRM_ROTATE_90):
> -		plane_ctl |= PLANE_CTL_ROTATE_90;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_90;
>  		break;
> -
>  	case BIT(DRM_ROTATE_180):
> -		plane_ctl |= PLANE_CTL_ROTATE_180;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_180;
>  		break;
> -
>  	case BIT(DRM_ROTATE_270):
> -		plane_ctl |= PLANE_CTL_ROTATE_270;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_270;
>  		break;
> +	default:
> +		MISSING_CASE(rotation);
>  	}
> 
> +	return plane_ctl_rotation;
> +}
> +
> +static void skylake_update_primary_plane(struct drm_crtc *crtc,
> +					 struct drm_framebuffer *fb,
> +					 int x, int y)
> +{
> +	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_i915_gem_object *obj;
> +	int pipe = intel_crtc->pipe;
> +	u32 plane_ctl, stride_div, stride;
> +	u32 tile_height, plane_offset, plane_size;
> +	unsigned int rotation;
> +	int x_offset, y_offset;
> +	unsigned long surf_addr;
> +	struct drm_plane *plane;
> +	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 = crtc->primary;
> +	plane_state = plane ? to_intel_plane_state(crtc->primary->state) :
> NULL;
> +
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> +		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> +		POSTING_READ(PLANE_CTL(pipe, 0));
> +		return;
> +	}
> +
> +	plane_ctl = PLANE_CTL_ENABLE |
> +		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> +		    PLANE_CTL_PIPE_CSC_ENABLE;
> +
> +	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);
> 
> +	if (plane_state && drm_rect_width(&plane_state->src)) {
> +		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);
> +	} else {
> +		src_w = intel_crtc->config->pipe_src_w;
> +		src_h = intel_crtc->config->pipe_src_h;
> +	}
> +
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
>  						fb->modifier[0]);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
> -		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
> +		x_offset = stride * tile_height - y - src_h;
>  		y_offset = x;
> -		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
> -					((plane->state->src_h >> 16) - 1);
> +		plane_size = (src_w - 1) << 16 | (src_h - 1);
>  	} else {
>  		stride = fb->pitches[0] / stride_div;
>  		x_offset = x;
>  		y_offset = y;
> -		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
> -			((plane->state->src_w >> 16) - 1);
> +		plane_size = (src_h - 1) << 16 | (src_w - 1);
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
> 
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> -	I915_WRITE(PLANE_POS(pipe, 0), 0);
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
>  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> +
> +	if (scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(!dst_w || !dst_h);
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> +			crtc_state->scaler_state.scalers[scaler_id].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), (dst_x << 16) |
> dst_y);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) |
> dst_h);
> +		I915_WRITE(PLANE_POS(pipe, 0), 0);
> +	} else {
> +		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
> +	}
> +
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
> 
>  	POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -4338,6 +4411,7 @@ skl_update_scaler_users(
>  	int *scaler_id;
>  	struct drm_framebuffer *fb;
>  	struct intel_crtc_scaler_state *scaler_state;
> +	unsigned int rotation;
> 
>  	if (!intel_crtc || !crtc_state)
>  		return 0;
> @@ -4353,6 +4427,7 @@ skl_update_scaler_users(
>  		dst_w = drm_rect_width(&plane_state->dst);
>  		dst_h = drm_rect_height(&plane_state->dst);
>  		scaler_id = &plane_state->scaler_id;
> +		rotation = plane_state->base.rotation;
>  	} else {
>  		struct drm_display_mode *adjusted_mode =
>  			&crtc_state->base.adjusted_mode;
> @@ -4361,8 +4436,12 @@ skl_update_scaler_users(
>  		dst_w = adjusted_mode->hdisplay;
>  		dst_h = adjusted_mode->vdisplay;
>  		scaler_id = &scaler_state->scaler_id;
> +		rotation = DRM_ROTATE_0;
>  	}
> -	need_scaling = (src_w != dst_w || src_h != dst_h);
> +
> +	need_scaling = intel_rotation_90_or_270(rotation) ?
> +		(src_h != dst_w || src_w != dst_h):
> +		(src_w != dst_w || src_h != dst_h);
> 
>  	/*
>  	 * if plane is being disabled or scaler is no more required or force detach
> @@ -12805,6 +12884,36 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  	}
>  }
> 
> +int
> +skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
> +{
> +	int max_scale;
> +	struct drm_device *dev;
> +	struct drm_i915_private *dev_priv;
> +	int crtc_clock, cdclk;
> +
> +	if (!intel_crtc || !crtc_state)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	dev = intel_crtc->base.dev;
> +	dev_priv = dev->dev_private;
> +	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> +	cdclk = dev_priv->display.get_display_clock_speed(dev);
> +
> +	if (!crtc_clock || !cdclk)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	/*
> +	 * skl max scale is lower of:
> +	 *    close to 3 but not 3, -1 is for that purpose
> +	 *            or
> +	 *    cdclk/crtc_clock
> +	 */
> +	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> +
> +	return max_scale;
> +}
> +
>  static int
>  intel_check_primary_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
> @@ -12813,23 +12922,31 @@ intel_check_primary_plane(struct drm_plane
> *plane,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc = state->base.crtc;
>  	struct intel_crtc *intel_crtc;
> +	struct intel_crtc_state *crtc_state;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	struct drm_rect *dest = &state->dst;
>  	struct drm_rect *src = &state->src;
>  	const struct drm_rect *clip = &state->clip;
>  	bool can_position = false;
> +	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
> +	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>  	int ret;
> 
>  	crtc = crtc ? crtc : plane->crtc;
>  	intel_crtc = to_intel_crtc(crtc);
> +	crtc_state = state->base.state ?
> +		intel_atomic_get_crtc_state(state->base.state, intel_crtc) :
> NULL;
> 
> -	if (INTEL_INFO(dev)->gen >= 9)
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
>  		can_position = true;
> +	}
> 
>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    src, dest, clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> +					    min_scale,
> +					    max_scale,
>  					    can_position, true,
>  					    &state->visible);
>  	if (ret)
> @@ -12874,6 +12991,13 @@ intel_check_primary_plane(struct drm_plane
> *plane,
>  			intel_crtc->atomic.update_wm = true;
>  	}
> 
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_update_scaler_users(intel_crtc, crtc_state,
> +			to_intel_plane(plane), state, 0);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
> 
> @@ -13053,6 +13177,9 @@ static struct drm_plane
> *intel_primary_plane_create(struct drm_device *dev,
> 
>  	primary->can_scale = false;
>  	primary->max_downscale = 1;
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		primary->can_scale = true;
> +	}
>  	state->scaler_id = -1;
>  	primary->pipe = pipe;
>  	primary->plane = pipe;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 275a4ef..c9bc975 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -288,11 +288,11 @@ struct intel_initial_plane_config {
>  #define SKL_MIN_SRC_W 8
>  #define SKL_MAX_SRC_W 4096
>  #define SKL_MIN_SRC_H 8
> -#define SKL_MAX_SRC_H 2304
> +#define SKL_MAX_SRC_H 4096
>  #define SKL_MIN_DST_W 8
>  #define SKL_MAX_DST_W 4096
>  #define SKL_MIN_DST_H 8
> -#define SKL_MAX_DST_H 2304
> +#define SKL_MAX_DST_H 4096
> 
>  struct intel_scaler {
>  	int id;
> @@ -1131,9 +1131,13 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc);
>  int skl_update_scaler_users(struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
>  	struct intel_plane_state *plane_state, int force_detach);
> +int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
> 
>  unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>  				     struct drm_i915_gem_object *obj);
> +u32 skl_plane_ctl_format(uint32_t pixel_format);
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> +u32 skl_plane_ctl_rotation(unsigned int rotation);
> 
>  /* intel_dp.c */
>  void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 0cb3767..2f42777 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1150,6 +1150,15 @@ int intel_sprite_set_colorkey(struct drm_device
> *dev, void *data,
>  	}
> 
>  	intel_plane = to_intel_plane(plane);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* plane scaling and colorkey are mutually exclusive */
> +		if (to_intel_plane_state(plane->state)->scaler_id >= 0) {
> +			DRM_ERROR("colorkey not allowed with scaler\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	intel_plane->ckey = *set;
> 
>  	/*
> --
> 1.7.9.5

Daniel Vetter April 16, 2015, 7:36 a.m. UTC | #2

On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote:
> This patch enables skylake primary plane scaling using shared
> scalers atomic desgin.
> 
> v2:
> -use single copy of scaler limits (Matt)
> 
> v3:
> -move detach_scalers to crtc commit path (Matt)
> -use values in plane_state->src as regular integers (me)
> 
> v4:
> -changes to align with updated scaler structures (Matt, me)
> -keep plane src rect in 16.16 format (Matt, Daniel)
> 
> v5:
> -Rebased on top of 90/270 rotation changes (me)
> -Fixed an issue introduced by 90/270 changes where plane programming
>  is using drm_plane->state rect instead of intel_plane->state rect.
>  This change also required for scaling to work properly. (me)
> -With 90/270, updated limits to cover both portrait and landscape usages (me)
> -Refactored skylake_update_primary_plane to reduce its size (Daniel)
>  Added helper functions for refactoring are comprehended enough to be
>  used for skylake_update_plane (for sprite) too. One stop towards
>  having single function for all planes.

Please split out the refactoring into a prep patch since it's really hard
to review for functional changes when the code moves around. But when
that's split into a pure code-motion patch (which shouldn't have any
functional changes) and a subsequent patch to actually change the logic
then everything's a lot clearer. My BKM for splitting out prep patches is:

1. Create a new branch based on drm-intel-nightly.
2. Extract the first refactor step: You can either cherry-pick parts of
the patch or create it anew. Don't bother thinking about conflicts. Test
this patch to make sure you don't introduce a bisect issue.
3. Commit a revert of your refactor patch.
4. Cherry-pick the original patch, it should apply cleanly because the
baseline is unchanged due to the revert.
5. You now have 3 patches in your branch
	- refactor patch
	- Revert "refactor patch"
	- real patch you want to split up
Now do a rebase and squash together the Revert and the real patch.
-> First step is cleanly split out without ever having to deal with
conflicts or risking bisectability issues. Your branch has now 2 patches:
	- refactor patch
	- real patch with refactor parts taken out

Repeat until you've split up the patch into all the different parts.

Cheers, Daniel
> 
> Signed-off-by: Chandra Konduru <[e-mail]>
> Reviewed-by: Matt Roper <[e-mail]>
> Testcase: kms_plane_scaling
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |  261 +++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h     |    8 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++
>  4 files changed, 213 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 3c4b7cd..cb6d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev,
>  		plane->state->state = NULL;
>  	}
>  
> -	/* swap crtc_state */
> +	/* swap crtc_scaler_state */
>  	for (i = 0; i < dev->mode_config.num_crtc; i++) {
>  		struct drm_crtc *crtc = state->crtcs[i];
>  		if (!crtc) {
> @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev,
>  
>  		to_intel_crtc(crtc)->config->scaler_state =
>  			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> +
> +		if (INTEL_INFO(dev)->gen >= 9)
> +			skl_detach_scalers(to_intel_crtc(crtc));
>  	}
>  
>  	drm_atomic_helper_commit_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5f7a40f..7f3ae8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc)
>  	}
>  }
>  
> -static void skylake_update_primary_plane(struct drm_crtc *crtc,
> -					 struct drm_framebuffer *fb,
> -					 int x, int y)
> +u32 skl_plane_ctl_format(uint32_t pixel_format)
>  {
> -	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_i915_gem_object *obj;
> -	int pipe = intel_crtc->pipe;
> -	u32 plane_ctl, stride_div, stride;
> -	u32 tile_height, plane_offset, plane_size;
> -	unsigned int rotation;
> -	int x_offset, y_offset;
> -	unsigned long surf_addr;
> -	struct drm_plane *plane;
> -
> -	if (!intel_crtc->primary_enabled) {
> -		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -		POSTING_READ(PLANE_CTL(pipe, 0));
> -		return;
> -	}
> -
> -	plane_ctl = PLANE_CTL_ENABLE |
> -		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> -		    PLANE_CTL_PIPE_CSC_ENABLE;
> -
> -	switch (fb->pixel_format) {
> +	u32 plane_ctl_format = 0;
> +	switch (pixel_format) {
>  	case DRM_FORMAT_RGB565:
> -		plane_ctl |= PLANE_CTL_FORMAT_RGB_565;
> -		break;
> -	case DRM_FORMAT_XRGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		break;
> -	case DRM_FORMAT_ARGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_RGB_565;
>  		break;
>  	case DRM_FORMAT_XBGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
>  		break;
> +	case DRM_FORMAT_XRGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888;
> +		break;
> +	/*
> +	 * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> +	 * to be already pre-multiplied. We need to add a knob (or a different
> +	 * DRM_FORMAT) for user-space to configure that.
> +	 */
>  	case DRM_FORMAT_ABGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>  		break;
>  	case DRM_FORMAT_XRGB2101010:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_2101010;
>  		break;
>  	case DRM_FORMAT_XBGR2101010:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
> +		break;
> +	case DRM_FORMAT_YUYV:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
> +		break;
> +	case DRM_FORMAT_YVYU:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YVYU;
> +		break;
> +	case DRM_FORMAT_UYVY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
> +		break;
> +	case DRM_FORMAT_VYUY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
>  		break;
>  	default:
>  		BUG();
>  	}
> +	return plane_ctl_format;
> +}
>  
> -	switch (fb->modifier[0]) {
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> +{
> +	u32 plane_ctl_tiling = 0;
> +	switch (fb_modifier) {
>  	case DRM_FORMAT_MOD_NONE:
>  		break;
>  	case I915_FORMAT_MOD_X_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_X;
> +		plane_ctl_tiling = PLANE_CTL_TILED_X;
>  		break;
>  	case I915_FORMAT_MOD_Y_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_Y;
> +		plane_ctl_tiling = PLANE_CTL_TILED_Y;
>  		break;
>  	case I915_FORMAT_MOD_Yf_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_YF;
> +		plane_ctl_tiling = PLANE_CTL_TILED_YF;
>  		break;
>  	default:
> -		MISSING_CASE(fb->modifier[0]);
> +		MISSING_CASE(fb_modifier);
>  	}
> +	return plane_ctl_tiling;
> +}
>  
> -	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -
> -	plane = crtc->primary;
> -	rotation = plane->state->rotation;
> +u32 skl_plane_ctl_rotation(unsigned int rotation)
> +{
> +	u32 plane_ctl_rotation = 0;
>  	switch (rotation) {
> +	case BIT(DRM_ROTATE_0):
> +		break;
>  	case BIT(DRM_ROTATE_90):
> -		plane_ctl |= PLANE_CTL_ROTATE_90;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_90;
>  		break;
> -
>  	case BIT(DRM_ROTATE_180):
> -		plane_ctl |= PLANE_CTL_ROTATE_180;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_180;
>  		break;
> -
>  	case BIT(DRM_ROTATE_270):
> -		plane_ctl |= PLANE_CTL_ROTATE_270;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_270;
>  		break;
> +	default:
> +		MISSING_CASE(rotation);
>  	}
>  
> +	return plane_ctl_rotation;
> +}
> +
> +static void skylake_update_primary_plane(struct drm_crtc *crtc,
> +					 struct drm_framebuffer *fb,
> +					 int x, int y)
> +{
> +	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_i915_gem_object *obj;
> +	int pipe = intel_crtc->pipe;
> +	u32 plane_ctl, stride_div, stride;
> +	u32 tile_height, plane_offset, plane_size;
> +	unsigned int rotation;
> +	int x_offset, y_offset;
> +	unsigned long surf_addr;
> +	struct drm_plane *plane;
> +	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 = crtc->primary;
> +	plane_state = plane ? to_intel_plane_state(crtc->primary->state) : NULL;
> +
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> +		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> +		POSTING_READ(PLANE_CTL(pipe, 0));
> +		return;
> +	}
> +
> +	plane_ctl = PLANE_CTL_ENABLE |
> +		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> +		    PLANE_CTL_PIPE_CSC_ENABLE;
> +
> +	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);
>  
> +	if (plane_state && drm_rect_width(&plane_state->src)) {
> +		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);
> +	} else {
> +		src_w = intel_crtc->config->pipe_src_w;
> +		src_h = intel_crtc->config->pipe_src_h;
> +	}
> +
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
>  						fb->modifier[0]);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
> -		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
> +		x_offset = stride * tile_height - y - src_h;
>  		y_offset = x;
> -		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
> -					((plane->state->src_h >> 16) - 1);
> +		plane_size = (src_w - 1) << 16 | (src_h - 1);
>  	} else {
>  		stride = fb->pitches[0] / stride_div;
>  		x_offset = x;
>  		y_offset = y;
> -		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
> -			((plane->state->src_w >> 16) - 1);
> +		plane_size = (src_h - 1) << 16 | (src_w - 1);
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
>  
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> -	I915_WRITE(PLANE_POS(pipe, 0), 0);
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
>  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> +
> +	if (scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(!dst_w || !dst_h);
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> +			crtc_state->scaler_state.scalers[scaler_id].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), (dst_x << 16) | dst_y);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
> +		I915_WRITE(PLANE_POS(pipe, 0), 0);
> +	} else {
> +		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
> +	}
> +
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
>  	POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -4338,6 +4411,7 @@ skl_update_scaler_users(
>  	int *scaler_id;
>  	struct drm_framebuffer *fb;
>  	struct intel_crtc_scaler_state *scaler_state;
> +	unsigned int rotation;
>  
>  	if (!intel_crtc || !crtc_state)
>  		return 0;
> @@ -4353,6 +4427,7 @@ skl_update_scaler_users(
>  		dst_w = drm_rect_width(&plane_state->dst);
>  		dst_h = drm_rect_height(&plane_state->dst);
>  		scaler_id = &plane_state->scaler_id;
> +		rotation = plane_state->base.rotation;
>  	} else {
>  		struct drm_display_mode *adjusted_mode =
>  			&crtc_state->base.adjusted_mode;
> @@ -4361,8 +4436,12 @@ skl_update_scaler_users(
>  		dst_w = adjusted_mode->hdisplay;
>  		dst_h = adjusted_mode->vdisplay;
>  		scaler_id = &scaler_state->scaler_id;
> +		rotation = DRM_ROTATE_0;
>  	}
> -	need_scaling = (src_w != dst_w || src_h != dst_h);
> +
> +	need_scaling = intel_rotation_90_or_270(rotation) ?
> +		(src_h != dst_w || src_w != dst_h):
> +		(src_w != dst_w || src_h != dst_h);
>  
>  	/*
>  	 * if plane is being disabled or scaler is no more required or force detach
> @@ -12805,6 +12884,36 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  	}
>  }
>  
> +int
> +skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
> +{
> +	int max_scale;
> +	struct drm_device *dev;
> +	struct drm_i915_private *dev_priv;
> +	int crtc_clock, cdclk;
> +
> +	if (!intel_crtc || !crtc_state)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	dev = intel_crtc->base.dev;
> +	dev_priv = dev->dev_private;
> +	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> +	cdclk = dev_priv->display.get_display_clock_speed(dev);
> +
> +	if (!crtc_clock || !cdclk)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	/*
> +	 * skl max scale is lower of:
> +	 *    close to 3 but not 3, -1 is for that purpose
> +	 *            or
> +	 *    cdclk/crtc_clock
> +	 */
> +	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> +
> +	return max_scale;
> +}
> +
>  static int
>  intel_check_primary_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
> @@ -12813,23 +12922,31 @@ intel_check_primary_plane(struct drm_plane *plane,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc = state->base.crtc;
>  	struct intel_crtc *intel_crtc;
> +	struct intel_crtc_state *crtc_state;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	struct drm_rect *dest = &state->dst;
>  	struct drm_rect *src = &state->src;
>  	const struct drm_rect *clip = &state->clip;
>  	bool can_position = false;
> +	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
> +	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>  	int ret;
>  
>  	crtc = crtc ? crtc : plane->crtc;
>  	intel_crtc = to_intel_crtc(crtc);
> +	crtc_state = state->base.state ?
> +		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
>  
> -	if (INTEL_INFO(dev)->gen >= 9)
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
>  		can_position = true;
> +	}
>  
>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    src, dest, clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> +					    min_scale,
> +					    max_scale,
>  					    can_position, true,
>  					    &state->visible);
>  	if (ret)
> @@ -12874,6 +12991,13 @@ intel_check_primary_plane(struct drm_plane *plane,
>  			intel_crtc->atomic.update_wm = true;
>  	}
>  
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_update_scaler_users(intel_crtc, crtc_state,
> +			to_intel_plane(plane), state, 0);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -13053,6 +13177,9 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  
>  	primary->can_scale = false;
>  	primary->max_downscale = 1;
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		primary->can_scale = true;
> +	}
>  	state->scaler_id = -1;
>  	primary->pipe = pipe;
>  	primary->plane = pipe;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 275a4ef..c9bc975 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -288,11 +288,11 @@ struct intel_initial_plane_config {
>  #define SKL_MIN_SRC_W 8
>  #define SKL_MAX_SRC_W 4096
>  #define SKL_MIN_SRC_H 8
> -#define SKL_MAX_SRC_H 2304
> +#define SKL_MAX_SRC_H 4096
>  #define SKL_MIN_DST_W 8
>  #define SKL_MAX_DST_W 4096
>  #define SKL_MIN_DST_H 8
> -#define SKL_MAX_DST_H 2304
> +#define SKL_MAX_DST_H 4096
>  
>  struct intel_scaler {
>  	int id;
> @@ -1131,9 +1131,13 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc);
>  int skl_update_scaler_users(struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
>  	struct intel_plane_state *plane_state, int force_detach);
> +int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>  
>  unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>  				     struct drm_i915_gem_object *obj);
> +u32 skl_plane_ctl_format(uint32_t pixel_format);
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> +u32 skl_plane_ctl_rotation(unsigned int rotation);
>  
>  /* intel_dp.c */
>  void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0cb3767..2f42777 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1150,6 +1150,15 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  	}
>  
>  	intel_plane = to_intel_plane(plane);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* plane scaling and colorkey are mutually exclusive */
> +		if (to_intel_plane_state(plane->state)->scaler_id >= 0) {
> +			DRM_ERROR("colorkey not allowed with scaler\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	intel_plane->ckey = *set;
>  
>  	/*
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[e-mail] April 16, 2015, 8:17 a.m. UTC | #3

For v5:

Reviewed-by: Sonika Jindal <[e-mail]> (v5)

Regards,
Sonika

On 4/16/2015 3:44 AM, Chandra Konduru wrote:
> This patch enables skylake primary plane scaling using shared
> scalers atomic desgin.
>
> v2:
> -use single copy of scaler limits (Matt)
>
> v3:
> -move detach_scalers to crtc commit path (Matt)
> -use values in plane_state->src as regular integers (me)
>
> v4:
> -changes to align with updated scaler structures (Matt, me)
> -keep plane src rect in 16.16 format (Matt, Daniel)
>
> v5:
> -Rebased on top of 90/270 rotation changes (me)
> -Fixed an issue introduced by 90/270 changes where plane programming
>   is using drm_plane->state rect instead of intel_plane->state rect.
>   This change also required for scaling to work properly. (me)
> -With 90/270, updated limits to cover both portrait and landscape usages (me)
> -Refactored skylake_update_primary_plane to reduce its size (Daniel)
>   Added helper functions for refactoring are comprehended enough to be
>   used for skylake_update_plane (for sprite) too. One stop towards
>   having single function for all planes.
>
> Signed-off-by: Chandra Konduru <[e-mail]>
> Reviewed-by: Matt Roper <[e-mail]>
> Testcase: kms_plane_scaling
> ---
>   drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>   drivers/gpu/drm/i915/intel_display.c |  261 +++++++++++++++++++++++++---------
>   drivers/gpu/drm/i915/intel_drv.h     |    8 +-
>   drivers/gpu/drm/i915/intel_sprite.c  |    9 ++
>   4 files changed, 213 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 3c4b7cd..cb6d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev,
>   		plane->state->state = NULL;
>   	}
>
> -	/* swap crtc_state */
> +	/* swap crtc_scaler_state */
>   	for (i = 0; i < dev->mode_config.num_crtc; i++) {
>   		struct drm_crtc *crtc = state->crtcs[i];
>   		if (!crtc) {
> @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev,
>
>   		to_intel_crtc(crtc)->config->scaler_state =
>   			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> +
> +		if (INTEL_INFO(dev)->gen >= 9)
> +			skl_detach_scalers(to_intel_crtc(crtc));
>   	}
>
>   	drm_atomic_helper_commit_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5f7a40f..7f3ae8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc)
>   	}
>   }
>
> -static void skylake_update_primary_plane(struct drm_crtc *crtc,
> -					 struct drm_framebuffer *fb,
> -					 int x, int y)
> +u32 skl_plane_ctl_format(uint32_t pixel_format)
>   {
> -	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_i915_gem_object *obj;
> -	int pipe = intel_crtc->pipe;
> -	u32 plane_ctl, stride_div, stride;
> -	u32 tile_height, plane_offset, plane_size;
> -	unsigned int rotation;
> -	int x_offset, y_offset;
> -	unsigned long surf_addr;
> -	struct drm_plane *plane;
> -
> -	if (!intel_crtc->primary_enabled) {
> -		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -		POSTING_READ(PLANE_CTL(pipe, 0));
> -		return;
> -	}
> -
> -	plane_ctl = PLANE_CTL_ENABLE |
> -		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> -		    PLANE_CTL_PIPE_CSC_ENABLE;
> -
> -	switch (fb->pixel_format) {
> +	u32 plane_ctl_format = 0;
> +	switch (pixel_format) {
>   	case DRM_FORMAT_RGB565:
> -		plane_ctl |= PLANE_CTL_FORMAT_RGB_565;
> -		break;
> -	case DRM_FORMAT_XRGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		break;
> -	case DRM_FORMAT_ARGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_RGB_565;
>   		break;
>   	case DRM_FORMAT_XBGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
>   		break;
> +	case DRM_FORMAT_XRGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888;
> +		break;
> +	/*
> +	 * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> +	 * to be already pre-multiplied. We need to add a knob (or a different
> +	 * DRM_FORMAT) for user-space to configure that.
> +	 */
>   	case DRM_FORMAT_ABGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>   		break;
>   	case DRM_FORMAT_XRGB2101010:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_2101010;
>   		break;
>   	case DRM_FORMAT_XBGR2101010:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
> +		break;
> +	case DRM_FORMAT_YUYV:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
> +		break;
> +	case DRM_FORMAT_YVYU:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YVYU;
> +		break;
> +	case DRM_FORMAT_UYVY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
> +		break;
> +	case DRM_FORMAT_VYUY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
>   		break;
>   	default:
>   		BUG();
>   	}
> +	return plane_ctl_format;
> +}
>
> -	switch (fb->modifier[0]) {
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> +{
> +	u32 plane_ctl_tiling = 0;
> +	switch (fb_modifier) {
>   	case DRM_FORMAT_MOD_NONE:
>   		break;
>   	case I915_FORMAT_MOD_X_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_X;
> +		plane_ctl_tiling = PLANE_CTL_TILED_X;
>   		break;
>   	case I915_FORMAT_MOD_Y_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_Y;
> +		plane_ctl_tiling = PLANE_CTL_TILED_Y;
>   		break;
>   	case I915_FORMAT_MOD_Yf_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_YF;
> +		plane_ctl_tiling = PLANE_CTL_TILED_YF;
>   		break;
>   	default:
> -		MISSING_CASE(fb->modifier[0]);
> +		MISSING_CASE(fb_modifier);
>   	}
> +	return plane_ctl_tiling;
> +}
>
> -	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -
> -	plane = crtc->primary;
> -	rotation = plane->state->rotation;
> +u32 skl_plane_ctl_rotation(unsigned int rotation)
> +{
> +	u32 plane_ctl_rotation = 0;
>   	switch (rotation) {
> +	case BIT(DRM_ROTATE_0):
> +		break;
>   	case BIT(DRM_ROTATE_90):
> -		plane_ctl |= PLANE_CTL_ROTATE_90;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_90;
>   		break;
> -
>   	case BIT(DRM_ROTATE_180):
> -		plane_ctl |= PLANE_CTL_ROTATE_180;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_180;
>   		break;
> -
>   	case BIT(DRM_ROTATE_270):
> -		plane_ctl |= PLANE_CTL_ROTATE_270;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_270;
>   		break;
> +	default:
> +		MISSING_CASE(rotation);
>   	}
>
> +	return plane_ctl_rotation;
> +}
> +
> +static void skylake_update_primary_plane(struct drm_crtc *crtc,
> +					 struct drm_framebuffer *fb,
> +					 int x, int y)
> +{
> +	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_i915_gem_object *obj;
> +	int pipe = intel_crtc->pipe;
> +	u32 plane_ctl, stride_div, stride;
> +	u32 tile_height, plane_offset, plane_size;
> +	unsigned int rotation;
> +	int x_offset, y_offset;
> +	unsigned long surf_addr;
> +	struct drm_plane *plane;
> +	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 = crtc->primary;
> +	plane_state = plane ? to_intel_plane_state(crtc->primary->state) : NULL;
> +
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> +		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> +		POSTING_READ(PLANE_CTL(pipe, 0));
> +		return;
> +	}
> +
> +	plane_ctl = PLANE_CTL_ENABLE |
> +		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> +		    PLANE_CTL_PIPE_CSC_ENABLE;
> +
> +	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);
>
> +	if (plane_state && drm_rect_width(&plane_state->src)) {
> +		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);
> +	} else {
> +		src_w = intel_crtc->config->pipe_src_w;
> +		src_h = intel_crtc->config->pipe_src_h;
> +	}
> +
>   	if (intel_rotation_90_or_270(rotation)) {
>   		/* stride = Surface height in tiles */
>   		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
>   						fb->modifier[0]);
>   		stride = DIV_ROUND_UP(fb->height, tile_height);
> -		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
> +		x_offset = stride * tile_height - y - src_h;
>   		y_offset = x;
> -		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
> -					((plane->state->src_h >> 16) - 1);
> +		plane_size = (src_w - 1) << 16 | (src_h - 1);
>   	} else {
>   		stride = fb->pitches[0] / stride_div;
>   		x_offset = x;
>   		y_offset = y;
> -		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
> -			((plane->state->src_w >> 16) - 1);
> +		plane_size = (src_h - 1) << 16 | (src_w - 1);
>   	}
>   	plane_offset = y_offset << 16 | x_offset;
>
>   	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> -	I915_WRITE(PLANE_POS(pipe, 0), 0);
>   	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
>   	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
>   	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> +
> +	if (scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(!dst_w || !dst_h);
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> +			crtc_state->scaler_state.scalers[scaler_id].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), (dst_x << 16) | dst_y);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
> +		I915_WRITE(PLANE_POS(pipe, 0), 0);
> +	} else {
> +		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
> +	}
> +
>   	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>
>   	POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -4338,6 +4411,7 @@ skl_update_scaler_users(
>   	int *scaler_id;
>   	struct drm_framebuffer *fb;
>   	struct intel_crtc_scaler_state *scaler_state;
> +	unsigned int rotation;
>
>   	if (!intel_crtc || !crtc_state)
>   		return 0;
> @@ -4353,6 +4427,7 @@ skl_update_scaler_users(
>   		dst_w = drm_rect_width(&plane_state->dst);
>   		dst_h = drm_rect_height(&plane_state->dst);
>   		scaler_id = &plane_state->scaler_id;
> +		rotation = plane_state->base.rotation;
>   	} else {
>   		struct drm_display_mode *adjusted_mode =
>   			&crtc_state->base.adjusted_mode;
> @@ -4361,8 +4436,12 @@ skl_update_scaler_users(
>   		dst_w = adjusted_mode->hdisplay;
>   		dst_h = adjusted_mode->vdisplay;
>   		scaler_id = &scaler_state->scaler_id;
> +		rotation = DRM_ROTATE_0;
>   	}
> -	need_scaling = (src_w != dst_w || src_h != dst_h);
> +
> +	need_scaling = intel_rotation_90_or_270(rotation) ?
> +		(src_h != dst_w || src_w != dst_h):
> +		(src_w != dst_w || src_h != dst_h);
>
>   	/*
>   	 * if plane is being disabled or scaler is no more required or force detach
> @@ -12805,6 +12884,36 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>   	}
>   }
>
> +int
> +skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
> +{
> +	int max_scale;
> +	struct drm_device *dev;
> +	struct drm_i915_private *dev_priv;
> +	int crtc_clock, cdclk;
> +
> +	if (!intel_crtc || !crtc_state)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	dev = intel_crtc->base.dev;
> +	dev_priv = dev->dev_private;
> +	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> +	cdclk = dev_priv->display.get_display_clock_speed(dev);
> +
> +	if (!crtc_clock || !cdclk)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	/*
> +	 * skl max scale is lower of:
> +	 *    close to 3 but not 3, -1 is for that purpose
> +	 *            or
> +	 *    cdclk/crtc_clock
> +	 */
> +	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> +
> +	return max_scale;
> +}
> +
>   static int
>   intel_check_primary_plane(struct drm_plane *plane,
>   			  struct intel_plane_state *state)
> @@ -12813,23 +12922,31 @@ intel_check_primary_plane(struct drm_plane *plane,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_crtc *crtc = state->base.crtc;
>   	struct intel_crtc *intel_crtc;
> +	struct intel_crtc_state *crtc_state;
>   	struct drm_framebuffer *fb = state->base.fb;
>   	struct drm_rect *dest = &state->dst;
>   	struct drm_rect *src = &state->src;
>   	const struct drm_rect *clip = &state->clip;
>   	bool can_position = false;
> +	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
> +	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>   	int ret;
>
>   	crtc = crtc ? crtc : plane->crtc;
>   	intel_crtc = to_intel_crtc(crtc);
> +	crtc_state = state->base.state ?
> +		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
>
> -	if (INTEL_INFO(dev)->gen >= 9)
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
>   		can_position = true;
> +	}
>
>   	ret = drm_plane_helper_check_update(plane, crtc, fb,
>   					    src, dest, clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> +					    min_scale,
> +					    max_scale,
>   					    can_position, true,
>   					    &state->visible);
>   	if (ret)
> @@ -12874,6 +12991,13 @@ intel_check_primary_plane(struct drm_plane *plane,
>   			intel_crtc->atomic.update_wm = true;
>   	}
>
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_update_scaler_users(intel_crtc, crtc_state,
> +			to_intel_plane(plane), state, 0);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	return 0;
>   }
>
> @@ -13053,6 +13177,9 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>
>   	primary->can_scale = false;
>   	primary->max_downscale = 1;
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		primary->can_scale = true;
> +	}
>   	state->scaler_id = -1;
>   	primary->pipe = pipe;
>   	primary->plane = pipe;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 275a4ef..c9bc975 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -288,11 +288,11 @@ struct intel_initial_plane_config {
>   #define SKL_MIN_SRC_W 8
>   #define SKL_MAX_SRC_W 4096
>   #define SKL_MIN_SRC_H 8
> -#define SKL_MAX_SRC_H 2304
> +#define SKL_MAX_SRC_H 4096
>   #define SKL_MIN_DST_W 8
>   #define SKL_MAX_DST_W 4096
>   #define SKL_MIN_DST_H 8
> -#define SKL_MAX_DST_H 2304
> +#define SKL_MAX_DST_H 4096
>
>   struct intel_scaler {
>   	int id;
> @@ -1131,9 +1131,13 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc);
>   int skl_update_scaler_users(struct intel_crtc *intel_crtc,
>   	struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
>   	struct intel_plane_state *plane_state, int force_detach);
> +int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>
>   unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>   				     struct drm_i915_gem_object *obj);
> +u32 skl_plane_ctl_format(uint32_t pixel_format);
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> +u32 skl_plane_ctl_rotation(unsigned int rotation);
>
>   /* intel_dp.c */
>   void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0cb3767..2f42777 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1150,6 +1150,15 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>   	}
>
>   	intel_plane = to_intel_plane(plane);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* plane scaling and colorkey are mutually exclusive */
> +		if (to_intel_plane_state(plane->state)->scaler_id >= 0) {
> +			DRM_ERROR("colorkey not allowed with scaler\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>   	intel_plane->ckey = *set;
>
>   	/*
>

Daniel Vetter April 23, 2015, 8:20 p.m. UTC | #4

On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote:
> This patch enables skylake primary plane scaling using shared
> scalers atomic desgin.
> 
> v2:
> -use single copy of scaler limits (Matt)
> 
> v3:
> -move detach_scalers to crtc commit path (Matt)
> -use values in plane_state->src as regular integers (me)
> 
> v4:
> -changes to align with updated scaler structures (Matt, me)
> -keep plane src rect in 16.16 format (Matt, Daniel)
> 
> v5:
> -Rebased on top of 90/270 rotation changes (me)
> -Fixed an issue introduced by 90/270 changes where plane programming
>  is using drm_plane->state rect instead of intel_plane->state rect.
>  This change also required for scaling to work properly. (me)
> -With 90/270, updated limits to cover both portrait and landscape usages (me)
> -Refactored skylake_update_primary_plane to reduce its size (Daniel)
>  Added helper functions for refactoring are comprehended enough to be
>  used for skylake_update_plane (for sprite) too. One stop towards
>  having single function for all planes.
> 
> Signed-off-by: Chandra Konduru <[e-mail]>
> Reviewed-by: Matt Roper <[e-mail]>
> Testcase: kms_plane_scaling

I wanted to pull this in but then spotted an issue. Since this needs one
more round can you perhaps use the older version as a baseline again
(without the refactoring)? Since skl planes aren't converted to universal
planes yet it might be better to wait with refactoring until that's done
actually. Two more comments below.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |  261 +++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h     |    8 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++
>  4 files changed, 213 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 3c4b7cd..cb6d5f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev,
>  		plane->state->state = NULL;
>  	}
>  
> -	/* swap crtc_state */
> +	/* swap crtc_scaler_state */
>  	for (i = 0; i < dev->mode_config.num_crtc; i++) {
>  		struct drm_crtc *crtc = state->crtcs[i];
>  		if (!crtc) {
> @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev,
>  
>  		to_intel_crtc(crtc)->config->scaler_state =
>  			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> +
> +		if (INTEL_INFO(dev)->gen >= 9)
> +			skl_detach_scalers(to_intel_crtc(crtc));
>  	}
>  
>  	drm_atomic_helper_commit_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5f7a40f..7f3ae8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc)
>  	}
>  }
>  
> -static void skylake_update_primary_plane(struct drm_crtc *crtc,
> -					 struct drm_framebuffer *fb,
> -					 int x, int y)
> +u32 skl_plane_ctl_format(uint32_t pixel_format)
>  {
> -	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_i915_gem_object *obj;
> -	int pipe = intel_crtc->pipe;
> -	u32 plane_ctl, stride_div, stride;
> -	u32 tile_height, plane_offset, plane_size;
> -	unsigned int rotation;
> -	int x_offset, y_offset;
> -	unsigned long surf_addr;
> -	struct drm_plane *plane;
> -
> -	if (!intel_crtc->primary_enabled) {
> -		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -		POSTING_READ(PLANE_CTL(pipe, 0));
> -		return;
> -	}
> -
> -	plane_ctl = PLANE_CTL_ENABLE |
> -		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> -		    PLANE_CTL_PIPE_CSC_ENABLE;
> -
> -	switch (fb->pixel_format) {
> +	u32 plane_ctl_format = 0;
> +	switch (pixel_format) {
>  	case DRM_FORMAT_RGB565:
> -		plane_ctl |= PLANE_CTL_FORMAT_RGB_565;
> -		break;
> -	case DRM_FORMAT_XRGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		break;
> -	case DRM_FORMAT_ARGB8888:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_RGB_565;
>  		break;
>  	case DRM_FORMAT_XBGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
>  		break;
> +	case DRM_FORMAT_XRGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888;
> +		break;
> +	/*
> +	 * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> +	 * to be already pre-multiplied. We need to add a knob (or a different
> +	 * DRM_FORMAT) for user-space to configure that.
> +	 */
>  	case DRM_FORMAT_ABGR8888:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
> -		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> +		break;
> +	case DRM_FORMAT_ARGB8888:
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
> +			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
>  		break;
>  	case DRM_FORMAT_XRGB2101010:
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_2101010;
>  		break;
>  	case DRM_FORMAT_XBGR2101010:
> -		plane_ctl |= PLANE_CTL_ORDER_RGBX;
> -		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
> +		plane_ctl_format = PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
> +		break;
> +	case DRM_FORMAT_YUYV:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
> +		break;
> +	case DRM_FORMAT_YVYU:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YVYU;
> +		break;
> +	case DRM_FORMAT_UYVY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
> +		break;
> +	case DRM_FORMAT_VYUY:
> +		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
>  		break;
>  	default:
>  		BUG();
>  	}
> +	return plane_ctl_format;
> +}
>  
> -	switch (fb->modifier[0]) {
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> +{
> +	u32 plane_ctl_tiling = 0;
> +	switch (fb_modifier) {
>  	case DRM_FORMAT_MOD_NONE:
>  		break;
>  	case I915_FORMAT_MOD_X_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_X;
> +		plane_ctl_tiling = PLANE_CTL_TILED_X;
>  		break;
>  	case I915_FORMAT_MOD_Y_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_Y;
> +		plane_ctl_tiling = PLANE_CTL_TILED_Y;
>  		break;
>  	case I915_FORMAT_MOD_Yf_TILED:
> -		plane_ctl |= PLANE_CTL_TILED_YF;
> +		plane_ctl_tiling = PLANE_CTL_TILED_YF;
>  		break;
>  	default:
> -		MISSING_CASE(fb->modifier[0]);
> +		MISSING_CASE(fb_modifier);
>  	}
> +	return plane_ctl_tiling;
> +}
>  
> -	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> -
> -	plane = crtc->primary;
> -	rotation = plane->state->rotation;
> +u32 skl_plane_ctl_rotation(unsigned int rotation)
> +{
> +	u32 plane_ctl_rotation = 0;
>  	switch (rotation) {
> +	case BIT(DRM_ROTATE_0):
> +		break;
>  	case BIT(DRM_ROTATE_90):
> -		plane_ctl |= PLANE_CTL_ROTATE_90;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_90;
>  		break;
> -
>  	case BIT(DRM_ROTATE_180):
> -		plane_ctl |= PLANE_CTL_ROTATE_180;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_180;
>  		break;
> -
>  	case BIT(DRM_ROTATE_270):
> -		plane_ctl |= PLANE_CTL_ROTATE_270;
> +		plane_ctl_rotation = PLANE_CTL_ROTATE_270;
>  		break;
> +	default:
> +		MISSING_CASE(rotation);
>  	}
>  
> +	return plane_ctl_rotation;
> +}
> +
> +static void skylake_update_primary_plane(struct drm_crtc *crtc,
> +					 struct drm_framebuffer *fb,
> +					 int x, int y)
> +{
> +	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_i915_gem_object *obj;
> +	int pipe = intel_crtc->pipe;
> +	u32 plane_ctl, stride_div, stride;
> +	u32 tile_height, plane_offset, plane_size;
> +	unsigned int rotation;
> +	int x_offset, y_offset;
> +	unsigned long surf_addr;
> +	struct drm_plane *plane;
> +	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 = crtc->primary;
> +	plane_state = plane ? to_intel_plane_state(crtc->primary->state) : NULL;
> +
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> +		I915_WRITE(PLANE_SURF(pipe, 0), 0);
> +		POSTING_READ(PLANE_CTL(pipe, 0));
> +		return;
> +	}
> +
> +	plane_ctl = PLANE_CTL_ENABLE |
> +		    PLANE_CTL_PIPE_GAMMA_ENABLE |
> +		    PLANE_CTL_PIPE_CSC_ENABLE;
> +
> +	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);
>  
> +	if (plane_state && drm_rect_width(&plane_state->src)) {

We discussed this a bit at osts, but can you please explain in a FIXME
comment why this is needed?

> +		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);
> +	} else {
> +		src_w = intel_crtc->config->pipe_src_w;
> +		src_h = intel_crtc->config->pipe_src_h;
> +	}
> +
>  	if (intel_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
>  						fb->modifier[0]);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
> -		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
> +		x_offset = stride * tile_height - y - src_h;
>  		y_offset = x;
> -		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
> -					((plane->state->src_h >> 16) - 1);
> +		plane_size = (src_w - 1) << 16 | (src_h - 1);
>  	} else {
>  		stride = fb->pitches[0] / stride_div;
>  		x_offset = x;
>  		y_offset = y;
> -		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
> -			((plane->state->src_w >> 16) - 1);
> +		plane_size = (src_h - 1) << 16 | (src_w - 1);
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
>  
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> -	I915_WRITE(PLANE_POS(pipe, 0), 0);
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
>  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> +
> +	if (scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(!dst_w || !dst_h);
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> +			crtc_state->scaler_state.scalers[scaler_id].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), (dst_x << 16) | dst_y);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
> +		I915_WRITE(PLANE_POS(pipe, 0), 0);
> +	} else {
> +		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
> +	}
> +
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
>  	POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -4338,6 +4411,7 @@ skl_update_scaler_users(
>  	int *scaler_id;
>  	struct drm_framebuffer *fb;
>  	struct intel_crtc_scaler_state *scaler_state;
> +	unsigned int rotation;
>  
>  	if (!intel_crtc || !crtc_state)
>  		return 0;
> @@ -4353,6 +4427,7 @@ skl_update_scaler_users(
>  		dst_w = drm_rect_width(&plane_state->dst);
>  		dst_h = drm_rect_height(&plane_state->dst);
>  		scaler_id = &plane_state->scaler_id;
> +		rotation = plane_state->base.rotation;
>  	} else {
>  		struct drm_display_mode *adjusted_mode =
>  			&crtc_state->base.adjusted_mode;
> @@ -4361,8 +4436,12 @@ skl_update_scaler_users(
>  		dst_w = adjusted_mode->hdisplay;
>  		dst_h = adjusted_mode->vdisplay;
>  		scaler_id = &scaler_state->scaler_id;
> +		rotation = DRM_ROTATE_0;
>  	}
> -	need_scaling = (src_w != dst_w || src_h != dst_h);
> +
> +	need_scaling = intel_rotation_90_or_270(rotation) ?
> +		(src_h != dst_w || src_w != dst_h):
> +		(src_w != dst_w || src_h != dst_h);
>  
>  	/*
>  	 * if plane is being disabled or scaler is no more required or force detach
> @@ -12805,6 +12884,36 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  	}
>  }
>  
> +int
> +skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
> +{
> +	int max_scale;
> +	struct drm_device *dev;
> +	struct drm_i915_private *dev_priv;
> +	int crtc_clock, cdclk;
> +
> +	if (!intel_crtc || !crtc_state)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	dev = intel_crtc->base.dev;
> +	dev_priv = dev->dev_private;
> +	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> +	cdclk = dev_priv->display.get_display_clock_speed(dev);
> +
> +	if (!crtc_clock || !cdclk)
> +		return DRM_PLANE_HELPER_NO_SCALING;
> +
> +	/*
> +	 * skl max scale is lower of:
> +	 *    close to 3 but not 3, -1 is for that purpose
> +	 *            or
> +	 *    cdclk/crtc_clock
> +	 */
> +	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> +
> +	return max_scale;
> +}
> +
>  static int
>  intel_check_primary_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
> @@ -12813,23 +12922,31 @@ intel_check_primary_plane(struct drm_plane *plane,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc = state->base.crtc;
>  	struct intel_crtc *intel_crtc;
> +	struct intel_crtc_state *crtc_state;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	struct drm_rect *dest = &state->dst;
>  	struct drm_rect *src = &state->src;
>  	const struct drm_rect *clip = &state->clip;
>  	bool can_position = false;
> +	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
> +	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>  	int ret;
>  
>  	crtc = crtc ? crtc : plane->crtc;
>  	intel_crtc = to_intel_crtc(crtc);
> +	crtc_state = state->base.state ?
> +		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
>  
> -	if (INTEL_INFO(dev)->gen >= 9)
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
>  		can_position = true;
> +	}
>  
>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    src, dest, clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> +					    min_scale,
> +					    max_scale,
>  					    can_position, true,
>  					    &state->visible);
>  	if (ret)
> @@ -12874,6 +12991,13 @@ intel_check_primary_plane(struct drm_plane *plane,
>  			intel_crtc->atomic.update_wm = true;
>  	}
>  
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_update_scaler_users(intel_crtc, crtc_state,
> +			to_intel_plane(plane), state, 0);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -13053,6 +13177,9 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  
>  	primary->can_scale = false;
>  	primary->max_downscale = 1;
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		primary->can_scale = true;
> +	}
>  	state->scaler_id = -1;
>  	primary->pipe = pipe;
>  	primary->plane = pipe;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 275a4ef..c9bc975 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -288,11 +288,11 @@ struct intel_initial_plane_config {
>  #define SKL_MIN_SRC_W 8
>  #define SKL_MAX_SRC_W 4096
>  #define SKL_MIN_SRC_H 8
> -#define SKL_MAX_SRC_H 2304
> +#define SKL_MAX_SRC_H 4096
>  #define SKL_MIN_DST_W 8
>  #define SKL_MAX_DST_W 4096
>  #define SKL_MIN_DST_H 8
> -#define SKL_MAX_DST_H 2304
> +#define SKL_MAX_DST_H 4096
>  
>  struct intel_scaler {
>  	int id;
> @@ -1131,9 +1131,13 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc);
>  int skl_update_scaler_users(struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
>  	struct intel_plane_state *plane_state, int force_detach);
> +int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>  
>  unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>  				     struct drm_i915_gem_object *obj);
> +u32 skl_plane_ctl_format(uint32_t pixel_format);
> +u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> +u32 skl_plane_ctl_rotation(unsigned int rotation);
>  
>  /* intel_dp.c */
>  void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0cb3767..2f42777 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1150,6 +1150,15 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  	}
>  
>  	intel_plane = to_intel_plane(plane);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* plane scaling and colorkey are mutually exclusive */
> +		if (to_intel_plane_state(plane->state)->scaler_id >= 0) {
> +			DRM_ERROR("colorkey not allowed with scaler\n");
> +			return -EINVAL;

The error path here doesn't correctly release locks. I guess adding some
igt to exercise this path would be good.
-Daniel

> +		}
> +	}
> +
>  	intel_plane->ckey = *set;
>  
>  	/*
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chandra Konduru April 27, 2015, 5:13 a.m. UTC | #5

> -----Original Message-----
> From: Daniel Vetter [mailto:[e-mail]] On Behalf Of Daniel Vetter
> Sent: Thursday, April 23, 2015 1:20 PM
> To: Konduru, Chandra
> Cc: [e-mail]; Vetter, Daniel; Conselvan De Oliveira, Ander
> Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling
> using shared scalers
> 
> On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote:
> > This patch enables skylake primary plane scaling using shared
> > scalers atomic desgin.
> >
> > v2:
> > -use single copy of scaler limits (Matt)
> >
> > v3:
> > -move detach_scalers to crtc commit path (Matt)
> > -use values in plane_state->src as regular integers (me)
> >
> > v4:
> > -changes to align with updated scaler structures (Matt, me)
> > -keep plane src rect in 16.16 format (Matt, Daniel)
> >
> > v5:
> > -Rebased on top of 90/270 rotation changes (me)
> > -Fixed an issue introduced by 90/270 changes where plane programming
> >  is using drm_plane->state rect instead of intel_plane->state rect.
> >  This change also required for scaling to work properly. (me)
> > -With 90/270, updated limits to cover both portrait and landscape usages (me)
> > -Refactored skylake_update_primary_plane to reduce its size (Daniel)
> >  Added helper functions for refactoring are comprehended enough to be
> >  used for skylake_update_plane (for sprite) too. One stop towards
> >  having single function for all planes.
> >
> > Signed-off-by: Chandra Konduru <[e-mail]>
> > Reviewed-by: Matt Roper <[e-mail]>
> > Testcase: kms_plane_scaling
> 
> I wanted to pull this in but then spotted an issue. Since this needs one
> more round can you perhaps use the older version as a baseline again
> (without the refactoring)? Since skl planes aren't converted to universal
> planes yet it might be better to wait with refactoring until that's done
> actually. Two more comments below.
Hi Daniel,
Sorry for delay due to osts/f2f travel.
Per last discussion at osts, you were ok to merge the changes made to
skl update_plane functions to reduce its size. To undo these
changes I have to redo all testing and also have to redo changes
to upcoming nv12 changes.
skl planes are already universal planes, Matt can comment more
on this. But irrespective of whether skl planes universal or not
with below changes, function sizes are more manageable.

Regarding the FIXME and lock issue, will send out updated 
patch shortly. 

-Chandra

Daniel Vetter May 4, 2015, 8:16 a.m. UTC | #6

On Mon, Apr 27, 2015 at 05:13:49AM +0000, Konduru, Chandra wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:[e-mail]] On Behalf Of Daniel Vetter
> > Sent: Thursday, April 23, 2015 1:20 PM
> > To: Konduru, Chandra
> > Cc: [e-mail]; Vetter, Daniel; Conselvan De Oliveira, Ander
> > Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling
> > using shared scalers
> > 
> > On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote:
> > > This patch enables skylake primary plane scaling using shared
> > > scalers atomic desgin.
> > >
> > > v2:
> > > -use single copy of scaler limits (Matt)
> > >
> > > v3:
> > > -move detach_scalers to crtc commit path (Matt)
> > > -use values in plane_state->src as regular integers (me)
> > >
> > > v4:
> > > -changes to align with updated scaler structures (Matt, me)
> > > -keep plane src rect in 16.16 format (Matt, Daniel)
> > >
> > > v5:
> > > -Rebased on top of 90/270 rotation changes (me)
> > > -Fixed an issue introduced by 90/270 changes where plane programming
> > >  is using drm_plane->state rect instead of intel_plane->state rect.
> > >  This change also required for scaling to work properly. (me)
> > > -With 90/270, updated limits to cover both portrait and landscape usages (me)
> > > -Refactored skylake_update_primary_plane to reduce its size (Daniel)
> > >  Added helper functions for refactoring are comprehended enough to be
> > >  used for skylake_update_plane (for sprite) too. One stop towards
> > >  having single function for all planes.
> > >
> > > Signed-off-by: Chandra Konduru <[e-mail]>
> > > Reviewed-by: Matt Roper <[e-mail]>
> > > Testcase: kms_plane_scaling
> > 
> > I wanted to pull this in but then spotted an issue. Since this needs one
> > more round can you perhaps use the older version as a baseline again
> > (without the refactoring)? Since skl planes aren't converted to universal
> > planes yet it might be better to wait with refactoring until that's done
> > actually. Two more comments below.
> Hi Daniel,
> Sorry for delay due to osts/f2f travel.
> Per last discussion at osts, you were ok to merge the changes made to
> skl update_plane functions to reduce its size. To undo these
> changes I have to redo all testing and also have to redo changes
> to upcoming nv12 changes.
> skl planes are already universal planes, Matt can comment more
> on this. But irrespective of whether skl planes universal or not
> with below changes, function sizes are more manageable.

Yeah I looked at splitting up the refactoring, but because of the
divergent history of the primary and sprite planes (which converged now in
the hw for skl) it was messier than your patches here. Agreed that this is
the exception which confirms the rule ;-)
> 
> Regarding the FIXME and lock issue, will send out updated 
> patch shortly. 

Thanks, both applied to dinq.
-Daniel

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 3c4b7cd..cb6d5f2 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -169,7 +169,7 @@  int intel_atomic_commit(struct drm_device *dev,
 		plane->state->state = NULL;
 	}
 
-	/* swap crtc_state */
+	/* swap crtc_scaler_state */
 	for (i = 0; i < dev->mode_config.num_crtc; i++) {
 		struct drm_crtc *crtc = state->crtcs[i];
 		if (!crtc) {
@@ -178,6 +178,9 @@  int intel_atomic_commit(struct drm_device *dev,
 
 		to_intel_crtc(crtc)->config->scaler_state =
 			to_intel_crtc_state(state->crtc_states[i])->scaler_state;
+
+		if (INTEL_INFO(dev)->gen >= 9)
+			skl_detach_scalers(to_intel_crtc(crtc));
 	}
 
 	drm_atomic_helper_commit_planes(dev, state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5f7a40f..7f3ae8e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2960,126 +2960,199 @@  void skl_detach_scalers(struct intel_crtc *intel_crtc)
 	}
 }
 
-static void skylake_update_primary_plane(struct drm_crtc *crtc,
-					 struct drm_framebuffer *fb,
-					 int x, int y)
+u32 skl_plane_ctl_format(uint32_t pixel_format)
 {
-	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_i915_gem_object *obj;
-	int pipe = intel_crtc->pipe;
-	u32 plane_ctl, stride_div, stride;
-	u32 tile_height, plane_offset, plane_size;
-	unsigned int rotation;
-	int x_offset, y_offset;
-	unsigned long surf_addr;
-	struct drm_plane *plane;
-
-	if (!intel_crtc->primary_enabled) {
-		I915_WRITE(PLANE_CTL(pipe, 0), 0);
-		I915_WRITE(PLANE_SURF(pipe, 0), 0);
-		POSTING_READ(PLANE_CTL(pipe, 0));
-		return;
-	}
-
-	plane_ctl = PLANE_CTL_ENABLE |
-		    PLANE_CTL_PIPE_GAMMA_ENABLE |
-		    PLANE_CTL_PIPE_CSC_ENABLE;
-
-	switch (fb->pixel_format) {
+	u32 plane_ctl_format = 0;
+	switch (pixel_format) {
 	case DRM_FORMAT_RGB565:
-		plane_ctl |= PLANE_CTL_FORMAT_RGB_565;
-		break;
-	case DRM_FORMAT_XRGB8888:
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
-		break;
-	case DRM_FORMAT_ARGB8888:
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
-		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+		plane_ctl_format = PLANE_CTL_FORMAT_RGB_565;
 		break;
 	case DRM_FORMAT_XBGR8888:
-		plane_ctl |= PLANE_CTL_ORDER_RGBX;
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
+		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
 		break;
+	case DRM_FORMAT_XRGB8888:
+		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888;
+		break;
+	/*
+	 * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
+	 * to be already pre-multiplied. We need to add a knob (or a different
+	 * DRM_FORMAT) for user-space to configure that.
+	 */
 	case DRM_FORMAT_ABGR8888:
-		plane_ctl |= PLANE_CTL_ORDER_RGBX;
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;
-		plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
+			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+		break;
+	case DRM_FORMAT_ARGB8888:
+		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_8888 |
+			PLANE_CTL_ALPHA_SW_PREMULTIPLY;
 		break;
 	case DRM_FORMAT_XRGB2101010:
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
+		plane_ctl_format = PLANE_CTL_FORMAT_XRGB_2101010;
 		break;
 	case DRM_FORMAT_XBGR2101010:
-		plane_ctl |= PLANE_CTL_ORDER_RGBX;
-		plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;
+		plane_ctl_format = PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
+		break;
+	case DRM_FORMAT_YUYV:
+		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
+		break;
+	case DRM_FORMAT_YVYU:
+		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YVYU;
+		break;
+	case DRM_FORMAT_UYVY:
+		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
+		break;
+	case DRM_FORMAT_VYUY:
+		plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
 		break;
 	default:
 		BUG();
 	}
+	return plane_ctl_format;
+}
 
-	switch (fb->modifier[0]) {
+u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
+{
+	u32 plane_ctl_tiling = 0;
+	switch (fb_modifier) {
 	case DRM_FORMAT_MOD_NONE:
 		break;
 	case I915_FORMAT_MOD_X_TILED:
-		plane_ctl |= PLANE_CTL_TILED_X;
+		plane_ctl_tiling = PLANE_CTL_TILED_X;
 		break;
 	case I915_FORMAT_MOD_Y_TILED:
-		plane_ctl |= PLANE_CTL_TILED_Y;
+		plane_ctl_tiling = PLANE_CTL_TILED_Y;
 		break;
 	case I915_FORMAT_MOD_Yf_TILED:
-		plane_ctl |= PLANE_CTL_TILED_YF;
+		plane_ctl_tiling = PLANE_CTL_TILED_YF;
 		break;
 	default:
-		MISSING_CASE(fb->modifier[0]);
+		MISSING_CASE(fb_modifier);
 	}
+	return plane_ctl_tiling;
+}
 
-	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-
-	plane = crtc->primary;
-	rotation = plane->state->rotation;
+u32 skl_plane_ctl_rotation(unsigned int rotation)
+{
+	u32 plane_ctl_rotation = 0;
 	switch (rotation) {
+	case BIT(DRM_ROTATE_0):
+		break;
 	case BIT(DRM_ROTATE_90):
-		plane_ctl |= PLANE_CTL_ROTATE_90;
+		plane_ctl_rotation = PLANE_CTL_ROTATE_90;
 		break;
-
 	case BIT(DRM_ROTATE_180):
-		plane_ctl |= PLANE_CTL_ROTATE_180;
+		plane_ctl_rotation = PLANE_CTL_ROTATE_180;
 		break;
-
 	case BIT(DRM_ROTATE_270):
-		plane_ctl |= PLANE_CTL_ROTATE_270;
+		plane_ctl_rotation = PLANE_CTL_ROTATE_270;
 		break;
+	default:
+		MISSING_CASE(rotation);
 	}
 
+	return plane_ctl_rotation;
+}
+
+static void skylake_update_primary_plane(struct drm_crtc *crtc,
+					 struct drm_framebuffer *fb,
+					 int x, int y)
+{
+	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_i915_gem_object *obj;
+	int pipe = intel_crtc->pipe;
+	u32 plane_ctl, stride_div, stride;
+	u32 tile_height, plane_offset, plane_size;
+	unsigned int rotation;
+	int x_offset, y_offset;
+	unsigned long surf_addr;
+	struct drm_plane *plane;
+	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 = crtc->primary;
+	plane_state = plane ? to_intel_plane_state(crtc->primary->state) : NULL;
+
+	if (!intel_crtc->primary_enabled) {
+		I915_WRITE(PLANE_CTL(pipe, 0), 0);
+		I915_WRITE(PLANE_SURF(pipe, 0), 0);
+		POSTING_READ(PLANE_CTL(pipe, 0));
+		return;
+	}
+
+	plane_ctl = PLANE_CTL_ENABLE |
+		    PLANE_CTL_PIPE_GAMMA_ENABLE |
+		    PLANE_CTL_PIPE_CSC_ENABLE;
+
+	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);
 
+	if (plane_state && drm_rect_width(&plane_state->src)) {
+		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);
+	} else {
+		src_w = intel_crtc->config->pipe_src_w;
+		src_h = intel_crtc->config->pipe_src_h;
+	}
+
 	if (intel_rotation_90_or_270(rotation)) {
 		/* stride = Surface height in tiles */
 		tile_height = intel_tile_height(dev, fb->bits_per_pixel,
 						fb->modifier[0]);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
-		x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
+		x_offset = stride * tile_height - y - src_h;
 		y_offset = x;
-		plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
-					((plane->state->src_h >> 16) - 1);
+		plane_size = (src_w - 1) << 16 | (src_h - 1);
 	} else {
 		stride = fb->pitches[0] / stride_div;
 		x_offset = x;
 		y_offset = y;
-		plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
-			((plane->state->src_w >> 16) - 1);
+		plane_size = (src_h - 1) << 16 | (src_w - 1);
 	}
 	plane_offset = y_offset << 16 | x_offset;
 
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
-	I915_WRITE(PLANE_POS(pipe, 0), 0);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
 	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
 	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
+
+	if (scaler_id >= 0) {
+		uint32_t ps_ctrl = 0;
+
+		WARN_ON(!dst_w || !dst_h);
+		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
+			crtc_state->scaler_state.scalers[scaler_id].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), (dst_x << 16) | dst_y);
+		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
+		I915_WRITE(PLANE_POS(pipe, 0), 0);
+	} else {
+		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
+	}
+
 	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
@@ -4338,6 +4411,7 @@  skl_update_scaler_users(
 	int *scaler_id;
 	struct drm_framebuffer *fb;
 	struct intel_crtc_scaler_state *scaler_state;
+	unsigned int rotation;
 
 	if (!intel_crtc || !crtc_state)
 		return 0;
@@ -4353,6 +4427,7 @@  skl_update_scaler_users(
 		dst_w = drm_rect_width(&plane_state->dst);
 		dst_h = drm_rect_height(&plane_state->dst);
 		scaler_id = &plane_state->scaler_id;
+		rotation = plane_state->base.rotation;
 	} else {
 		struct drm_display_mode *adjusted_mode =
 			&crtc_state->base.adjusted_mode;
@@ -4361,8 +4436,12 @@  skl_update_scaler_users(
 		dst_w = adjusted_mode->hdisplay;
 		dst_h = adjusted_mode->vdisplay;
 		scaler_id = &scaler_state->scaler_id;
+		rotation = DRM_ROTATE_0;
 	}
-	need_scaling = (src_w != dst_w || src_h != dst_h);
+
+	need_scaling = intel_rotation_90_or_270(rotation) ?
+		(src_h != dst_w || src_w != dst_h):
+		(src_w != dst_w || src_h != dst_h);
 
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
@@ -12805,6 +12884,36 @@  intel_cleanup_plane_fb(struct drm_plane *plane,
 	}
 }
 
+int
+skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
+{
+	int max_scale;
+	struct drm_device *dev;
+	struct drm_i915_private *dev_priv;
+	int crtc_clock, cdclk;
+
+	if (!intel_crtc || !crtc_state)
+		return DRM_PLANE_HELPER_NO_SCALING;
+
+	dev = intel_crtc->base.dev;
+	dev_priv = dev->dev_private;
+	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
+	cdclk = dev_priv->display.get_display_clock_speed(dev);
+
+	if (!crtc_clock || !cdclk)
+		return DRM_PLANE_HELPER_NO_SCALING;
+
+	/*
+	 * skl max scale is lower of:
+	 *    close to 3 but not 3, -1 is for that purpose
+	 *            or
+	 *    cdclk/crtc_clock
+	 */
+	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
+
+	return max_scale;
+}
+
 static int
 intel_check_primary_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
@@ -12813,23 +12922,31 @@  intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *crtc_state;
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_rect *dest = &state->dst;
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
 	bool can_position = false;
+	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
+	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
 	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
+	crtc_state = state->base.state ?
+		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
 
-	if (INTEL_INFO(dev)->gen >= 9)
+	if (INTEL_INFO(dev)->gen >= 9) {
+		min_scale = 1;
+		max_scale = skl_max_scale(intel_crtc, crtc_state);
 		can_position = true;
+	}
 
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    src, dest, clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
+					    min_scale,
+					    max_scale,
 					    can_position, true,
 					    &state->visible);
 	if (ret)
@@ -12874,6 +12991,13 @@  intel_check_primary_plane(struct drm_plane *plane,
 			intel_crtc->atomic.update_wm = true;
 	}
 
+	if (INTEL_INFO(dev)->gen >= 9) {
+		ret = skl_update_scaler_users(intel_crtc, crtc_state,
+			to_intel_plane(plane), state, 0);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -13053,6 +13177,9 @@  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 
 	primary->can_scale = false;
 	primary->max_downscale = 1;
+	if (INTEL_INFO(dev)->gen >= 9) {
+		primary->can_scale = true;
+	}
 	state->scaler_id = -1;
 	primary->pipe = pipe;
 	primary->plane = pipe;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 275a4ef..c9bc975 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -288,11 +288,11 @@  struct intel_initial_plane_config {
 #define SKL_MIN_SRC_W 8
 #define SKL_MAX_SRC_W 4096
 #define SKL_MIN_SRC_H 8
-#define SKL_MAX_SRC_H 2304
+#define SKL_MAX_SRC_H 4096
 #define SKL_MIN_DST_W 8
 #define SKL_MAX_DST_W 4096
 #define SKL_MIN_DST_H 8
-#define SKL_MAX_DST_H 2304
+#define SKL_MAX_DST_H 4096
 
 struct intel_scaler {
 	int id;
@@ -1131,9 +1131,13 @@  void skl_detach_scalers(struct intel_crtc *intel_crtc);
 int skl_update_scaler_users(struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
 	struct intel_plane_state *plane_state, int force_detach);
+int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
 
 unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
 				     struct drm_i915_gem_object *obj);
+u32 skl_plane_ctl_format(uint32_t pixel_format);
+u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
+u32 skl_plane_ctl_rotation(unsigned int rotation);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0cb3767..2f42777 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1150,6 +1150,15 @@  int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 	}
 
 	intel_plane = to_intel_plane(plane);
+
+	if (INTEL_INFO(dev)->gen >= 9) {
+		/* plane scaling and colorkey are mutually exclusive */
+		if (to_intel_plane_state(plane->state)->scaler_id >= 0) {
+			DRM_ERROR("colorkey not allowed with scaler\n");
+			return -EINVAL;
+		}
+	}
+
 	intel_plane->ckey = *set;
 
 	/*