Re: [PATCH 1/3] drm/i915: Do not acquire crtc state to check clock during modeset, v2.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 27, 2015 at 02:26:31PM +0100, Maarten Lankhorst wrote:
> Parallel modesets are still not allowed, but this will allow updating
> a different crtc during a modeset if the clock is not changed.
> 
> Additionally when all pipes are DPMS off the cdclk will be lowered
> to the minimum allowed.
> 
> Changes since v1:
> - Add dev_priv->active_crtcs for tracking which crtcs are active.
> - Rename min_cdclk to min_pixclk and move to dev_priv.
> - Add a active_crtcs mask which is updated atomically.
> - Add intel_atomic_state->modeset which is set on modesets.
> - Commit new pixclk/active_crtcs right after state swap.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   5 ++
>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
>  drivers/gpu/drm/i915/intel_display.c | 160 +++++++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_drv.h     |   7 +-
>  4 files changed, 125 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 59c94929f25b..37eef86e4a0a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1837,8 +1837,13 @@ struct drm_i915_private {
>  	struct intel_pipe_crc pipe_crc[I915_MAX_PIPES];
>  #endif
>  
> +	/* dpll and cdclk state is protected by connection_mutex */
>  	int num_shared_dpll;
>  	struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
> +
> +	unsigned int active_crtcs;
> +	unsigned int min_pixclk[I915_MAX_PIPES];
> +
>  	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>  
>  	struct i915_workarounds workarounds;
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 25a891aa3824..cb8fcf4fe7f2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -305,5 +305,5 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
>  {
>  	struct intel_atomic_state *state = to_intel_atomic_state(s);
>  	drm_atomic_state_default_clear(&state->base);
> -	state->dpll_set = false;
> +	state->dpll_set = state->modeset = false;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 022e628b8520..a0508d5e9e13 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5237,6 +5237,7 @@ static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
>  
>  static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
>  {
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	unsigned long put_domains[I915_MAX_PIPES] = {};
> @@ -5245,13 +5246,18 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
>  	int i;
>  
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (needs_modeset(crtc->state))
> -			put_domains[to_intel_crtc(crtc)->pipe] =
> -				modeset_get_crtc_power_domains(crtc);
> +		struct intel_crtc *intel_crtc =
> +			to_intel_crtc(crtc);
> +		enum pipe pipe = intel_crtc->pipe;
> +
> +		if (!needs_modeset(crtc->state))
> +			continue;
> +
> +		put_domains[pipe] = modeset_get_crtc_power_domains(crtc);
>  	}
>  
>  	if (dev_priv->display.modeset_commit_cdclk) {
> -		unsigned int cdclk = to_intel_atomic_state(state)->cdclk;
> +		unsigned int cdclk = intel_state->cdclk;
>  
>  		if (cdclk != dev_priv->cdclk_freq &&
>  		    !WARN_ON(!state->allow_modeset))
> @@ -5938,23 +5944,32 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
>  static int intel_mode_max_pixclk(struct drm_device *dev,
>  				 struct drm_atomic_state *state)
>  {
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int max_pixclk = 0;
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	unsigned max_pixel_rate = 0, i;
> +	enum pipe pipe;
>  
> -	for_each_intel_crtc(dev, intel_crtc) {
> -		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> +	memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
> +	       sizeof(intel_state->min_pixclk));
>  
> -		if (!crtc_state->base.enable)
> -			continue;
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		int pixclk = 0;
>  
> -		max_pixclk = max(max_pixclk,
> -				 crtc_state->base.adjusted_mode.crtc_clock);
> +		if (crtc_state->enable)
> +			pixclk = crtc_state->adjusted_mode.crtc_clock;
> +
> +		intel_state->min_pixclk[i] = pixclk;
>  	}
>  
> -	return max_pixclk;
> +	if (!intel_state->active_crtcs)
> +		return 0;
> +
> +	for_each_pipe(dev_priv, pipe)
> +		max_pixel_rate = max(intel_state->min_pixclk[pipe], max_pixel_rate);
> +
> +	return max_pixel_rate;
>  }
>  
>  static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
> @@ -6255,6 +6270,9 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
>  	for_each_power_domain(domain, domains)
>  		intel_display_power_put(dev_priv, domain);
>  	intel_crtc->enabled_power_domains = 0;
> +
> +	dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
> +	dev_priv->min_pixclk[intel_crtc->pipe] = 0;
>  }
>  
>  /*
> @@ -9490,29 +9508,39 @@ static void broxton_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  /* compute the max rate for new configuration */
>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  {
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int max_pixel_rate = 0;
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	struct drm_i915_private *dev_priv = state->dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	unsigned max_pixel_rate = 0, i;
> +	enum pipe pipe;
>  
> -	for_each_intel_crtc(state->dev, intel_crtc) {
> -		int pixel_rate;
> +	memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
> +	       sizeof(intel_state->min_pixclk));
>  
> -		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		int pixclk = 0;
>  
> -		if (!crtc_state->base.enable)
> -			continue;
> +		if (crtc_state->enable) {
> +			struct intel_crtc_state *intel_crtc_state =
> +				to_intel_crtc_state(crtc_state);
>  
> -		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> +			pixclk = ilk_pipe_pixel_rate(intel_crtc_state);

Could you avoid the reindent and renames please?

>  
> -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> -		if (IS_BROADWELL(state->dev) && crtc_state->ips_enabled)
> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +			/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +			if (IS_BROADWELL(dev_priv) && intel_crtc_state->ips_enabled)
> +				pixclk = DIV_ROUND_UP(pixclk * 100, 95);
> +		}
>  
> -		max_pixel_rate = max(max_pixel_rate, pixel_rate);
> +		intel_state->min_pixclk[i] = pixclk;
>  	}
>  
> +	if (!intel_state->active_crtcs)
> +		return 0;
> +
> +	for_each_pipe(dev_priv, pipe)
> +		max_pixel_rate = max(intel_state->min_pixclk[pipe], max_pixel_rate);
> +
>  	return max_pixel_rate;
>  }
>  
> @@ -12977,15 +13005,27 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state)
>  
>  static int intel_modeset_checks(struct drm_atomic_state *state)
>  {
> -	struct drm_device *dev = state->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int ret;
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	struct drm_i915_private *dev_priv = state->dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int ret = 0, i;
>  
>  	if (!check_digital_port_conflicts(state)) {
>  		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
>  		return -EINVAL;
>  	}
>  
> +	intel_state->modeset = true;
> +	intel_state->active_crtcs = dev_priv->active_crtcs;
> +
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (crtc_state->active)
> +			intel_state->active_crtcs |= 1 << i;
> +		else
> +			intel_state->active_crtcs &= ~(1 << i);
> +	}
> +
>  	/*
>  	 * See if the config requires any additional preparation, e.g.
>  	 * to adjust global state with pipes off.  We need to do this
> @@ -13009,7 +13049,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>  
>  	intel_modeset_clear_plls(state);
>  
> -	if (IS_HASWELL(dev))
> +	if (IS_HASWELL(dev_priv))
>  		return haswell_mode_set_planes_workaround(state);
>  
>  	return 0;
> @@ -13179,12 +13219,13 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			       struct drm_atomic_state *state,
>  			       bool async)
>  {
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_crtc *crtc;
>  	int ret = 0;
>  	int i;
> -	bool any_ms = false;
> +	bool hw_check = intel_state->modeset;
>  
>  	ret = intel_atomic_prepare_commit(dev, state, async);
>  	if (ret) {
> @@ -13194,13 +13235,18 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  	drm_atomic_helper_swap_state(dev, state);
>  
> +	if (intel_state->modeset) {
> +		memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
> +		       sizeof(intel_state->min_pixclk));
> +		dev_priv->active_crtcs = intel_state->active_crtcs;
> +	}
> +
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
>  		if (!needs_modeset(crtc->state))
>  			continue;
>  
> -		any_ms = true;
>  		intel_pre_plane_update(intel_crtc);
>  
>  		if (crtc_state->active) {
> @@ -13215,7 +13261,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	 * update the the output configuration. */
>  	intel_modeset_update_crtc_state(state);
>  
> -	if (any_ms) {
> +	if (intel_state->modeset) {
>  		intel_shared_dpll_commit(state);
>  
>  		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
> @@ -13239,7 +13285,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			put_domains = modeset_get_crtc_power_domains(crtc);
>  
>  			/* make sure intel_modeset_check_state runs */
> -			any_ms = true;
> +			hw_check = true;
>  		}
>  
>  		if (!modeset)
> @@ -13263,7 +13309,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	if (any_ms)
> +	if (hw_check)
>  		intel_modeset_check_state(dev, state);
>  
>  	drm_atomic_state_free(state);
> @@ -15263,16 +15309,36 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	struct intel_connector *connector;
>  	int i;
>  
> +	dev_priv->active_crtcs = 0;
> +
>  	for_each_intel_crtc(dev, crtc) {
> -		__drm_atomic_helper_crtc_destroy_state(&crtc->base, crtc->base.state);
> -		memset(crtc->config, 0, sizeof(*crtc->config));
> -		crtc->config->base.crtc = &crtc->base;
> +		struct intel_crtc_state *crtc_state = crtc->config;
> +		int pixclk = 0;
>  
> -		crtc->active = dev_priv->display.get_pipe_config(crtc,
> -								 crtc->config);
> +		__drm_atomic_helper_crtc_destroy_state(&crtc->base, &crtc_state->base);
> +		memset(crtc_state, 0, sizeof(*crtc_state));
> +		crtc_state->base.crtc = &crtc->base;
>  
> -		crtc->base.state->active = crtc->active;
> -		crtc->base.enabled = crtc->active;
> +		crtc_state->base.active = crtc_state->base.enable =
> +			dev_priv->display.get_pipe_config(crtc, crtc_state);
> +
> +		crtc->base.enabled = crtc_state->base.enable;
> +		crtc->active = crtc_state->base.active;
> +
> +		if (crtc_state->base.active) {
> +			dev_priv->active_crtcs |= 1 << crtc->pipe;
> +
> +			if (IS_BROADWELL(dev_priv)) {
> +				pixclk = ilk_pipe_pixel_rate(crtc_state);
> +
> +				/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +				if (crtc_state->ips_enabled)
> +					pixclk = DIV_ROUND_UP(pixclk * 100, 95);
> +			} else if (IS_BROXTON(dev_priv) || IS_VALLEYVIEW(dev_priv))
> +				pixclk = crtc_state->base.adjusted_mode.crtc_clock;
> +		}
> +
> +		dev_priv->min_pixclk[crtc->pipe] = pixclk;
>  
>  		readout_plane_state(crtc);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 682f70f95e17..578afc7ac4bd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -248,7 +248,12 @@ struct intel_atomic_state {
>  	struct drm_atomic_state base;
>  
>  	unsigned int cdclk;
> -	bool dpll_set;
> +
> +	bool dpll_set, modeset;
> +
> +	unsigned int active_crtcs;
> +	unsigned int min_pixclk[I915_MAX_PIPES];
> +
>  	struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
>  };
>  
> -- 
> 2.1.0

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux