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

[Daniel Vetter <daniel@ffwll.ch>]

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

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