[Intel-gfx] [PATCH v2 2/4] drm/i915: Simplify intel_cx0_program_phy_lane() with loop

[Kahola, Mika]

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa at intel.com>
> Sent: Monday, August 14, 2023 4:13 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula at intel.com>; Kahola, Mika <mika.kahola at intel.com>
> Subject: [PATCH v2 2/4] drm/i915: Simplify intel_cx0_program_phy_lane() with loop
> 
> It is possible to generalize the "disable" value for the transmitters to be a bit mask based on the port width and the port reversal
> boolean, with a small exception for DP-alt mode with "x1" port width.
> 
> Simplify the code by using such a mask and a for-loop instead of using switch-case statements.
> 
> v2:
>   - Use (i < 2) instead of (i / 2 == 0) for PHY lane mask selection.
>     (Jani)
> 
> BSpec: 64539
> Cc: Jani Nikula <jani.nikula at intel.com>

Looks ok to me.

Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 79 +++++---------------
>  1 file changed, 20 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index b903ceb0b56a..2b112ed78943 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2604,7 +2604,8 @@ static void intel_cx0_program_phy_lane(struct drm_i915_private *i915,
>  				       struct intel_encoder *encoder, int lane_count,
>  				       bool lane_reversal)
>  {
> -	u8 l0t1, l0t2, l1t1, l1t2;
> +	int i;
> +	u8 disables;
>  	bool dp_alt_mode = intel_tc_port_in_dp_alt_mode(enc_to_dig_port(encoder));
>  	enum port port = encoder->port;
> 
> @@ -2614,66 +2615,26 @@ static void intel_cx0_program_phy_lane(struct drm_i915_private *i915,
>  			      C10_VDR_CTRL_MSGBUS_ACCESS,
>  			      MB_WRITE_COMMITTED);
> 
> -	/* TODO: DP-alt MFD case where only one PHY lane should be programmed. */
> -	l0t1 = intel_cx0_read(i915, port, INTEL_CX0_LANE0, PHY_CX0_TX_CONTROL(1, 2));
> -	l0t2 = intel_cx0_read(i915, port, INTEL_CX0_LANE0, PHY_CX0_TX_CONTROL(2, 2));
> -	l1t1 = intel_cx0_read(i915, port, INTEL_CX0_LANE1, PHY_CX0_TX_CONTROL(1, 2));
> -	l1t2 = intel_cx0_read(i915, port, INTEL_CX0_LANE1, PHY_CX0_TX_CONTROL(2, 2));
> -
> -	l0t1 |= CONTROL2_DISABLE_SINGLE_TX;
> -	l0t2 |= CONTROL2_DISABLE_SINGLE_TX;
> -	l1t1 |= CONTROL2_DISABLE_SINGLE_TX;
> -	l1t2 |= CONTROL2_DISABLE_SINGLE_TX;
> -
> -	if (lane_reversal) {
> -		switch (lane_count) {
> -		case 4:
> -			l0t1 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			fallthrough;
> -		case 3:
> -			l0t2 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			fallthrough;
> -		case 2:
> -			l1t1 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			fallthrough;
> -		case 1:
> -			l1t2 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			break;
> -		default:
> -			MISSING_CASE(lane_count);
> -		}
> -	} else {
> -		switch (lane_count) {
> -		case 4:
> -			l1t2 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			fallthrough;
> -		case 3:
> -			l1t1 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			fallthrough;
> -		case 2:
> -			l0t2 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			l0t1 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			break;
> -		case 1:
> -			if (dp_alt_mode)
> -				l0t2 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			else
> -				l0t1 &= ~CONTROL2_DISABLE_SINGLE_TX;
> -			break;
> -		default:
> -			MISSING_CASE(lane_count);
> -		}
> +	if (lane_reversal)
> +		disables = REG_GENMASK8(3, 0) >> lane_count;
> +	else
> +		disables = REG_GENMASK8(3, 0) << lane_count;
> +
> +	if (dp_alt_mode && lane_count == 1) {
> +		disables &= ~REG_GENMASK8(1, 0);
> +		disables |= REG_FIELD_PREP8(REG_GENMASK8(1, 0), 0x1);
>  	}
> 
> -	/* disable MLs */
> -	intel_cx0_write(i915, port, INTEL_CX0_LANE0, PHY_CX0_TX_CONTROL(1, 2),
> -			l0t1, MB_WRITE_COMMITTED);
> -	intel_cx0_write(i915, port, INTEL_CX0_LANE0, PHY_CX0_TX_CONTROL(2, 2),
> -			l0t2, MB_WRITE_COMMITTED);
> -	intel_cx0_write(i915, port, INTEL_CX0_LANE1, PHY_CX0_TX_CONTROL(1, 2),
> -			l1t1, MB_WRITE_COMMITTED);
> -	intel_cx0_write(i915, port, INTEL_CX0_LANE1, PHY_CX0_TX_CONTROL(2, 2),
> -			l1t2, MB_WRITE_COMMITTED);
> +	/* TODO: DP-alt MFD case where only one PHY lane should be programmed. */
> +	for (i = 0; i < 4; i++) {
> +		int tx = i % 2 + 1;
> +		u8 lane_mask = i < 2 ? INTEL_CX0_LANE0 : INTEL_CX0_LANE1;
> +
> +		intel_cx0_rmw(i915, port, lane_mask, PHY_CX0_TX_CONTROL(tx, 2),
> +			      CONTROL2_DISABLE_SINGLE_TX,
> +			      disables & BIT(i) ? CONTROL2_DISABLE_SINGLE_TX : 0,
> +			      MB_WRITE_COMMITTED);
> +	}
> 
>  	if (intel_is_c10phy(i915, intel_port_to_phy(i915, port)))
>  		intel_cx0_rmw(i915, port, INTEL_CX0_BOTH_LANES,
> --
> 2.41.0