Re: [PATCH v2] drm/i915: Remove ddi_pll_sel from intel_crtc_state

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

 



> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Tuesday, May 24, 2016 1:03 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: R, Durgadoss <durgadoss.r@xxxxxxxxx>; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira@xxxxxxxxx>
> Subject: [PATCH v2] drm/i915: Remove ddi_pll_sel from intel_crtc_state
> 
> The value of ddi_pll_sel is derived from the selection of shared dpll,
> so just calculate the final value when necessary.
> 
> v2: Actually remove it from crtc state and delete remaining usages. (CI)
> 

Looks good to me,

Reviewed-by: Durgadoss R <durgadoss.r@xxxxxxxxx>

Thanks,
Durga

> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 45 ++++++++++++++++++++++++++--
> -------
>  drivers/gpu/drm/i915/intel_display.c  | 43 +++++++--------------------------
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  3 ++-
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 27 ---------------------
>  drivers/gpu/drm/i915/intel_drv.h      |  8 +------
>  5 files changed, 45 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1cbdd66..2ee5c4c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -531,6 +531,27 @@ static void intel_wait_ddi_buf_idle(struct
> drm_i915_private *dev_priv,
>  	DRM_ERROR("Timeout waiting for DDI BUF %c idle bit\n",
> port_name(port));
>  }
> 
> +static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
> +{
> +	switch (pll->id) {
> +	case DPLL_ID_WRPLL1:
> +		return PORT_CLK_SEL_WRPLL1;
> +	case DPLL_ID_WRPLL2:
> +		return PORT_CLK_SEL_WRPLL2;
> +	case DPLL_ID_SPLL:
> +		return PORT_CLK_SEL_SPLL;
> +	case DPLL_ID_LCPLL_810:
> +		return PORT_CLK_SEL_LCPLL_810;
> +	case DPLL_ID_LCPLL_1350:
> +		return PORT_CLK_SEL_LCPLL_1350;
> +	case DPLL_ID_LCPLL_2700:
> +		return PORT_CLK_SEL_LCPLL_2700;
> +	default:
> +		MISSING_CASE(pll->id);
> +		return PORT_CLK_SEL_NONE;
> +	}
> +}
> +
>  /* Starting with Haswell, different DDI ports can work in FDI mode for
>   * connection to the PCH-located connectors. For this, it is necessary to train
>   * both the DDI port and PCH receiver for the desired DDI buffer settings.
> @@ -546,7 +567,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	u32 temp, i, rx_ctl_val;
> +	u32 temp, i, rx_ctl_val, ddi_pll_sel;
> 
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		WARN_ON(encoder->type != INTEL_OUTPUT_ANALOG);
> @@ -577,8 +598,9 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  	I915_WRITE(FDI_RX_CTL(PIPE_A), rx_ctl_val);
> 
>  	/* Configure Port Clock Select */
> -	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->config-
> >ddi_pll_sel);
> -	WARN_ON(intel_crtc->config->ddi_pll_sel != PORT_CLK_SEL_SPLL);
> +	ddi_pll_sel = hsw_pll_to_ddi_pll_sel(intel_crtc->config-
> >shared_dpll);
> +	I915_WRITE(PORT_CLK_SEL(PORT_E), ddi_pll_sel);
> +	WARN_ON(ddi_pll_sel != PORT_CLK_SEL_SPLL);
> 
>  	/* Start the training iterating through available voltages and
> emphasis,
>  	 * testing each value twice. */
> @@ -855,7 +877,7 @@ static void skl_ddi_clock_get(struct intel_encoder
> *encoder,
>  	int link_clock = 0;
>  	uint32_t dpll_ctl1, dpll;
> 
> -	dpll = pipe_config->ddi_pll_sel;
> +	dpll = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> 
>  	dpll_ctl1 = I915_READ(DPLL_CTRL1);
> 
> @@ -903,7 +925,7 @@ static void hsw_ddi_clock_get(struct intel_encoder
> *encoder,
>  	int link_clock = 0;
>  	u32 val, pll;
> 
> -	val = pipe_config->ddi_pll_sel;
> +	val = hsw_pll_to_ddi_pll_sel(pipe_config->shared_dpll);
>  	switch (val & PORT_CLK_SEL_MASK) {
>  	case PORT_CLK_SEL_LCPLL_810:
>  		link_clock = 81000;
> @@ -1568,13 +1590,15 @@ uint32_t ddi_signal_levels(struct intel_dp
> *intel_dp)
>  }
> 
>  void intel_ddi_clk_select(struct intel_encoder *encoder,
> -			  const struct intel_crtc_state *pipe_config)
> +			  struct intel_shared_dpll *pll)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> 
> +	if (WARN_ON(!pll))
> +		return;
> +
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> -		uint32_t dpll = pipe_config->ddi_pll_sel;
>  		uint32_t val;
> 
>  		/* DDI -> PLL mapping  */
> @@ -1582,14 +1606,13 @@ void intel_ddi_clk_select(struct intel_encoder
> *encoder,
> 
>  		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
>  			DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
> -		val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) |
> +		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) |
>  			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
> 
>  		I915_WRITE(DPLL_CTRL2, val);
> 
>  	} else if (INTEL_INFO(dev_priv)->gen < 9) {
> -		WARN_ON(pipe_config->ddi_pll_sel ==
> PORT_CLK_SEL_NONE);
> -		I915_WRITE(PORT_CLK_SEL(port), pipe_config->ddi_pll_sel);
> +		I915_WRITE(PORT_CLK_SEL(port),
> hsw_pll_to_ddi_pll_sel(pll));
>  	}
>  }
> 
> @@ -1614,7 +1637,7 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder)
>  		intel_edp_panel_on(intel_dp);
>  	}
> 
> -	intel_ddi_clk_select(intel_encoder, crtc->config);
> +	intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll);
> 
>  	if (type == INTEL_OUTPUT_DISPLAYPORT || type ==
> INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a500f08..0251841 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9669,15 +9669,12 @@ static void bxt_get_ddi_pll(struct
> drm_i915_private *dev_priv,
> 
>  	switch (port) {
>  	case PORT_A:
> -		pipe_config->ddi_pll_sel = SKL_DPLL0;
>  		id = DPLL_ID_SKL_DPLL0;
>  		break;
>  	case PORT_B:
> -		pipe_config->ddi_pll_sel = SKL_DPLL1;
>  		id = DPLL_ID_SKL_DPLL1;
>  		break;
>  	case PORT_C:
> -		pipe_config->ddi_pll_sel = SKL_DPLL2;
>  		id = DPLL_ID_SKL_DPLL2;
>  		break;
>  	default:
> @@ -9696,25 +9693,10 @@ static void skylake_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  	u32 temp;
> 
>  	temp = I915_READ(DPLL_CTRL2) &
> DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
> -	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
> +	id = temp >> (port * 3 + 1);
> 
> -	switch (pipe_config->ddi_pll_sel) {
> -	case SKL_DPLL0:
> -		id = DPLL_ID_SKL_DPLL0;
> -		break;
> -	case SKL_DPLL1:
> -		id = DPLL_ID_SKL_DPLL1;
> -		break;
> -	case SKL_DPLL2:
> -		id = DPLL_ID_SKL_DPLL2;
> -		break;
> -	case SKL_DPLL3:
> -		id = DPLL_ID_SKL_DPLL3;
> -		break;
> -	default:
> -		MISSING_CASE(pipe_config->ddi_pll_sel);
> +	if (WARN_ON(id < SKL_DPLL0 || id > SKL_DPLL3))
>  		return;
> -	}
> 
>  	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv,
> id);
>  }
> @@ -9724,10 +9706,9 @@ static void haswell_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  				struct intel_crtc_state *pipe_config)
>  {
>  	enum intel_dpll_id id;
> +	uint32_t ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
> 
> -	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
> -
> -	switch (pipe_config->ddi_pll_sel) {
> +	switch (ddi_pll_sel) {
>  	case PORT_CLK_SEL_WRPLL1:
>  		id = DPLL_ID_WRPLL1;
>  		break;
> @@ -9747,7 +9728,7 @@ static void haswell_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  		id = DPLL_ID_LCPLL_2700;
>  		break;
>  	default:
> -		MISSING_CASE(pipe_config->ddi_pll_sel);
> +		MISSING_CASE(ddi_pll_sel);
>  		/* fall through */
>  	case PORT_CLK_SEL_NONE:
>  		return;
> @@ -11484,10 +11465,9 @@ static void intel_dump_pipe_config(struct
> intel_crtc *crtc,
>  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config-
> >double_wide);
> 
>  	if (IS_BROXTON(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: ebb0:
> 0x%x, ebb4: 0x%x,"
> +		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4:
> 0x%x,"
>  			      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
>  			      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x,
> pcsdw12: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
>  			      pipe_config->dpll_hw_state.ebb0,
>  			      pipe_config->dpll_hw_state.ebb4,
>  			      pipe_config->dpll_hw_state.pll0,
> @@ -11500,15 +11480,13 @@ static void intel_dump_pipe_config(struct
> intel_crtc *crtc,
>  			      pipe_config->dpll_hw_state.pll10,
>  			      pipe_config->dpll_hw_state.pcsdw12);
>  	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: "
> +		DRM_DEBUG_KMS("dpll_hw_state: "
>  			      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
>  			      pipe_config->dpll_hw_state.ctrl1,
>  			      pipe_config->dpll_hw_state.cfgcr1,
>  			      pipe_config->dpll_hw_state.cfgcr2);
>  	} else if (HAS_DDI(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: 0x%x; dpll_hw_state: wrpll:
> 0x%x spll: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
> +		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll:
> 0x%x\n",
>  			      pipe_config->dpll_hw_state.wrpll,
>  			      pipe_config->dpll_hw_state.spll);
>  	} else {
> @@ -11611,7 +11589,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	struct intel_crtc_scaler_state scaler_state;
>  	struct intel_dpll_hw_state dpll_hw_state;
>  	struct intel_shared_dpll *shared_dpll;
> -	uint32_t ddi_pll_sel;
>  	bool force_thru;
> 
>  	/* FIXME: before the switch to atomic started, a new pipe_config
> was
> @@ -11623,7 +11600,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	scaler_state = crtc_state->scaler_state;
>  	shared_dpll = crtc_state->shared_dpll;
>  	dpll_hw_state = crtc_state->dpll_hw_state;
> -	ddi_pll_sel = crtc_state->ddi_pll_sel;
>  	force_thru = crtc_state->pch_pfit.force_thru;
> 
>  	memset(crtc_state, 0, sizeof *crtc_state);
> @@ -11632,7 +11608,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	crtc_state->scaler_state = scaler_state;
>  	crtc_state->shared_dpll = shared_dpll;
>  	crtc_state->dpll_hw_state = dpll_hw_state;
> -	crtc_state->ddi_pll_sel = ddi_pll_sel;
>  	crtc_state->pch_pfit.force_thru = force_thru;
>  }
> 
> @@ -12043,8 +12018,6 @@ intel_pipe_config_compare(struct drm_device
> *dev,
> 
>  	PIPE_CONF_CHECK_I(double_wide);
> 
> -	PIPE_CONF_CHECK_X(ddi_pll_sel);
> -
>  	PIPE_CONF_CHECK_P(shared_dpll);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 553a25e..ad1913f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -173,7 +173,8 @@ static void intel_mst_pre_enable_dp(struct
> intel_encoder *encoder)
>  	if (intel_dp->active_mst_links == 0) {
>  		intel_prepare_ddi_buffer(&intel_dig_port->base);
> 
> -		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc-
> >config);
> +		intel_ddi_clk_select(&intel_dig_port->base,
> +				     intel_crtc->config->shared_dpll);
> 
>  		intel_dp_set_link_params(intel_dp,
>  					 intel_crtc->config->port_clock,
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c283ba4..70b3c12 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -449,26 +449,6 @@ static bool hsw_ddi_spll_get_hw_state(struct
> drm_i915_private *dev_priv,
>  	return val & SPLL_PLL_ENABLE;
>  }
> 
> -static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
> -{
> -	switch (pll->id) {
> -	case DPLL_ID_WRPLL1:
> -		return PORT_CLK_SEL_WRPLL1;
> -	case DPLL_ID_WRPLL2:
> -		return PORT_CLK_SEL_WRPLL2;
> -	case DPLL_ID_SPLL:
> -		return PORT_CLK_SEL_SPLL;
> -	case DPLL_ID_LCPLL_810:
> -		return PORT_CLK_SEL_LCPLL_810;
> -	case DPLL_ID_LCPLL_1350:
> -		return PORT_CLK_SEL_LCPLL_1350;
> -	case DPLL_ID_LCPLL_2700:
> -		return PORT_CLK_SEL_LCPLL_2700;
> -	default:
> -		return PORT_CLK_SEL_NONE;
> -	}
> -}
> -
>  #define LC_FREQ 2700
>  #define LC_FREQ_2K U64_C(LC_FREQ * 2000)
> 
> @@ -748,8 +728,6 @@ hsw_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	if (!pll)
>  		return NULL;
> 
> -	crtc_state->ddi_pll_sel = hsw_pll_to_ddi_pll_sel(pll);
> -
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
>  	return pll;
> @@ -1270,8 +1248,6 @@ skl_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	if (!pll)
>  		return NULL;
> 
> -	crtc_state->ddi_pll_sel = pll->id;
> -
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
>  	return pll;
> @@ -1618,9 +1594,6 @@ bxt_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
> 
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
> -	/* shared DPLL id 0 is DPLL A */
> -	crtc_state->ddi_pll_sel = pll->id;
> -
>  	return pll;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 91fee9f..6dd851b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -545,12 +545,6 @@ struct intel_crtc_state {
>  	/* Selected dpll when shared or NULL. */
>  	struct intel_shared_dpll *shared_dpll;
> 
> -	/*
> -	 * - PORT_CLK_SEL for DDI ports on HSW/BDW.
> -	 * - enum skl_dpll on SKL
> -	 */
> -	uint32_t ddi_pll_sel;
> -
>  	/* Actual register state of the dpll, for shared dpll cross-checking. */
>  	struct intel_dpll_hw_state dpll_hw_state;
> 
> @@ -1086,7 +1080,7 @@ void intel_crt_init(struct drm_device *dev);
> 
>  /* intel_ddi.c */
>  void intel_ddi_clk_select(struct intel_encoder *encoder,
> -			  const struct intel_crtc_state *pipe_config);
> +			  struct intel_shared_dpll *pll);
>  void intel_prepare_ddi_buffer(struct intel_encoder *encoder);
>  void hsw_fdi_link_train(struct drm_crtc *crtc);
>  void intel_ddi_init(struct drm_device *dev, enum port port);
> --
> 2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://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