diff mbox

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

Message ID 1428445727-18103-14-git-send-email-chandra.konduru@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chandra Konduru April 7, 2015, 10:28 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)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
 drivers/gpu/drm/i915/intel_display.c |   96 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
 4 files changed, 105 insertions(+), 6 deletions(-)

Comments

Matt Roper April 9, 2015, 9:51 p.m. UTC | #1
On Tue, Apr 07, 2015 at 03:28:46PM -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)

See comments on patch #6 that relate to this.  If you want to take the
approach here (perform the unshift in skl_update_plane) then you also
need to do the same for all other platforms (ivb, ilk, vlv, etc.).  But
my suggestion on the other patch (do the unshifting in commit_plane and
leave skl_update_plane alone) might be a bit simpler.


Matt

> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |   96 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
>  4 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 2fc04ec..8f759c6 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -167,7 +167,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) {
> @@ -176,6 +176,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 aa4da1f..c7ee232 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl, stride_div;
>  	unsigned long surf_addr;
> +	struct intel_crtc_state *crtc_state = intel_crtc->config;
> +	struct intel_plane_state *plane_state;
> +	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> +	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> +	int scaler_id = -1;
> +
> +	plane_state = crtc->primary ?
> +		to_intel_plane_state(crtc->primary->state) : NULL;
>  
>  	if (!intel_crtc->primary_enabled) {
>  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  					       fb->pixel_format);
>  	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
>  
> +	if (plane_state) {
> +		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);
> +	}
> +
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>  	I915_WRITE(PLANE_POS(pipe, 0), 0);
> +
> +	if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(x != src_x || y != src_y);
> +		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_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w - 1));
> +	} else {
> +		I915_WRITE(PLANE_SIZE(pipe, 0),
> +			(intel_crtc->config->pipe_src_h - 1) << 16 |
> +			(intel_crtc->config->pipe_src_w - 1));
> +	}
> +
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> -	I915_WRITE(PLANE_SIZE(pipe, 0),
> -		   (intel_crtc->config->pipe_src_h - 1) << 16 |
> -		   (intel_crtc->config->pipe_src_w - 1));
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
> @@ -12778,6 +12814,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)
> @@ -12786,19 +12852,29 @@ 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;
> +	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) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
> +	}
>  
>  	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,
>  					    false, true, &state->visible);
>  	if (ret)
>  		return ret;
> @@ -12842,6 +12918,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;
>  }
>  
> @@ -13021,6 +13104,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 7b28bff..0543150 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1125,6 +1125,7 @@ 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);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c05fb36..955965c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1119,6 +1119,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
>
Chandra Konduru April 9, 2015, 10:14 p.m. UTC | #2
> -----Original Message-----
> From: Roper, Matthew D
> Sent: Thursday, April 09, 2015 2:52 PM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander
> Subject: Re: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared
> scalers
> 
> On Tue, Apr 07, 2015 at 03:28:46PM -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)
> 
> See comments on patch #6 that relate to this.  If you want to take the approach
> here (perform the unshift in skl_update_plane) then you also need to do the
> same for all other platforms (ivb, ilk, vlv, etc.).  But my suggestion on the other
> patch (do the unshifting in commit_plane and leave skl_update_plane alone)
> might be a bit simpler.

Above v4: comment is saying that the change done in v3 was undone to keep 
primary_plane src rect in 16.16 format.

As I responded to your patch #6 comment, moving unshift hunk (which is for
sprite plane) from #14 to #6 will avoid any potential bisect issue that you mentioned.

For other than skylake, primary planes platform level update functions doesn't use
src_rect instead operate based on passed parameters which are in regular ints.
Only for skylake primary plane update function, src rect is used for windowing,
scaling purposes.

> 
> 
> Matt
> 
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
> >  drivers/gpu/drm/i915/intel_display.c |   96
> ++++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_drv.h     |    1 +
> >  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
> >  4 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index 2fc04ec..8f759c6 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -167,7 +167,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) {
> > @@ -176,6 +176,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 aa4da1f..c7ee232 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> >  	int pipe = intel_crtc->pipe;
> >  	u32 plane_ctl, stride_div;
> >  	unsigned long surf_addr;
> > +	struct intel_crtc_state *crtc_state = intel_crtc->config;
> > +	struct intel_plane_state *plane_state;
> > +	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> > +	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> > +	int scaler_id = -1;
> > +
> > +	plane_state = crtc->primary ?
> > +		to_intel_plane_state(crtc->primary->state) : NULL;
> >
> >  	if (!intel_crtc->primary_enabled) {
> >  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> > @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> >  					       fb->pixel_format);
> >  	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary),
> > obj);
> >
> > +	if (plane_state) {
> > +		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);
> > +	}
> > +
> >  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> >  	I915_WRITE(PLANE_POS(pipe, 0), 0);
> > +
> > +	if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
> > +		uint32_t ps_ctrl = 0;
> > +
> > +		WARN_ON(x != src_x || y != src_y);
> > +		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_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w -
> 1));
> > +	} else {
> > +		I915_WRITE(PLANE_SIZE(pipe, 0),
> > +			(intel_crtc->config->pipe_src_h - 1) << 16 |
> > +			(intel_crtc->config->pipe_src_w - 1));
> > +	}
> > +
> >  	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> > -	I915_WRITE(PLANE_SIZE(pipe, 0),
> > -		   (intel_crtc->config->pipe_src_h - 1) << 16 |
> > -		   (intel_crtc->config->pipe_src_w - 1));
> >  	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
> >  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
> >
> > @@ -12778,6 +12814,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) @@ -12786,19
> +12852,29 @@
> > 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;
> > +	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) {
> > +		min_scale = 1;
> > +		max_scale = skl_max_scale(intel_crtc, crtc_state);
> > +	}
> >
> >  	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,
> >  					    false, true, &state->visible);
> >  	if (ret)
> >  		return ret;
> > @@ -12842,6 +12918,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;
> >  }
> >
> > @@ -13021,6 +13104,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 7b28bff..0543150 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1125,6 +1125,7 @@ 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); diff --git
> > a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index c05fb36..955965c 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1119,6 +1119,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
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
Daniel Vetter April 13, 2015, 9:46 a.m. UTC | #3
On Thu, Apr 09, 2015 at 10:14:36PM +0000, Konduru, Chandra wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D
> > Sent: Thursday, April 09, 2015 2:52 PM
> > To: Konduru, Chandra
> > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander
> > Subject: Re: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared
> > scalers
> > 
> > On Tue, Apr 07, 2015 at 03:28:46PM -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)
> > 
> > See comments on patch #6 that relate to this.  If you want to take the approach
> > here (perform the unshift in skl_update_plane) then you also need to do the
> > same for all other platforms (ivb, ilk, vlv, etc.).  But my suggestion on the other
> > patch (do the unshifting in commit_plane and leave skl_update_plane alone)
> > might be a bit simpler.
> 
> Above v4: comment is saying that the change done in v3 was undone to keep 
> primary_plane src rect in 16.16 format.
> 
> As I responded to your patch #6 comment, moving unshift hunk (which is for
> sprite plane) from #14 to #6 will avoid any potential bisect issue that you mentioned.
> 
> For other than skylake, primary planes platform level update functions doesn't use
> src_rect instead operate based on passed parameters which are in regular ints.
> Only for skylake primary plane update function, src rect is used for windowing,
> scaling purposes.

I merged up to patch 12, but this one here doesn't apply any more and I'm
not sure whether there's any changes still needed to it (it sounds like
not, but chasing that unshifting business is tricky). Chandra, can you
please resend rebased patches (with Matt's r-b added)?

Thanks, Daniel
Daniel Vetter April 13, 2015, 6:12 p.m. UTC | #4
On Tue, Apr 07, 2015 at 03:28:46PM -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)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

Ok there's a real functional conflict here now with 90/270 rotation. It's
big enough that imo we also need to add igt coverage to test
rotation+scaling together (the scaled plane setup is different from the
normal one, and we need to also make sure we scale correctly itself when
rotated).

Chandra can you please figure out with Sonika who can do this rebasing/igt
extension?

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |   96 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
>  4 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 2fc04ec..8f759c6 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -167,7 +167,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) {
> @@ -176,6 +176,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 aa4da1f..c7ee232 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl, stride_div;
>  	unsigned long surf_addr;
> +	struct intel_crtc_state *crtc_state = intel_crtc->config;
> +	struct intel_plane_state *plane_state;
> +	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> +	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> +	int scaler_id = -1;
> +
> +	plane_state = crtc->primary ?
> +		to_intel_plane_state(crtc->primary->state) : NULL;
>  
>  	if (!intel_crtc->primary_enabled) {
>  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  					       fb->pixel_format);
>  	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
>  
> +	if (plane_state) {
> +		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);
> +	}
> +
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>  	I915_WRITE(PLANE_POS(pipe, 0), 0);
> +
> +	if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
> +		uint32_t ps_ctrl = 0;
> +
> +		WARN_ON(x != src_x || y != src_y);
> +		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_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w - 1));
> +	} else {
> +		I915_WRITE(PLANE_SIZE(pipe, 0),
> +			(intel_crtc->config->pipe_src_h - 1) << 16 |
> +			(intel_crtc->config->pipe_src_w - 1));
> +	}
> +
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> -	I915_WRITE(PLANE_SIZE(pipe, 0),
> -		   (intel_crtc->config->pipe_src_h - 1) << 16 |
> -		   (intel_crtc->config->pipe_src_w - 1));
>  	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
> @@ -12778,6 +12814,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)
> @@ -12786,19 +12852,29 @@ 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;
> +	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) {
> +		min_scale = 1;
> +		max_scale = skl_max_scale(intel_crtc, crtc_state);
> +	}
>  
>  	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,
>  					    false, true, &state->visible);
>  	if (ret)
>  		return ret;
> @@ -12842,6 +12918,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;
>  }
>  
> @@ -13021,6 +13104,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 7b28bff..0543150 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1125,6 +1125,7 @@ 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);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c05fb36..955965c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1119,6 +1119,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
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chandra Konduru April 13, 2015, 6:18 p.m. UTC | #5
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Monday, April 13, 2015 11:13 AM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel
> Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling
> using shared scalers
> 
> On Tue, Apr 07, 2015 at 03:28:46PM -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)
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> 
> Ok there's a real functional conflict here now with 90/270 rotation. It's big
> enough that imo we also need to add igt coverage to test
> rotation+scaling together (the scaled plane setup is different from the
> normal one, and we need to also make sure we scale correctly itself when
> rotated).
> 
> Chandra can you please figure out with Sonika who can do this rebasing/igt
> extension?

To give some context, this was one of the reasons I gave heads up to Sonika 
to rebase 90/270 on top of scalers but looks they weren't.

Agree that we need igt for rotation+scaling too. In current kms_plane_scaling
didn't planned to cover 90/270 for the same reason that igt for 90/270 taking 
care of that.

Sure, will sync up offline with Sonika and get back on the last two patches and
igt too.

> 
> Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
> >  drivers/gpu/drm/i915/intel_display.c |   96
> ++++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_drv.h     |    1 +
> >  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
> >  4 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index 2fc04ec..8f759c6 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -167,7 +167,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) {
> > @@ -176,6 +176,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 aa4da1f..c7ee232 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> >  	int pipe = intel_crtc->pipe;
> >  	u32 plane_ctl, stride_div;
> >  	unsigned long surf_addr;
> > +	struct intel_crtc_state *crtc_state = intel_crtc->config;
> > +	struct intel_plane_state *plane_state;
> > +	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> > +	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> > +	int scaler_id = -1;
> > +
> > +	plane_state = crtc->primary ?
> > +		to_intel_plane_state(crtc->primary->state) : NULL;
> >
> >  	if (!intel_crtc->primary_enabled) {
> >  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
> > @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> >  					       fb->pixel_format);
> >  	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary),
> > obj);
> >
> > +	if (plane_state) {
> > +		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);
> > +	}
> > +
> >  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> >  	I915_WRITE(PLANE_POS(pipe, 0), 0);
> > +
> > +	if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
> > +		uint32_t ps_ctrl = 0;
> > +
> > +		WARN_ON(x != src_x || y != src_y);
> > +		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_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w -
> 1));
> > +	} else {
> > +		I915_WRITE(PLANE_SIZE(pipe, 0),
> > +			(intel_crtc->config->pipe_src_h - 1) << 16 |
> > +			(intel_crtc->config->pipe_src_w - 1));
> > +	}
> > +
> >  	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> > -	I915_WRITE(PLANE_SIZE(pipe, 0),
> > -		   (intel_crtc->config->pipe_src_h - 1) << 16 |
> > -		   (intel_crtc->config->pipe_src_w - 1));
> >  	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
> >  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
> >
> > @@ -12778,6 +12814,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) @@ -12786,19
> +12852,29 @@
> > 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;
> > +	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) {
> > +		min_scale = 1;
> > +		max_scale = skl_max_scale(intel_crtc, crtc_state);
> > +	}
> >
> >  	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,
> >  					    false, true, &state->visible);
> >  	if (ret)
> >  		return ret;
> > @@ -12842,6 +12918,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;
> >  }
> >
> > @@ -13021,6 +13104,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 7b28bff..0543150 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1125,6 +1125,7 @@ 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); diff --git
> > a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index c05fb36..955965c 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1119,6 +1119,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
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter April 14, 2015, 7 a.m. UTC | #6
On Tue, Apr 07, 2015 at 03:28:46PM -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)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
>  drivers/gpu/drm/i915/intel_display.c |   96 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_sprite.c  |    9 ++++
>  4 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 2fc04ec..8f759c6 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -167,7 +167,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) {
> @@ -176,6 +176,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 aa4da1f..c7ee232 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,

And one real piece of review feedback: This function is now definitely too
long. Same holds for the sprite update function below.

Can you (or Sonika) please follow up with a few patches to extract
subroutines to make this a bit easier to read?

Thanks, Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 2fc04ec..8f759c6 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -167,7 +167,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) {
@@ -176,6 +176,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 aa4da1f..c7ee232 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2978,6 +2978,14 @@  static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl, stride_div;
 	unsigned long surf_addr;
+	struct intel_crtc_state *crtc_state = intel_crtc->config;
+	struct intel_plane_state *plane_state;
+	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
+	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
+	int scaler_id = -1;
+
+	plane_state = crtc->primary ?
+		to_intel_plane_state(crtc->primary->state) : NULL;
 
 	if (!intel_crtc->primary_enabled) {
 		I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -3046,12 +3054,40 @@  static void skylake_update_primary_plane(struct drm_crtc *crtc,
 					       fb->pixel_format);
 	surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
 
+	if (plane_state) {
+		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);
+	}
+
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_POS(pipe, 0), 0);
+
+	if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
+		uint32_t ps_ctrl = 0;
+
+		WARN_ON(x != src_x || y != src_y);
+		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_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w - 1));
+	} else {
+		I915_WRITE(PLANE_SIZE(pipe, 0),
+			(intel_crtc->config->pipe_src_h - 1) << 16 |
+			(intel_crtc->config->pipe_src_w - 1));
+	}
+
 	I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
-	I915_WRITE(PLANE_SIZE(pipe, 0),
-		   (intel_crtc->config->pipe_src_h - 1) << 16 |
-		   (intel_crtc->config->pipe_src_w - 1));
 	I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
 	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
 
@@ -12778,6 +12814,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)
@@ -12786,19 +12852,29 @@  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;
+	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) {
+		min_scale = 1;
+		max_scale = skl_max_scale(intel_crtc, crtc_state);
+	}
 
 	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,
 					    false, true, &state->visible);
 	if (ret)
 		return ret;
@@ -12842,6 +12918,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;
 }
 
@@ -13021,6 +13104,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 7b28bff..0543150 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1125,6 +1125,7 @@  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);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c05fb36..955965c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1119,6 +1119,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;
 
 	/*