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

Konduru, Chandra chandra.konduru at intel.com
Mon Apr 13 11:18:22 PDT 2015



> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Monday, April 13, 2015 11:13 AM
> To: Konduru, Chandra
> Cc: intel-gfx at 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 at 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


More information about the Intel-gfx mailing list