On Wed, Jul 03, 2019 at 05:06:48PM -0700, Lucas De Marchi wrote:
> PORT_TX_DFLEXDPMLE1 is a FIA register so move it to intel_tc.c where we
> access other FIA registers. In Tiger Lake we have multiple/modular FIAs
> so it makes sense to start moving all access to their registers to a
> common place.
>
> While at it, make it clear that we will only ever call this function
> for ports with TC phy. Previously we were relying on tc_mode being
> TC_PORT_TBT_ALT for combo phy ports. However it's confusing since in
> this same function we have checks for is_tc_port. Also, if we manage to
> make each phy access only their own field, we may in future add them as
> a union inside intel_digital_port.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 49 ++++--------------------
> drivers/gpu/drm/i915/display/intel_tc.c | 31 +++++++++++++++
> drivers/gpu/drm/i915/display/intel_tc.h | 2 +
> 3 files changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a4172595c8d8..f6d60b71da7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3594,37 +3594,6 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> intel_hdcp_disable(to_intel_connector(conn_state->connector));
> }
>
> -static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
> - const struct intel_crtc_state *pipe_config,
> - enum port port)
> -{
> - struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> - enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> - u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
> - bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> -
> - WARN_ON(lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
> -
> - val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> - switch (pipe_config->lane_count) {
> - case 1:
> - val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> - DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> - break;
> - case 2:
> - val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> - DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> - break;
> - case 4:
> - val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
> - break;
> - default:
> - MISSING_CASE(pipe_config->lane_count);
> - }
> - I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
> -}
> -
> static void
> intel_ddi_update_prepare(struct intel_atomic_state *state,
> struct intel_encoder *encoder,
> @@ -3657,7 +3626,6 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> bool is_tc_port = intel_port_is_tc(dev_priv, encoder->port);
> - enum port port = encoder->port;
>
> if (is_tc_port)
> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> @@ -3666,18 +3634,15 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> intel_display_power_get(dev_priv,
> intel_ddi_main_link_aux_domain(dig_port));
>
> - if (IS_GEN9_LP(dev_priv))
> + if (is_tc_port && dig_port->tc_mode != TC_PORT_TBT_ALT)
> + /*
> + * Program the lane count for static/dynamic connections on
> + * Type-C ports. Skip this step for TBT.
> + */
> + intel_tc_port_set_fia_lane_count(dig_port, crtc_state->lane_count);
> + else if (IS_GEN9_LP(dev_priv))
> bxt_ddi_phy_set_lane_optim_mask(encoder,
> crtc_state->lane_lat_optim_mask);
> -
> - /*
> - * Program the lane count for static/dynamic connections on Type-C ports.
> - * Skip this step for TBT.
> - */
> - if (dig_port->tc_mode == TC_PORT_TBT_ALT)
> - return;
> -
> - intel_ddi_set_fia_lane_count(encoder, crtc_state, port);
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index e6e6163c1232..ba6932f48e90 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -68,6 +68,37 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
> }
> }
>
> +void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
> + int required_lanes)
> +{
> + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
> + bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> + struct intel_uncore *uncore = &i915->uncore;
> + u32 val = intel_uncore_read(uncore, PORT_TX_DFLEXDPMLE1);
The rule I follow now is not to do reg read/writes in the declaration
part, based on Jani's suggestion. Could fix that too while at it.
> +
> + WARN_ON(lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
> +
> + val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> + switch (required_lanes) {
> + case 1:
> + val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> + DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> + break;
> + case 2:
> + val |= (lane_reversal) ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> + DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
Could fix the indent error while at it?
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> + break;
> + case 4:
> + val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
> + break;
> + default:
> + MISSING_CASE(required_lanes);
> + }
> +
> + intel_uncore_write(uncore, PORT_TX_DFLEXDPMLE1, val);
> +}
> +
> static void tc_port_fixup_legacy_flag(struct intel_digital_port *dig_port,
> u32 live_status_mask)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> index 45ae30537b78..df4d97971354 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -14,6 +14,8 @@
> bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
> int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
> +void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
> + int required_lanes);
>
> void intel_tc_port_sanitize(struct intel_digital_port *dig_port);
> void intel_tc_port_lock(struct intel_digital_port *dig_port);
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
