[Intel-gfx] [PATCH] drm/i915/icl: Add pre_pll_enable hook for ICL and set DFLEXDPMLE in this hook

[Souza, Jose]

On Tue, 2018-07-31 at 17:30 -0700, Manasi Navare wrote:
> In case of Legacy DP connector on TypeC port (C, D, E or F), the

Legacy DP connector(DisplayPort Alternative Mode)

> flex IO DPMLE register is set to maximum number of lanes since
> there is no muxing with other controllers in this case.

Okay there is no muxing but driver could choose to use few lanes do
save power.

> While in case of the TypeC connector, it is set to the lane count
> obained from DFLEXDPSP register.

obtained

> This needs to be programmed before enabling the shared PLLs hence
> add a pre_pll_enable hook for ICL and add this programming in that
> hook.
> 
> v3:
> * Call intel_dp_max_common_lane_count that gets lane count
> common between sink, source, fia
> v2:
> * Add pre pll enable hook and move dflexdpmle programming
> to that hook (Animesh)
> 
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Animesh Manna <animesh.manna at intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  3 files changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043..eb00ac4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3193,6 +3193,22 @@ static void bxt_ddi_pre_pll_enable(struct
> intel_encoder *encoder,
>  	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
>  }
>  
> +static void icl_ddi_pre_pll_enable(struct intel_encoder *encoder,
> +				   const struct intel_crtc_state
> *pipe_config,
> +				   const struct drm_connector_state
> *conn_state)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> >base.base.dev);
> +	enum tc_port tc_port = intel_port_to_tc(dev_priv,
> +						intel_dig_port-
> >base.port);
> +
> +	if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type ==
> TC_PORT_TBT)
> +		return;
> +
> +	intel_dp_set_fia_lane_count(intel_dp);
> +}
> +
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> @@ -3709,6 +3725,8 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv, enum port port)
>  	intel_encoder->enable = intel_enable_ddi;
>  	if (IS_GEN9_LP(dev_priv))
>  		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> +	if (IS_ICELAKE(dev_priv))
> +		intel_encoder->pre_pll_enable = icl_ddi_pre_pll_enable;

pre_pll_enable is set initialy in intel_crt_init() for most of the
gens, maybe those 4 lines should be moved there but that can be done in
another patch.

>  	intel_encoder->pre_enable = intel_ddi_pre_enable;
>  	intel_encoder->disable = intel_disable_ddi;
>  	intel_encoder->post_disable = intel_ddi_post_disable;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 49a3149..6683cdc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2127,6 +2127,26 @@ void intel_dp_set_link_params(struct intel_dp
> *intel_dp,
>  	intel_dp->link_mst = link_mst;
>  }
>  
> +void intel_dp_set_fia_lane_count(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> +	enum tc_port tc_port = intel_port_to_tc(dev_priv,
> +						intel_dig_port-
> >base.port);
> +	u8 lane_count = intel_dp_max_common_lane_count(intel_dp);

intel_dp->max_link_lane_count could be used instead as it is set on
hotplug and don't change over time.

But I guess this is also wrong, it should not enable the max lane
count, it should enable the number of the lanes that the driver decides
that is enough to the mode set to save power.

> +	u32 val;
> +
> +	/* In case of legacy/static DP over Type-C port there is no
> muxing
> +	 * with other controllers so this is set to max lane count.
> +	 * In case of Type_C it is set to the DFLEXDPSP.DPX4TXLATC
> value.
> +	 */
> +	val = I915_READ(PORT_TX_DFLEXDPMLE1);
> +	val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> +	val |= DFLEXDPMLE1_DPMLETC(tc_port, lane_count);

this macro is wrong, each bit set maps to a lane, so to enable all 4
lanes it should 0xf for the first TC.

From spec:

For example, in DP Pin Assignment C, the register
DFLEXDPSP1.DPX4TXLATC0 tells Display Driver that all the 4 TX Lane in
PHY can be used. However, Display Driver may choose to use only x1,
i.e. for ML0. Then Display Driver will program “0001b” to this
register. For x2 and x4, Display Driver will program “0011b” and
“1111b”, respectively.


> +	I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
> +}
> +
>  static void intel_dp_prepare(struct intel_encoder *encoder,
>  			     const struct intel_crtc_state
> *pipe_config)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1ad7c11..d98cdb9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1676,6 +1676,7 @@ bool intel_dp_init_connector(struct
> intel_digital_port *intel_dig_port,
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  			      int link_rate, uint8_t lane_count,
>  			      bool link_mst);
> +void intel_dp_set_fia_lane_count(struct intel_dp *intel_dp);
>  int intel_dp_get_link_train_fallback_values(struct intel_dp
> *intel_dp,
>  					    int link_rate, uint8_t
> lane_count);
>  void intel_dp_start_link_train(struct intel_dp *intel_dp);