diff mbox

[17/19] drm/i915: Convert intel_pipe_will_have_type() to using atomic state

Message ID 1426240142-24538-18-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira March 13, 2015, 9:49 a.m. UTC
Pass a crtc_state to it and find whether the pipe has an encoder of a
given type by looking at the drm_atomic_state the crtc_state points to.

Until recently i9xx_get_refclk() used to be called indirectly from
vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
converted to be part of a full drm atomic state, so add a WARN in case
someone decides to call that again with such a dummy state.

v2: Warn if there is no connectors for a given crtc. (Daniel)
    Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 133 +++++++++++++++++++++--------------
 2 files changed, 83 insertions(+), 52 deletions(-)

Comments

Chandra Konduru March 19, 2015, 7:24 p.m. UTC | #1
> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Friday, March 13, 2015 2:49 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Konduru, Chandra; Conselvan De Oliveira, Ander
> Subject: [PATCH 17/19] drm/i915: Convert intel_pipe_will_have_type() to using
> atomic state
> 
> Pass a crtc_state to it and find whether the pipe has an encoder of a given type
> by looking at the drm_atomic_state the crtc_state points to.
> 
> Until recently i9xx_get_refclk() used to be called indirectly from
> vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
> converted to be part of a full drm atomic state, so add a WARN in case someone
> decides to call that again with such a dummy state.
> 
> v2: Warn if there is no connectors for a given crtc. (Daniel)
>     Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)
> 
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
>  drivers/gpu/drm/i915/intel_display.c | 133 +++++++++++++++++++++-------------
> -
>  2 files changed, 83 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 490ab8c..576e101 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
>  	 * Returns true on success, false on failure.
>  	 */
>  	bool (*find_dpll)(const struct intel_limit *limit,
> -			  struct intel_crtc *crtc,
> +			  struct intel_crtc_state *crtc_state,
>  			  int target, int refclk,
>  			  struct dpll *match_clock,
>  			  struct dpll *best_clock);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8c97186..be1979e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc *crtc,
> enum intel_output_type type)
>   * intel_pipe_has_type() but looking at encoder->new_crtc instead of
>   * encoder->crtc.
>   */
> -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
> +static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> +				      int type)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> +	int i, num_connectors = 0;
> +
> +	for (i = 0; i < state->num_connector; i++) {
> +		if (!state->connectors[i])
> +			continue;
> +
> +		connector_state = state->connector_states[i];
> +		if (connector_state->crtc != crtc_state->base.crtc)
> +			continue;
> +
> +		num_connectors++;

Above piece of code to loop through state->connectors[] is present in
multiple places, can you have a helper something like below to save
some typing and code reduction?

struct connector_state * intel_crtc_state_connector(crtc_state, crtc);

also similarily for encoder
struct encoder_state *intel_crtc_state_encoder(crtc_state, crtc);

> 
> -	for_each_intel_encoder(dev, encoder)
> -		if (encoder->new_crtc == crtc && encoder->type == type)
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +		if (encoder->type == type)
>  			return true;
> +	}
> +
> +	WARN_ON(num_connectors == 0);
> 
>  	return false;
>  }
> 
> -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
> -						int refclk)
> +static const intel_limit_t *
> +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev)) {
>  			if (refclk == 100000)
>  				limit = &intel_limits_ironlake_dual_lvds_100m;
> @@ -467,20 +483,21 @@ static const intel_limit_t *intel_ironlake_limit(struct
> intel_crtc *crtc,
>  	return limit;
>  }
> 
> -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
> +static const intel_limit_t *
> +intel_g4x_limit(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev))
>  			limit = &intel_limits_g4x_dual_channel_lvds;
>  		else
>  			limit = &intel_limits_g4x_single_channel_lvds;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
> -		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI)
> ||
> +		   intel_pipe_will_have_type(crtc_state,
> INTEL_OUTPUT_ANALOG)) {
>  		limit = &intel_limits_g4x_hdmi;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
>  		limit = &intel_limits_g4x_sdvo;
>  	} else /* The option is for other outputs */
>  		limit = &intel_limits_i9xx_sdvo;
> @@ -488,17 +505,18 @@ static const intel_limit_t *intel_g4x_limit(struct
> intel_crtc *crtc)
>  	return limit;
>  }
> 
> -static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
> +static const intel_limit_t *
> +intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
> 
>  	if (HAS_PCH_SPLIT(dev))
> -		limit = intel_ironlake_limit(crtc, refclk);
> +		limit = intel_ironlake_limit(crtc_state, refclk);
>  	else if (IS_G4X(dev)) {
> -		limit = intel_g4x_limit(crtc);
> +		limit = intel_g4x_limit(crtc_state);
>  	} else if (IS_PINEVIEW(dev)) {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state,
> INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_pineview_lvds;
>  		else
>  			limit = &intel_limits_pineview_sdvo; @@ -507,14
> +525,14 @@ static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int
> refclk)
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		limit = &intel_limits_vlv;
>  	} else if (!IS_GEN2(dev)) {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state,
> INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i9xx_lvds;
>  		else
>  			limit = &intel_limits_i9xx_sdvo;
>  	} else {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state,
> INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i8xx_lvds;
> -		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
> +		else if (intel_pipe_will_have_type(crtc_state,
> INTEL_OUTPUT_DVO))
>  			limit = &intel_limits_i8xx_dvo;
>  		else
>  			limit = &intel_limits_i8xx_dac;
> @@ -601,15 +619,17 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
> }
> 
>  static bool
> -i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +i9xx_find_best_dpll(const intel_limit_t *limit,
> +		    struct intel_crtc_state *crtc_state,
>  		    int target, int refclk, intel_clock_t *match_clock,
>  		    intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int err = target;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
>  		 * For LVDS just rely on its current settings for dual-channel.
>  		 * We haven't figured out how to reliably set up different @@ -
> 662,15 +682,17 @@ i9xx_find_best_dpll(const intel_limit_t *limit, struct
> intel_crtc *crtc,  }
> 
>  static bool
> -pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +pnv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int err = target;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
>  		 * For LVDS just rely on its current settings for dual-channel.
>  		 * We haven't figured out how to reliably set up different @@ -
> 721,10 +743,12 @@ pnv_find_best_dpll(const intel_limit_t *limit, struct
> intel_crtc *crtc,  }
> 
>  static bool
> -g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +g4x_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int max_n;
> @@ -733,7 +757,7 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct
> intel_crtc *crtc,
>  	int err_most = (target >> 8) + (target >> 9);
>  	found = false;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev))
>  			clock.p2 = limit->p2.p2_fast;
>  		else
> @@ -778,10 +802,12 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct
> intel_crtc *crtc,  }
> 
>  static bool
> -vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +vlv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	unsigned int bestppm = 1000000;
> @@ -835,10 +861,12 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct
> intel_crtc *crtc,  }
> 
>  static bool
> -chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +chv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	uint64_t m2;
> @@ -5726,7 +5754,7 @@ static int intel_crtc_compute_config(struct intel_crtc
> *crtc,
>  	 * - LVDS dual channel mode
>  	 * - Double wide pipe
>  	 */
> -	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
>  	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
>  		pipe_config->pipe_src_w &= ~1;
> 
> @@ -5905,15 +5933,18 @@ static inline bool intel_panel_use_ssc(struct
> drm_i915_private *dev_priv)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);  }
> 
> -static int i9xx_get_refclk(struct intel_crtc *crtc, int num_connectors)
> +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> +			   int num_connectors)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int refclk;
> 
> +	WARN_ON(!crtc_state->base.state);
> +
>  	if (IS_VALLEYVIEW(dev)) {
>  		refclk = 100000;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)
> &&
>  	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
>  		refclk = dev_priv->vbt.lvds_ssc_freq;
>  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> refclk); @@ -5956,7 +5987,7 @@ static void i9xx_update_pll_dividers(struct
> intel_crtc *crtc,
>  	crtc_state->dpll_hw_state.fp0 = fp;
> 
>  	crtc->lowfreq_avail = false;
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  	    reduced_clock && i915.powersave) {
>  		crtc_state->dpll_hw_state.fp1 = fp2;
>  		crtc->lowfreq_avail = true;
> @@ -6314,6 +6345,7 @@ void vlv_force_pll_on(struct drm_device *dev, enum
> pipe pipe,
>  	struct intel_crtc *crtc =
>  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
>  	struct intel_crtc_state pipe_config = {
> +		.base.crtc = &crtc->base,
>  		.pixel_multiplier = 1,
>  		.dpll = *dpll,
>  	};
> @@ -6358,12 +6390,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
> 
>  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
> 
> -	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)
> ||
> +		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
> 
>  	dpll = DPLL_VGA_MODE_DIS;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  		dpll |= DPLLB_MODE_LVDS;
>  	else
>  		dpll |= DPLLB_MODE_DAC_SERIAL;
> @@ -6406,7 +6438,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
> 
>  	if (crtc_state->sdvo_tv_clock)
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> -	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -6436,7 +6468,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
> 
>  	dpll = DPLL_VGA_MODE_DIS;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		dpll |= (1 << (clock->p1 - 1)) <<
> DPLL_FPA01_P1_POST_DIV_SHIFT;
>  	} else {
>  		if (clock->p1 == 2)
> @@ -6447,10 +6479,10 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
> 
> -	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc,
> INTEL_OUTPUT_DVO))
> +	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state,
> +INTEL_OUTPUT_DVO))
>  		dpll |= DPLL_DVO_2X_MODE;
> 
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -6687,7 +6719,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc
> *crtc,
>  		return 0;
> 
>  	if (!crtc_state->clock_set) {
> -		refclk = i9xx_get_refclk(crtc, num_connectors);
> +		refclk = i9xx_get_refclk(crtc_state, num_connectors);
> 
>  		/*
>  		 * Returns a set of divisors for the desired target clock with @@
> -6695,8 +6727,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
>  		 * 2) / p1 / p2.
>  		 */
> -		limit = intel_limit(crtc, refclk);
> -		ok = dev_priv->display.find_dpll(limit, crtc,
> +		limit = intel_limit(crtc_state, refclk);
> +		ok = dev_priv->display.find_dpll(limit, crtc_state,
>  						 crtc_state->port_clock,
>  						 refclk, NULL, &clock);
>  		if (!ok) {
> @@ -6712,7 +6744,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc
> *crtc,
>  			 * we will disable the LVDS downclock feature.
>  			 */
>  			has_reduced_clock =
> -				dev_priv->display.find_dpll(limit, crtc,
> +				dev_priv->display.find_dpll(limit, crtc_state,
>  							    dev_priv-
> >lvds_downclock,
>  							    refclk, &clock,
>  							    &reduced_clock);
> @@ -7540,12 +7572,11 @@ static bool ironlake_compute_clocks(struct
> drm_crtc *crtc,  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int refclk;
>  	const intel_limit_t *limit;
>  	bool ret, is_lvds = false;
> 
> -	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);
> +	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
> 
>  	refclk = ironlake_get_refclk(crtc);
> 
> @@ -7554,8 +7585,8 @@ static bool ironlake_compute_clocks(struct drm_crtc
> *crtc,
>  	 * refclk, or FALSE.  The returned values represent the clock equation:
>  	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
>  	 */
> -	limit = intel_limit(intel_crtc, refclk);
> -	ret = dev_priv->display.find_dpll(limit, intel_crtc,
> +	limit = intel_limit(crtc_state, refclk);
> +	ret = dev_priv->display.find_dpll(limit, crtc_state,
>  					  crtc_state->port_clock,
>  					  refclk, NULL, clock);
>  	if (!ret)
> @@ -7569,7 +7600,7 @@ static bool ironlake_compute_clocks(struct drm_crtc
> *crtc,
>  		 * downclock feature.
>  		*/
>  		*has_reduced_clock =
> -			dev_priv->display.find_dpll(limit, intel_crtc,
> +			dev_priv->display.find_dpll(limit, crtc_state,
>  						    dev_priv->lvds_downclock,
>  						    refclk, clock,
>  						    reduced_clock);
> --
> 2.1.0
Ander Conselvan de Oliveira March 20, 2015, 6:28 a.m. UTC | #2
On Thu, 2015-03-19 at 19:24 +0000, Konduru, Chandra wrote:
> 
> > -----Original Message-----
> > From: Conselvan De Oliveira, Ander
> > Sent: Friday, March 13, 2015 2:49 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Konduru, Chandra; Conselvan De Oliveira, Ander
> > Subject: [PATCH 17/19] drm/i915: Convert intel_pipe_will_have_type() to using
> > atomic state
> > 
> > Pass a crtc_state to it and find whether the pipe has an encoder of a given type
> > by looking at the drm_atomic_state the crtc_state points to.
> > 
> > Until recently i9xx_get_refclk() used to be called indirectly from
> > vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
> > converted to be part of a full drm atomic state, so add a WARN in case someone
> > decides to call that again with such a dummy state.
> > 
> > v2: Warn if there is no connectors for a given crtc. (Daniel)
> >     Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)
> > 
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 133 +++++++++++++++++++++-------------
> > -
> >  2 files changed, 83 insertions(+), 52 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 490ab8c..576e101 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
> >  	 * Returns true on success, false on failure.
> >  	 */
> >  	bool (*find_dpll)(const struct intel_limit *limit,
> > -			  struct intel_crtc *crtc,
> > +			  struct intel_crtc_state *crtc_state,
> >  			  int target, int refclk,
> >  			  struct dpll *match_clock,
> >  			  struct dpll *best_clock);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 8c97186..be1979e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc *crtc,
> > enum intel_output_type type)
> >   * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> >   * encoder->crtc.
> >   */
> > -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
> > +static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> > +				      int type)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_atomic_state *state = crtc_state->base.state;
> > +	struct drm_connector_state *connector_state;
> >  	struct intel_encoder *encoder;
> > +	int i, num_connectors = 0;
> > +
> > +	for (i = 0; i < state->num_connector; i++) {
> > +		if (!state->connectors[i])
> > +			continue;
> > +
> > +		connector_state = state->connector_states[i];
> > +		if (connector_state->crtc != crtc_state->base.crtc)
> > +			continue;
> > +
> > +		num_connectors++;
> 
> Above piece of code to loop through state->connectors[] is present in
> multiple places, can you have a helper something like below to save
> some typing and code reduction?
> 
> struct connector_state * intel_crtc_state_connector(crtc_state, crtc);
> 
> also similarily for encoder
> struct encoder_state *intel_crtc_state_encoder(crtc_state, crtc);

This wouldn't work for cloned setups, where we have more than one
connector/encoder in a single pipe. 

Ander

> > 
> > -	for_each_intel_encoder(dev, encoder)
> > -		if (encoder->new_crtc == crtc && encoder->type == type)
> > +		encoder = to_intel_encoder(connector_state->best_encoder);
> > +		if (encoder->type == type)
> >  			return true;
> > +	}
> > +
> > +	WARN_ON(num_connectors == 0);
> > 
> >  	return false;
> >  }
> > 
> > -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
> > -						int refclk)
> > +static const intel_limit_t *
> > +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	const intel_limit_t *limit;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		if (intel_is_dual_link_lvds(dev)) {
> >  			if (refclk == 100000)
> >  				limit = &intel_limits_ironlake_dual_lvds_100m;
> > @@ -467,20 +483,21 @@ static const intel_limit_t *intel_ironlake_limit(struct
> > intel_crtc *crtc,
> >  	return limit;
> >  }
> > 
> > -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
> > +static const intel_limit_t *
> > +intel_g4x_limit(struct intel_crtc_state *crtc_state)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	const intel_limit_t *limit;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		if (intel_is_dual_link_lvds(dev))
> >  			limit = &intel_limits_g4x_dual_channel_lvds;
> >  		else
> >  			limit = &intel_limits_g4x_single_channel_lvds;
> > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
> > -		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
> > +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI)
> > ||
> > +		   intel_pipe_will_have_type(crtc_state,
> > INTEL_OUTPUT_ANALOG)) {
> >  		limit = &intel_limits_g4x_hdmi;
> > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
> > +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
> >  		limit = &intel_limits_g4x_sdvo;
> >  	} else /* The option is for other outputs */
> >  		limit = &intel_limits_i9xx_sdvo;
> > @@ -488,17 +505,18 @@ static const intel_limit_t *intel_g4x_limit(struct
> > intel_crtc *crtc)
> >  	return limit;
> >  }
> > 
> > -static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
> > +static const intel_limit_t *
> > +intel_limit(struct intel_crtc_state *crtc_state, int refclk)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	const intel_limit_t *limit;
> > 
> >  	if (HAS_PCH_SPLIT(dev))
> > -		limit = intel_ironlake_limit(crtc, refclk);
> > +		limit = intel_ironlake_limit(crtc_state, refclk);
> >  	else if (IS_G4X(dev)) {
> > -		limit = intel_g4x_limit(crtc);
> > +		limit = intel_g4x_limit(crtc_state);
> >  	} else if (IS_PINEVIEW(dev)) {
> > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> > +		if (intel_pipe_will_have_type(crtc_state,
> > INTEL_OUTPUT_LVDS))
> >  			limit = &intel_limits_pineview_lvds;
> >  		else
> >  			limit = &intel_limits_pineview_sdvo; @@ -507,14
> > +525,14 @@ static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int
> > refclk)
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		limit = &intel_limits_vlv;
> >  	} else if (!IS_GEN2(dev)) {
> > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> > +		if (intel_pipe_will_have_type(crtc_state,
> > INTEL_OUTPUT_LVDS))
> >  			limit = &intel_limits_i9xx_lvds;
> >  		else
> >  			limit = &intel_limits_i9xx_sdvo;
> >  	} else {
> > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> > +		if (intel_pipe_will_have_type(crtc_state,
> > INTEL_OUTPUT_LVDS))
> >  			limit = &intel_limits_i8xx_lvds;
> > -		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
> > +		else if (intel_pipe_will_have_type(crtc_state,
> > INTEL_OUTPUT_DVO))
> >  			limit = &intel_limits_i8xx_dvo;
> >  		else
> >  			limit = &intel_limits_i8xx_dac;
> > @@ -601,15 +619,17 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
> > }
> > 
> >  static bool
> > -i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> > +i9xx_find_best_dpll(const intel_limit_t *limit,
> > +		    struct intel_crtc_state *crtc_state,
> >  		    int target, int refclk, intel_clock_t *match_clock,
> >  		    intel_clock_t *best_clock)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	intel_clock_t clock;
> >  	int err = target;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		/*
> >  		 * For LVDS just rely on its current settings for dual-channel.
> >  		 * We haven't figured out how to reliably set up different @@ -
> > 662,15 +682,17 @@ i9xx_find_best_dpll(const intel_limit_t *limit, struct
> > intel_crtc *crtc,  }
> > 
> >  static bool
> > -pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> > +pnv_find_best_dpll(const intel_limit_t *limit,
> > +		   struct intel_crtc_state *crtc_state,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	intel_clock_t clock;
> >  	int err = target;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		/*
> >  		 * For LVDS just rely on its current settings for dual-channel.
> >  		 * We haven't figured out how to reliably set up different @@ -
> > 721,10 +743,12 @@ pnv_find_best_dpll(const intel_limit_t *limit, struct
> > intel_crtc *crtc,  }
> > 
> >  static bool
> > -g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> > +g4x_find_best_dpll(const intel_limit_t *limit,
> > +		   struct intel_crtc_state *crtc_state,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	intel_clock_t clock;
> >  	int max_n;
> > @@ -733,7 +757,7 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct
> > intel_crtc *crtc,
> >  	int err_most = (target >> 8) + (target >> 9);
> >  	found = false;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		if (intel_is_dual_link_lvds(dev))
> >  			clock.p2 = limit->p2.p2_fast;
> >  		else
> > @@ -778,10 +802,12 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct
> > intel_crtc *crtc,  }
> > 
> >  static bool
> > -vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> > +vlv_find_best_dpll(const intel_limit_t *limit,
> > +		   struct intel_crtc_state *crtc_state,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	intel_clock_t clock;
> >  	unsigned int bestppm = 1000000;
> > @@ -835,10 +861,12 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct
> > intel_crtc *crtc,  }
> > 
> >  static bool
> > -chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> > +chv_find_best_dpll(const intel_limit_t *limit,
> > +		   struct intel_crtc_state *crtc_state,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	intel_clock_t clock;
> >  	uint64_t m2;
> > @@ -5726,7 +5754,7 @@ static int intel_crtc_compute_config(struct intel_crtc
> > *crtc,
> >  	 * - LVDS dual channel mode
> >  	 * - Double wide pipe
> >  	 */
> > -	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> > +	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
> >  	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
> >  		pipe_config->pipe_src_w &= ~1;
> > 
> > @@ -5905,15 +5933,18 @@ static inline bool intel_panel_use_ssc(struct
> > drm_i915_private *dev_priv)
> >  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);  }
> > 
> > -static int i9xx_get_refclk(struct intel_crtc *crtc, int num_connectors)
> > +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> > +			   int num_connectors)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_device *dev = crtc_state->base.crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int refclk;
> > 
> > +	WARN_ON(!crtc_state->base.state);
> > +
> >  	if (IS_VALLEYVIEW(dev)) {
> >  		refclk = 100000;
> > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> > +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)
> > &&
> >  	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
> >  		refclk = dev_priv->vbt.lvds_ssc_freq;
> >  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> > refclk); @@ -5956,7 +5987,7 @@ static void i9xx_update_pll_dividers(struct
> > intel_crtc *crtc,
> >  	crtc_state->dpll_hw_state.fp0 = fp;
> > 
> >  	crtc->lowfreq_avail = false;
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> >  	    reduced_clock && i915.powersave) {
> >  		crtc_state->dpll_hw_state.fp1 = fp2;
> >  		crtc->lowfreq_avail = true;
> > @@ -6314,6 +6345,7 @@ void vlv_force_pll_on(struct drm_device *dev, enum
> > pipe pipe,
> >  	struct intel_crtc *crtc =
> >  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> >  	struct intel_crtc_state pipe_config = {
> > +		.base.crtc = &crtc->base,
> >  		.pixel_multiplier = 1,
> >  		.dpll = *dpll,
> >  	};
> > @@ -6358,12 +6390,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
> > 
> >  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
> > 
> > -	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||
> > -		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);
> > +	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)
> > ||
> > +		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
> > 
> >  	dpll = DPLL_VGA_MODE_DIS;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
> >  		dpll |= DPLLB_MODE_LVDS;
> >  	else
> >  		dpll |= DPLLB_MODE_DAC_SERIAL;
> > @@ -6406,7 +6438,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
> > 
> >  	if (crtc_state->sdvo_tv_clock)
> >  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> > -	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> > +	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> >  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> >  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> >  	else
> > @@ -6436,7 +6468,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
> > 
> >  	dpll = DPLL_VGA_MODE_DIS;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> >  		dpll |= (1 << (clock->p1 - 1)) <<
> > DPLL_FPA01_P1_POST_DIV_SHIFT;
> >  	} else {
> >  		if (clock->p1 == 2)
> > @@ -6447,10 +6479,10 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
> >  			dpll |= PLL_P2_DIVIDE_BY_4;
> >  	}
> > 
> > -	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc,
> > INTEL_OUTPUT_DVO))
> > +	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state,
> > +INTEL_OUTPUT_DVO))
> >  		dpll |= DPLL_DVO_2X_MODE;
> > 
> > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> >  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> >  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> >  	else
> > @@ -6687,7 +6719,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc
> > *crtc,
> >  		return 0;
> > 
> >  	if (!crtc_state->clock_set) {
> > -		refclk = i9xx_get_refclk(crtc, num_connectors);
> > +		refclk = i9xx_get_refclk(crtc_state, num_connectors);
> > 
> >  		/*
> >  		 * Returns a set of divisors for the desired target clock with @@
> > -6695,8 +6727,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> >  		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
> >  		 * 2) / p1 / p2.
> >  		 */
> > -		limit = intel_limit(crtc, refclk);
> > -		ok = dev_priv->display.find_dpll(limit, crtc,
> > +		limit = intel_limit(crtc_state, refclk);
> > +		ok = dev_priv->display.find_dpll(limit, crtc_state,
> >  						 crtc_state->port_clock,
> >  						 refclk, NULL, &clock);
> >  		if (!ok) {
> > @@ -6712,7 +6744,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc
> > *crtc,
> >  			 * we will disable the LVDS downclock feature.
> >  			 */
> >  			has_reduced_clock =
> > -				dev_priv->display.find_dpll(limit, crtc,
> > +				dev_priv->display.find_dpll(limit, crtc_state,
> >  							    dev_priv-
> > >lvds_downclock,
> >  							    refclk, &clock,
> >  							    &reduced_clock);
> > @@ -7540,12 +7572,11 @@ static bool ironlake_compute_clocks(struct
> > drm_crtc *crtc,  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	int refclk;
> >  	const intel_limit_t *limit;
> >  	bool ret, is_lvds = false;
> > 
> > -	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);
> > +	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
> > 
> >  	refclk = ironlake_get_refclk(crtc);
> > 
> > @@ -7554,8 +7585,8 @@ static bool ironlake_compute_clocks(struct drm_crtc
> > *crtc,
> >  	 * refclk, or FALSE.  The returned values represent the clock equation:
> >  	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> >  	 */
> > -	limit = intel_limit(intel_crtc, refclk);
> > -	ret = dev_priv->display.find_dpll(limit, intel_crtc,
> > +	limit = intel_limit(crtc_state, refclk);
> > +	ret = dev_priv->display.find_dpll(limit, crtc_state,
> >  					  crtc_state->port_clock,
> >  					  refclk, NULL, clock);
> >  	if (!ret)
> > @@ -7569,7 +7600,7 @@ static bool ironlake_compute_clocks(struct drm_crtc
> > *crtc,
> >  		 * downclock feature.
> >  		*/
> >  		*has_reduced_clock =
> > -			dev_priv->display.find_dpll(limit, intel_crtc,
> > +			dev_priv->display.find_dpll(limit, crtc_state,
> >  						    dev_priv->lvds_downclock,
> >  						    refclk, clock,
> >  						    reduced_clock);
> > --
> > 2.1.0
>
Chandra Konduru March 22, 2015, 4:14 p.m. UTC | #3
> -----Original Message-----

> From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]

> Sent: Thursday, March 19, 2015 11:28 PM

> To: Konduru, Chandra

> Cc: intel-gfx@lists.freedesktop.org

> Subject: Re: [PATCH 17/19] drm/i915: Convert intel_pipe_will_have_type() to

> using atomic state

> 

> On Thu, 2015-03-19 at 19:24 +0000, Konduru, Chandra wrote:

> >

> > > -----Original Message-----

> > > From: Conselvan De Oliveira, Ander

> > > Sent: Friday, March 13, 2015 2:49 AM

> > > To: intel-gfx@lists.freedesktop.org

> > > Cc: Konduru, Chandra; Conselvan De Oliveira, Ander

> > > Subject: [PATCH 17/19] drm/i915: Convert intel_pipe_will_have_type()

> > > to using atomic state

> > >

> > > Pass a crtc_state to it and find whether the pipe has an encoder of

> > > a given type by looking at the drm_atomic_state the crtc_state points to.

> > >

> > > Until recently i9xx_get_refclk() used to be called indirectly from

> > > vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is

> > > not converted to be part of a full drm atomic state, so add a WARN

> > > in case someone decides to call that again with such a dummy state.

> > >

> > > v2: Warn if there is no connectors for a given crtc. (Daniel)

> > >     Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)

> > >

> > > Signed-off-by: Ander Conselvan de Oliveira

> > > <ander.conselvan.de.oliveira@intel.com>

> > > ---

> > >  drivers/gpu/drm/i915/i915_drv.h      |   2 +-

> > >  drivers/gpu/drm/i915/intel_display.c | 133

> > > +++++++++++++++++++++-------------

> > > -

> > >  2 files changed, 83 insertions(+), 52 deletions(-)

> > >

> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h

> > > b/drivers/gpu/drm/i915/i915_drv.h index 490ab8c..576e101 100644

> > > --- a/drivers/gpu/drm/i915/i915_drv.h

> > > +++ b/drivers/gpu/drm/i915/i915_drv.h

> > > @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {

> > >  	 * Returns true on success, false on failure.

> > >  	 */

> > >  	bool (*find_dpll)(const struct intel_limit *limit,

> > > -			  struct intel_crtc *crtc,

> > > +			  struct intel_crtc_state *crtc_state,

> > >  			  int target, int refclk,

> > >  			  struct dpll *match_clock,

> > >  			  struct dpll *best_clock);

> > > diff --git a/drivers/gpu/drm/i915/intel_display.c

> > > b/drivers/gpu/drm/i915/intel_display.c

> > > index 8c97186..be1979e 100644

> > > --- a/drivers/gpu/drm/i915/intel_display.c

> > > +++ b/drivers/gpu/drm/i915/intel_display.c

> > > @@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc

> > > *crtc, enum intel_output_type type)

> > >   * intel_pipe_has_type() but looking at encoder->new_crtc instead of

> > >   * encoder->crtc.

> > >   */

> > > -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int

> > > type)

> > > +static bool intel_pipe_will_have_type(const struct intel_crtc_state

> *crtc_state,

> > > +				      int type)

> > >  {

> > > -	struct drm_device *dev = crtc->base.dev;

> > > +	struct drm_atomic_state *state = crtc_state->base.state;

> > > +	struct drm_connector_state *connector_state;

> > >  	struct intel_encoder *encoder;

> > > +	int i, num_connectors = 0;

> > > +

> > > +	for (i = 0; i < state->num_connector; i++) {

> > > +		if (!state->connectors[i])

> > > +			continue;

> > > +

> > > +		connector_state = state->connector_states[i];

> > > +		if (connector_state->crtc != crtc_state->base.crtc)

> > > +			continue;

> > > +

> > > +		num_connectors++;

> >

> > Above piece of code to loop through state->connectors[] is present in

> > multiple places, can you have a helper something like below to save

> > some typing and code reduction?

> >

> > struct connector_state * intel_crtc_state_connector(crtc_state, crtc);

> >

> > also similarily for encoder

> > struct encoder_state *intel_crtc_state_encoder(crtc_state, crtc);

> 

> This wouldn't work for cloned setups, where we have more than one

> connector/encoder in a single pipe.


May be it is for now. But as part of full atomic crtc, I think clone 
setup should be taken care by having this called from respective
crtcs and its crtc_states.

> 

> Ander

> 

> > >

> > > -	for_each_intel_encoder(dev, encoder)

> > > -		if (encoder->new_crtc == crtc && encoder->type == type)

> > > +		encoder = to_intel_encoder(connector_state->best_encoder);

> > > +		if (encoder->type == type)

> > >  			return true;

> > > +	}

> > > +

> > > +	WARN_ON(num_connectors == 0);

> > >

> > >  	return false;

> > >  }

> > >

> > > -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,

> > > -						int refclk)

> > > +static const intel_limit_t *

> > > +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int

> > > +refclk)

> > >  {

> > > -	struct drm_device *dev = crtc->base.dev;

> > > +	struct drm_device *dev = crtc_state->base.crtc->dev;

> > >  	const intel_limit_t *limit;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		if (intel_is_dual_link_lvds(dev)) {

> > >  			if (refclk == 100000)

> > >  				limit = &intel_limits_ironlake_dual_lvds_100m;

> > > @@ -467,20 +483,21 @@ static const intel_limit_t

> > > *intel_ironlake_limit(struct intel_crtc *crtc,

> > >  	return limit;

> > >  }

> > >

> > > -static const intel_limit_t *intel_g4x_limit(struct intel_crtc

> > > *crtc)

> > > +static const intel_limit_t *

> > > +intel_g4x_limit(struct intel_crtc_state *crtc_state)

> > >  {

> > > -	struct drm_device *dev = crtc->base.dev;

> > > +	struct drm_device *dev = crtc_state->base.crtc->dev;

> > >  	const intel_limit_t *limit;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		if (intel_is_dual_link_lvds(dev))

> > >  			limit = &intel_limits_g4x_dual_channel_lvds;

> > >  		else

> > >  			limit = &intel_limits_g4x_single_channel_lvds;

> > > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||

> > > -		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {

> > > +	} else if (intel_pipe_will_have_type(crtc_state,

> > > +INTEL_OUTPUT_HDMI)

> > > ||

> > > +		   intel_pipe_will_have_type(crtc_state,

> > > INTEL_OUTPUT_ANALOG)) {

> > >  		limit = &intel_limits_g4x_hdmi;

> > > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {

> > > +	} else if (intel_pipe_will_have_type(crtc_state,

> > > +INTEL_OUTPUT_SDVO)) {

> > >  		limit = &intel_limits_g4x_sdvo;

> > >  	} else /* The option is for other outputs */

> > >  		limit = &intel_limits_i9xx_sdvo;

> > > @@ -488,17 +505,18 @@ static const intel_limit_t

> > > *intel_g4x_limit(struct intel_crtc *crtc)

> > >  	return limit;

> > >  }

> > >

> > > -static const intel_limit_t *intel_limit(struct intel_crtc *crtc,

> > > int refclk)

> > > +static const intel_limit_t *

> > > +intel_limit(struct intel_crtc_state *crtc_state, int refclk)

> > >  {

> > > -	struct drm_device *dev = crtc->base.dev;

> > > +	struct drm_device *dev = crtc_state->base.crtc->dev;

> > >  	const intel_limit_t *limit;

> > >

> > >  	if (HAS_PCH_SPLIT(dev))

> > > -		limit = intel_ironlake_limit(crtc, refclk);

> > > +		limit = intel_ironlake_limit(crtc_state, refclk);

> > >  	else if (IS_G4X(dev)) {

> > > -		limit = intel_g4x_limit(crtc);

> > > +		limit = intel_g4x_limit(crtc_state);

> > >  	} else if (IS_PINEVIEW(dev)) {

> > > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))

> > > +		if (intel_pipe_will_have_type(crtc_state,

> > > INTEL_OUTPUT_LVDS))

> > >  			limit = &intel_limits_pineview_lvds;

> > >  		else

> > >  			limit = &intel_limits_pineview_sdvo; @@ -507,14

> > > +525,14 @@ static const intel_limit_t *intel_limit(struct intel_crtc

> > > +*crtc, int

> > > refclk)

> > >  	} else if (IS_VALLEYVIEW(dev)) {

> > >  		limit = &intel_limits_vlv;

> > >  	} else if (!IS_GEN2(dev)) {

> > > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))

> > > +		if (intel_pipe_will_have_type(crtc_state,

> > > INTEL_OUTPUT_LVDS))

> > >  			limit = &intel_limits_i9xx_lvds;

> > >  		else

> > >  			limit = &intel_limits_i9xx_sdvo;

> > >  	} else {

> > > -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))

> > > +		if (intel_pipe_will_have_type(crtc_state,

> > > INTEL_OUTPUT_LVDS))

> > >  			limit = &intel_limits_i8xx_lvds;

> > > -		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))

> > > +		else if (intel_pipe_will_have_type(crtc_state,

> > > INTEL_OUTPUT_DVO))

> > >  			limit = &intel_limits_i8xx_dvo;

> > >  		else

> > >  			limit = &intel_limits_i8xx_dac;

> > > @@ -601,15 +619,17 @@ static bool intel_PLL_is_valid(struct

> > > drm_device *dev, }

> > >

> > >  static bool

> > > -i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc

> > > *crtc,

> > > +i9xx_find_best_dpll(const intel_limit_t *limit,

> > > +		    struct intel_crtc_state *crtc_state,

> > >  		    int target, int refclk, intel_clock_t *match_clock,

> > >  		    intel_clock_t *best_clock)

> > >  {

> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);

> > >  	struct drm_device *dev = crtc->base.dev;

> > >  	intel_clock_t clock;

> > >  	int err = target;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		/*

> > >  		 * For LVDS just rely on its current settings for dual-channel.

> > >  		 * We haven't figured out how to reliably set up different @@ -

> > > 662,15 +682,17 @@ i9xx_find_best_dpll(const intel_limit_t *limit,

> > > struct intel_crtc *crtc,  }

> > >

> > >  static bool

> > > -pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc

> > > *crtc,

> > > +pnv_find_best_dpll(const intel_limit_t *limit,

> > > +		   struct intel_crtc_state *crtc_state,

> > >  		   int target, int refclk, intel_clock_t *match_clock,

> > >  		   intel_clock_t *best_clock)

> > >  {

> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);

> > >  	struct drm_device *dev = crtc->base.dev;

> > >  	intel_clock_t clock;

> > >  	int err = target;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		/*

> > >  		 * For LVDS just rely on its current settings for dual-channel.

> > >  		 * We haven't figured out how to reliably set up different @@ -

> > > 721,10 +743,12 @@ pnv_find_best_dpll(const intel_limit_t *limit,

> > > struct intel_crtc *crtc,  }

> > >

> > >  static bool

> > > -g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc

> > > *crtc,

> > > +g4x_find_best_dpll(const intel_limit_t *limit,

> > > +		   struct intel_crtc_state *crtc_state,

> > >  		   int target, int refclk, intel_clock_t *match_clock,

> > >  		   intel_clock_t *best_clock)

> > >  {

> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);

> > >  	struct drm_device *dev = crtc->base.dev;

> > >  	intel_clock_t clock;

> > >  	int max_n;

> > > @@ -733,7 +757,7 @@ g4x_find_best_dpll(const intel_limit_t *limit,

> > > struct intel_crtc *crtc,

> > >  	int err_most = (target >> 8) + (target >> 9);

> > >  	found = false;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		if (intel_is_dual_link_lvds(dev))

> > >  			clock.p2 = limit->p2.p2_fast;

> > >  		else

> > > @@ -778,10 +802,12 @@ g4x_find_best_dpll(const intel_limit_t *limit,

> > > struct intel_crtc *crtc,  }

> > >

> > >  static bool

> > > -vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc

> > > *crtc,

> > > +vlv_find_best_dpll(const intel_limit_t *limit,

> > > +		   struct intel_crtc_state *crtc_state,

> > >  		   int target, int refclk, intel_clock_t *match_clock,

> > >  		   intel_clock_t *best_clock)

> > >  {

> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);

> > >  	struct drm_device *dev = crtc->base.dev;

> > >  	intel_clock_t clock;

> > >  	unsigned int bestppm = 1000000;

> > > @@ -835,10 +861,12 @@ vlv_find_best_dpll(const intel_limit_t *limit,

> > > struct intel_crtc *crtc,  }

> > >

> > >  static bool

> > > -chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc

> > > *crtc,

> > > +chv_find_best_dpll(const intel_limit_t *limit,

> > > +		   struct intel_crtc_state *crtc_state,

> > >  		   int target, int refclk, intel_clock_t *match_clock,

> > >  		   intel_clock_t *best_clock)

> > >  {

> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);

> > >  	struct drm_device *dev = crtc->base.dev;

> > >  	intel_clock_t clock;

> > >  	uint64_t m2;

> > > @@ -5726,7 +5754,7 @@ static int intel_crtc_compute_config(struct

> > > intel_crtc *crtc,

> > >  	 * - LVDS dual channel mode

> > >  	 * - Double wide pipe

> > >  	 */

> > > -	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&

> > > +	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&

> > >  	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)

> > >  		pipe_config->pipe_src_w &= ~1;

> > >

> > > @@ -5905,15 +5933,18 @@ static inline bool

> > > intel_panel_use_ssc(struct drm_i915_private *dev_priv)

> > >  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);  }

> > >

> > > -static int i9xx_get_refclk(struct intel_crtc *crtc, int

> > > num_connectors)

> > > +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,

> > > +			   int num_connectors)

> > >  {

> > > -	struct drm_device *dev = crtc->base.dev;

> > > +	struct drm_device *dev = crtc_state->base.crtc->dev;

> > >  	struct drm_i915_private *dev_priv = dev->dev_private;

> > >  	int refclk;

> > >

> > > +	WARN_ON(!crtc_state->base.state);

> > > +

> > >  	if (IS_VALLEYVIEW(dev)) {

> > >  		refclk = 100000;

> > > -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&

> > > +	} else if (intel_pipe_will_have_type(crtc_state,

> > > +INTEL_OUTPUT_LVDS)

> > > &&

> > >  	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {

> > >  		refclk = dev_priv->vbt.lvds_ssc_freq;

> > >  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",

> refclk);

> > > @@ -5956,7 +5987,7 @@ static void i9xx_update_pll_dividers(struct

> > > intel_crtc *crtc,

> > >  	crtc_state->dpll_hw_state.fp0 = fp;

> > >

> > >  	crtc->lowfreq_avail = false;

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&

> > >  	    reduced_clock && i915.powersave) {

> > >  		crtc_state->dpll_hw_state.fp1 = fp2;

> > >  		crtc->lowfreq_avail = true;

> > > @@ -6314,6 +6345,7 @@ void vlv_force_pll_on(struct drm_device *dev,

> > > enum pipe pipe,

> > >  	struct intel_crtc *crtc =

> > >  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));

> > >  	struct intel_crtc_state pipe_config = {

> > > +		.base.crtc = &crtc->base,

> > >  		.pixel_multiplier = 1,

> > >  		.dpll = *dpll,

> > >  	};

> > > @@ -6358,12 +6390,12 @@ static void i9xx_update_pll(struct

> > > intel_crtc *crtc,

> > >

> > >  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);

> > >

> > > -	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||

> > > -		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);

> > > +	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)

> > > ||

> > > +		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);

> > >

> > >  	dpll = DPLL_VGA_MODE_DIS;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))

> > >  		dpll |= DPLLB_MODE_LVDS;

> > >  	else

> > >  		dpll |= DPLLB_MODE_DAC_SERIAL;

> > > @@ -6406,7 +6438,7 @@ static void i9xx_update_pll(struct intel_crtc

> > > *crtc,

> > >

> > >  	if (crtc_state->sdvo_tv_clock)

> > >  		dpll |= PLL_REF_INPUT_TVCLKINBC;

> > > -	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&

> > > +	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)

> > > +&&

> > >  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)

> > >  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;

> > >  	else

> > > @@ -6436,7 +6468,7 @@ static void i8xx_update_pll(struct intel_crtc

> > > *crtc,

> > >

> > >  	dpll = DPLL_VGA_MODE_DIS;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {

> > >  		dpll |= (1 << (clock->p1 - 1)) <<

> DPLL_FPA01_P1_POST_DIV_SHIFT;

> > >  	} else {

> > >  		if (clock->p1 == 2)

> > > @@ -6447,10 +6479,10 @@ static void i8xx_update_pll(struct intel_crtc

> *crtc,

> > >  			dpll |= PLL_P2_DIVIDE_BY_4;

> > >  	}

> > >

> > > -	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc,

> > > INTEL_OUTPUT_DVO))

> > > +	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state,

> > > +INTEL_OUTPUT_DVO))

> > >  		dpll |= DPLL_DVO_2X_MODE;

> > >

> > > -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&

> > > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&

> > >  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)

> > >  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;

> > >  	else

> > > @@ -6687,7 +6719,7 @@ static int i9xx_crtc_compute_clock(struct

> > > intel_crtc *crtc,

> > >  		return 0;

> > >

> > >  	if (!crtc_state->clock_set) {

> > > -		refclk = i9xx_get_refclk(crtc, num_connectors);

> > > +		refclk = i9xx_get_refclk(crtc_state, num_connectors);

> > >

> > >  		/*

> > >  		 * Returns a set of divisors for the desired target clock with @@

> > > -6695,8 +6727,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc

> *crtc,

> > >  		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +

> > >  		 * 2) / p1 / p2.

> > >  		 */

> > > -		limit = intel_limit(crtc, refclk);

> > > -		ok = dev_priv->display.find_dpll(limit, crtc,

> > > +		limit = intel_limit(crtc_state, refclk);

> > > +		ok = dev_priv->display.find_dpll(limit, crtc_state,

> > >  						 crtc_state->port_clock,

> > >  						 refclk, NULL, &clock);

> > >  		if (!ok) {

> > > @@ -6712,7 +6744,7 @@ static int i9xx_crtc_compute_clock(struct

> > > intel_crtc *crtc,

> > >  			 * we will disable the LVDS downclock feature.

> > >  			 */

> > >  			has_reduced_clock =

> > > -				dev_priv->display.find_dpll(limit, crtc,

> > > +				dev_priv->display.find_dpll(limit, crtc_state,

> > >  							    dev_priv-

> > > >lvds_downclock,

> > >  							    refclk, &clock,

> > >  							    &reduced_clock);

> > > @@ -7540,12 +7572,11 @@ static bool ironlake_compute_clocks(struct

> > > drm_crtc *crtc,  {

> > >  	struct drm_device *dev = crtc->dev;

> > >  	struct drm_i915_private *dev_priv = dev->dev_private;

> > > -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

> > >  	int refclk;

> > >  	const intel_limit_t *limit;

> > >  	bool ret, is_lvds = false;

> > >

> > > -	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);

> > > +	is_lvds = intel_pipe_will_have_type(crtc_state,

> > > +INTEL_OUTPUT_LVDS);

> > >

> > >  	refclk = ironlake_get_refclk(crtc);

> > >

> > > @@ -7554,8 +7585,8 @@ static bool ironlake_compute_clocks(struct

> > > drm_crtc *crtc,

> > >  	 * refclk, or FALSE.  The returned values represent the clock equation:

> > >  	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.

> > >  	 */

> > > -	limit = intel_limit(intel_crtc, refclk);

> > > -	ret = dev_priv->display.find_dpll(limit, intel_crtc,

> > > +	limit = intel_limit(crtc_state, refclk);

> > > +	ret = dev_priv->display.find_dpll(limit, crtc_state,

> > >  					  crtc_state->port_clock,

> > >  					  refclk, NULL, clock);

> > >  	if (!ret)

> > > @@ -7569,7 +7600,7 @@ static bool ironlake_compute_clocks(struct

> > > drm_crtc *crtc,

> > >  		 * downclock feature.

> > >  		*/

> > >  		*has_reduced_clock =

> > > -			dev_priv->display.find_dpll(limit, intel_crtc,

> > > +			dev_priv->display.find_dpll(limit, crtc_state,

> > >  						    dev_priv->lvds_downclock,

> > >  						    refclk, clock,

> > >  						    reduced_clock);

> > > --

> > > 2.1.0

> >

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 490ab8c..576e101 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -541,7 +541,7 @@  struct drm_i915_display_funcs {
 	 * Returns true on success, false on failure.
 	 */
 	bool (*find_dpll)(const struct intel_limit *limit,
-			  struct intel_crtc *crtc,
+			  struct intel_crtc_state *crtc_state,
 			  int target, int refclk,
 			  struct dpll *match_clock,
 			  struct dpll *best_clock);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c97186..be1979e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -431,25 +431,41 @@  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
  * intel_pipe_has_type() but looking at encoder->new_crtc instead of
  * encoder->crtc.
  */
-static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
+static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
+				      int type)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
+	int i, num_connectors = 0;
+
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
+			continue;
+
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != crtc_state->base.crtc)
+			continue;
+
+		num_connectors++;
 
-	for_each_intel_encoder(dev, encoder)
-		if (encoder->new_crtc == crtc && encoder->type == type)
+		encoder = to_intel_encoder(connector_state->best_encoder);
+		if (encoder->type == type)
 			return true;
+	}
+
+	WARN_ON(num_connectors == 0);
 
 	return false;
 }
 
-static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
-						int refclk)
+static const intel_limit_t *
+intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev)) {
 			if (refclk == 100000)
 				limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -467,20 +483,21 @@  static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
 	return limit;
 }
 
-static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
+static const intel_limit_t *
+intel_g4x_limit(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			limit = &intel_limits_g4x_dual_channel_lvds;
 		else
 			limit = &intel_limits_g4x_single_channel_lvds;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
-		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
+		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		limit = &intel_limits_g4x_hdmi;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
 	} else /* The option is for other outputs */
 		limit = &intel_limits_i9xx_sdvo;
@@ -488,17 +505,18 @@  static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
 	return limit;
 }
 
-static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
+static const intel_limit_t *
+intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
 	if (HAS_PCH_SPLIT(dev))
-		limit = intel_ironlake_limit(crtc, refclk);
+		limit = intel_ironlake_limit(crtc_state, refclk);
 	else if (IS_G4X(dev)) {
-		limit = intel_g4x_limit(crtc);
+		limit = intel_g4x_limit(crtc_state);
 	} else if (IS_PINEVIEW(dev)) {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_pineview_lvds;
 		else
 			limit = &intel_limits_pineview_sdvo;
@@ -507,14 +525,14 @@  static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
 	} else if (IS_VALLEYVIEW(dev)) {
 		limit = &intel_limits_vlv;
 	} else if (!IS_GEN2(dev)) {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
 		else
 			limit = &intel_limits_i9xx_sdvo;
 	} else {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i8xx_lvds;
-		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
+		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
 			limit = &intel_limits_i8xx_dvo;
 		else
 			limit = &intel_limits_i8xx_dac;
@@ -601,15 +619,17 @@  static bool intel_PLL_is_valid(struct drm_device *dev,
 }
 
 static bool
-i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+i9xx_find_best_dpll(const intel_limit_t *limit,
+		    struct intel_crtc_state *crtc_state,
 		    int target, int refclk, intel_clock_t *match_clock,
 		    intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -662,15 +682,17 @@  i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+pnv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -721,10 +743,12 @@  pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+g4x_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int max_n;
@@ -733,7 +757,7 @@  g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 	int err_most = (target >> 8) + (target >> 9);
 	found = false;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			clock.p2 = limit->p2.p2_fast;
 		else
@@ -778,10 +802,12 @@  g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+vlv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	unsigned int bestppm = 1000000;
@@ -835,10 +861,12 @@  vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+chv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	uint64_t m2;
@@ -5726,7 +5754,7 @@  static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	 * - LVDS dual channel mode
 	 * - Double wide pipe
 	 */
-	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
 	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
 		pipe_config->pipe_src_w &= ~1;
 
@@ -5905,15 +5933,18 @@  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static int i9xx_get_refclk(struct intel_crtc *crtc, int num_connectors)
+static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
+			   int num_connectors)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int refclk;
 
+	WARN_ON(!crtc_state->base.state);
+
 	if (IS_VALLEYVIEW(dev)) {
 		refclk = 100000;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
 		refclk = dev_priv->vbt.lvds_ssc_freq;
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -5956,7 +5987,7 @@  static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 	crtc_state->dpll_hw_state.fp0 = fp;
 
 	crtc->lowfreq_avail = false;
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    reduced_clock && i915.powersave) {
 		crtc_state->dpll_hw_state.fp1 = fp2;
 		crtc->lowfreq_avail = true;
@@ -6314,6 +6345,7 @@  void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
 	struct intel_crtc *crtc =
 		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
 	struct intel_crtc_state pipe_config = {
+		.base.crtc = &crtc->base,
 		.pixel_multiplier = 1,
 		.dpll = *dpll,
 	};
@@ -6358,12 +6390,12 @@  static void i9xx_update_pll(struct intel_crtc *crtc,
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
-	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO) ||
+		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
@@ -6406,7 +6438,7 @@  static void i9xx_update_pll(struct intel_crtc *crtc,
 
 	if (crtc_state->sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -6436,7 +6468,7 @@  static void i8xx_update_pll(struct intel_crtc *crtc,
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	} else {
 		if (clock->p1 == 2)
@@ -6447,10 +6479,10 @@  static void i8xx_update_pll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -6687,7 +6719,7 @@  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		return 0;
 
 	if (!crtc_state->clock_set) {
-		refclk = i9xx_get_refclk(crtc, num_connectors);
+		refclk = i9xx_get_refclk(crtc_state, num_connectors);
 
 		/*
 		 * Returns a set of divisors for the desired target clock with
@@ -6695,8 +6727,8 @@  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
 		 * 2) / p1 / p2.
 		 */
-		limit = intel_limit(crtc, refclk);
-		ok = dev_priv->display.find_dpll(limit, crtc,
+		limit = intel_limit(crtc_state, refclk);
+		ok = dev_priv->display.find_dpll(limit, crtc_state,
 						 crtc_state->port_clock,
 						 refclk, NULL, &clock);
 		if (!ok) {
@@ -6712,7 +6744,7 @@  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 			 * we will disable the LVDS downclock feature.
 			 */
 			has_reduced_clock =
-				dev_priv->display.find_dpll(limit, crtc,
+				dev_priv->display.find_dpll(limit, crtc_state,
 							    dev_priv->lvds_downclock,
 							    refclk, &clock,
 							    &reduced_clock);
@@ -7540,12 +7572,11 @@  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int refclk;
 	const intel_limit_t *limit;
 	bool ret, is_lvds = false;
 
-	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);
+	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
 
 	refclk = ironlake_get_refclk(crtc);
 
@@ -7554,8 +7585,8 @@  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 * refclk, or FALSE.  The returned values represent the clock equation:
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
-	limit = intel_limit(intel_crtc, refclk);
-	ret = dev_priv->display.find_dpll(limit, intel_crtc,
+	limit = intel_limit(crtc_state, refclk);
+	ret = dev_priv->display.find_dpll(limit, crtc_state,
 					  crtc_state->port_clock,
 					  refclk, NULL, clock);
 	if (!ret)
@@ -7569,7 +7600,7 @@  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 		 * downclock feature.
 		*/
 		*has_reduced_clock =
-			dev_priv->display.find_dpll(limit, intel_crtc,
+			dev_priv->display.find_dpll(limit, crtc_state,
 						    dev_priv->lvds_downclock,
 						    refclk, clock,
 						    reduced_clock);