Commit d7164a50 authored by Imre Deak's avatar Imre Deak Committed by Tvrtko Ursulin
Browse files

drm/i915/tgl+: Add locking around DKL PHY register accesses 



Accessing the TypeC DKL PHY registers during modeset-commit,
-verification, DP link-retraining and AUX power well toggling is racy
due to these code paths being concurrent and the PHY register bank
selection register (HIP_INDEX_REG) being shared between PHY instances
(aka TC ports) and the bank selection being not atomic wrt. the actual
PHY register access.

Add the required locking around each PHY register bank selection->
register access sequence.

Kudos to Ville for noticing the race conditions.

v2:
- Add the DKL PHY register accessors to intel_dkl_phy.[ch]. (Jani)
- Make the DKL_REG_TC_PORT macro independent of PHY internals.
- Move initing the DKL PHY lock to a more logical place.

v3:
- Fix parameter reuse in the DKL_REG_TC_PORT definition.
- Document the usage of phy_lock.

v4:
- Fix adding TC_PORT_1 offset in the DKL_REG_TC_PORT definition.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: <stable@vger.kernel.org> # v5.5+
Acked-by: default avatarJani Nikula <jani.nikula@intel.com>
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221025114457.2191004-1-imre.deak@intel.com


(cherry picked from commit 89cb0ba4)
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
parent 30a0b95b

drivers/gpu/drm/i915/Makefile

+1 −0
Original line number Diff line number Diff line
@@ -282,6 +282,7 @@ i915-y += \ 
	display/intel_ddi.o \

	display/intel_ddi_buf_trans.o \

	display/intel_display_trace.o \

	display/intel_dkl_phy.o \

	display/intel_dp.o \

	display/intel_dp_aux.o \

	display/intel_dp_aux_backlight.o \

drivers/gpu/drm/i915/display/intel_ddi.c

+28 −40
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ 
#include "intel_de.h"

#include "intel_display_power.h"

#include "intel_display_types.h"

#include "intel_dkl_phy.h"

#include "intel_dp.h"

#include "intel_dp_link_training.h"

#include "intel_dp_mst.h"
@@ -1262,14 +1263,11 @@ static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder, 
	for (ln = 0; ln < 2; ln++) {

		int level;



		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, ln));



		intel_de_write(dev_priv, DKL_TX_PMD_LANE_SUS(tc_port), 0);

		intel_dkl_phy_write(dev_priv, DKL_TX_PMD_LANE_SUS(tc_port), ln, 0);



		level = intel_ddi_level(encoder, crtc_state, 2*ln+0);



		intel_de_rmw(dev_priv, DKL_TX_DPCNTL0(tc_port),

		intel_dkl_phy_rmw(dev_priv, DKL_TX_DPCNTL0(tc_port), ln,

				  DKL_TX_PRESHOOT_COEFF_MASK |

				  DKL_TX_DE_EMPAHSIS_COEFF_MASK |

				  DKL_TX_VSWING_CONTROL_MASK,
@@ -1279,7 +1277,7 @@ static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder, 


		level = intel_ddi_level(encoder, crtc_state, 2*ln+1);



		intel_de_rmw(dev_priv, DKL_TX_DPCNTL1(tc_port),

		intel_dkl_phy_rmw(dev_priv, DKL_TX_DPCNTL1(tc_port), ln,

				  DKL_TX_PRESHOOT_COEFF_MASK |

				  DKL_TX_DE_EMPAHSIS_COEFF_MASK |

				  DKL_TX_VSWING_CONTROL_MASK,
@@ -1287,7 +1285,7 @@ static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder, 
				  DKL_TX_DE_EMPHASIS_COEFF(trans->entries[level].dkl.de_emphasis) |

				  DKL_TX_VSWING_CONTROL(trans->entries[level].dkl.vswing));



		intel_de_rmw(dev_priv, DKL_TX_DPCNTL2(tc_port),

		intel_dkl_phy_rmw(dev_priv, DKL_TX_DPCNTL2(tc_port), ln,

				  DKL_TX_DP20BITMODE, 0);



		if (IS_ALDERLAKE_P(dev_priv)) {
@@ -1306,7 +1304,7 @@ static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder, 
				val |= DKL_TX_DPCNTL2_CFG_LOADGENSELECT_TX2(0);

			}



			intel_de_rmw(dev_priv, DKL_TX_DPCNTL2(tc_port),

			intel_dkl_phy_rmw(dev_priv, DKL_TX_DPCNTL2(tc_port), ln,

					  DKL_TX_DPCNTL2_CFG_LOADGENSELECT_TX1_MASK |

					  DKL_TX_DPCNTL2_CFG_LOADGENSELECT_TX2_MASK,

					  val);
@@ -2019,12 +2017,8 @@ icl_program_mg_dp_mode(struct intel_digital_port *dig_port, 
		return;



	if (DISPLAY_VER(dev_priv) >= 12) {

		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, 0x0));

		ln0 = intel_de_read(dev_priv, DKL_DP_MODE(tc_port));

		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, 0x1));

		ln1 = intel_de_read(dev_priv, DKL_DP_MODE(tc_port));

		ln0 = intel_dkl_phy_read(dev_priv, DKL_DP_MODE(tc_port), 0);

		ln1 = intel_dkl_phy_read(dev_priv, DKL_DP_MODE(tc_port), 1);

	} else {

		ln0 = intel_de_read(dev_priv, MG_DP_MODE(0, tc_port));

		ln1 = intel_de_read(dev_priv, MG_DP_MODE(1, tc_port));
@@ -2085,12 +2079,8 @@ icl_program_mg_dp_mode(struct intel_digital_port *dig_port, 
	}



	if (DISPLAY_VER(dev_priv) >= 12) {

		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, 0x0));

		intel_de_write(dev_priv, DKL_DP_MODE(tc_port), ln0);

		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, 0x1));

		intel_de_write(dev_priv, DKL_DP_MODE(tc_port), ln1);

		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port), 0, ln0);

		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port), 1, ln1);

	} else {

		intel_de_write(dev_priv, MG_DP_MODE(0, tc_port), ln0);

		intel_de_write(dev_priv, MG_DP_MODE(1, tc_port), ln1);
@@ -3094,10 +3084,8 @@ static void adlp_tbt_to_dp_alt_switch_wa(struct intel_encoder *encoder) 
	enum tc_port tc_port = intel_port_to_tc(i915, encoder->port);

	int ln;



	for (ln = 0; ln < 2; ln++) {

		intel_de_write(i915, HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln));

		intel_de_rmw(i915, DKL_PCS_DW5(tc_port), DKL_PCS_DW5_CORE_SOFTRESET, 0);

	}

	for (ln = 0; ln < 2; ln++)

		intel_dkl_phy_rmw(i915, DKL_PCS_DW5(tc_port), ln, DKL_PCS_DW5_CORE_SOFTRESET, 0);

}



static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp,

drivers/gpu/drm/i915/display/intel_display_core.h

+8 −0
Original line number Diff line number Diff line
@@ -315,6 +315,14 @@ struct intel_display { 
		struct intel_global_obj obj;

	} dbuf;



	struct {

		/*

		 * dkl.phy_lock protects against concurrent access of the

		 * Dekel TypeC PHYs.

		 */

		spinlock_t phy_lock;

	} dkl;



	struct {

		/* VLV/CHV/BXT/GLK DSI MMIO register base address */

		u32 mmio_base;

drivers/gpu/drm/i915/display/intel_display_power_well.c

+3 −4
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ 
#include "intel_de.h"

#include "intel_display_power_well.h"

#include "intel_display_types.h"

#include "intel_dkl_phy.h"

#include "intel_dmc.h"

#include "intel_dpio_phy.h"

#include "intel_dpll.h"
@@ -529,10 +530,8 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, 
		enum tc_port tc_port;



		tc_port = TGL_AUX_PW_TO_TC_PORT(i915_power_well_instance(power_well)->hsw.idx);

		intel_de_write(dev_priv, HIP_INDEX_REG(tc_port),

			       HIP_INDEX_VAL(tc_port, 0x2));



		if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port),

		if (wait_for(intel_dkl_phy_read(dev_priv, DKL_CMN_UC_DW_27(tc_port), 2) &

			     DKL_CMN_UC_DW27_UC_HEALTH, 1))

			drm_warn(&dev_priv->drm,

				 "Timeout waiting TC uC health\n");

drivers/gpu/drm/i915/display/intel_dkl_phy.c

0 → 100644
+109 −0
Original line number Diff line number Diff line 
// SPDX-License-Identifier: MIT

/*

 * Copyright © 2022 Intel Corporation

 */



#include "i915_drv.h"

#include "i915_reg.h"



#include "intel_de.h"

#include "intel_display.h"

#include "intel_dkl_phy.h"



static void

dkl_phy_set_hip_idx(struct drm_i915_private *i915, i915_reg_t reg, int idx)

{

	enum tc_port tc_port = DKL_REG_TC_PORT(reg);



	drm_WARN_ON(&i915->drm, tc_port < TC_PORT_1 || tc_port >= I915_MAX_TC_PORTS);



	intel_de_write(i915,

		       HIP_INDEX_REG(tc_port),

		       HIP_INDEX_VAL(tc_port, idx));

}



/**

 * intel_dkl_phy_read - read a Dekel PHY register

 * @i915: i915 device instance

 * @reg: Dekel PHY register

 * @ln: lane instance of @reg

 *

 * Read the @reg Dekel PHY register.

 *

 * Returns the read value.

 */

u32

intel_dkl_phy_read(struct drm_i915_private *i915, i915_reg_t reg, int ln)

{

	u32 val;



	spin_lock(&i915->display.dkl.phy_lock);



	dkl_phy_set_hip_idx(i915, reg, ln);

	val = intel_de_read(i915, reg);



	spin_unlock(&i915->display.dkl.phy_lock);



	return val;

}



/**

 * intel_dkl_phy_write - write a Dekel PHY register

 * @i915: i915 device instance

 * @reg: Dekel PHY register

 * @ln: lane instance of @reg

 * @val: value to write

 *

 * Write @val to the @reg Dekel PHY register.

 */

void

intel_dkl_phy_write(struct drm_i915_private *i915, i915_reg_t reg, int ln, u32 val)

{

	spin_lock(&i915->display.dkl.phy_lock);



	dkl_phy_set_hip_idx(i915, reg, ln);

	intel_de_write(i915, reg, val);



	spin_unlock(&i915->display.dkl.phy_lock);

}



/**

 * intel_dkl_phy_rmw - read-modify-write a Dekel PHY register

 * @i915: i915 device instance

 * @reg: Dekel PHY register

 * @ln: lane instance of @reg

 * @clear: mask to clear

 * @set: mask to set

 *

 * Read the @reg Dekel PHY register, clearing then setting the @clear/@set bits in it, and writing

 * this value back to the register if the value differs from the read one.

 */

void

intel_dkl_phy_rmw(struct drm_i915_private *i915, i915_reg_t reg, int ln, u32 clear, u32 set)

{

	spin_lock(&i915->display.dkl.phy_lock);



	dkl_phy_set_hip_idx(i915, reg, ln);

	intel_de_rmw(i915, reg, clear, set);



	spin_unlock(&i915->display.dkl.phy_lock);

}



/**

 * intel_dkl_phy_posting_read - do a posting read from a Dekel PHY register

 * @i915: i915 device instance

 * @reg: Dekel PHY register

 * @ln: lane instance of @reg

 *

 * Read the @reg Dekel PHY register without returning the read value.

 */

void

intel_dkl_phy_posting_read(struct drm_i915_private *i915, i915_reg_t reg, int ln)

{

	spin_lock(&i915->display.dkl.phy_lock);



	dkl_phy_set_hip_idx(i915, reg, ln);

	intel_de_posting_read(i915, reg);



	spin_unlock(&i915->display.dkl.phy_lock);

}