On Tue, 25 Jul 2023, Gustavo Sousa <[email protected]> wrote:
> 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.
>
> BSpec: 64539
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
> 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..f10ebdfd696a 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 == 0 ? INTEL_CX0_LANE0 : INTEL_CX0_LANE1;
I'm just catching up on mails and quickly eyeballing stuff, but
i / 2 == 0
looks suspect.
BR,
Jani.
> +
> + 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,
--
Jani Nikula, Intel Open Source Graphics Center
