On Wed, Sep 18, 2019 at 5:07 PM José Roberto de Souza
<jose.souza@xxxxxxxxx> wrote:
>
> If platform supports and has modular FIA is enabled, the registers
> bits also change, example: reading TC3 registers with modular FIA
> enabled, driver should read from FIA2 but with TC1 bits offsets.
>
> It is described in BSpec 50231 for DFLEXDPSP, other registers don't
> have the BSpec description but testing in real hardware have proven
> that it had moved for all other registers too.
>
> v2:
> - Caching index in tc_phy_fia_idx, instead of calculate it each time
>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_tc.c | 46 +++++++++++--------
> drivers/gpu/drm/i915/display/intel_tc.h | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 28 +++++------
> 4 files changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d5cc4b810d9e..d258a48f77d3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1285,6 +1285,7 @@ struct intel_digital_port {
> char tc_port_name[8];
> enum tc_port_mode tc_mode;
> enum phy_fia tc_phy_fia;
> + u8 tc_phy_fia_idx;
>
> void (*write_infoframe)(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 85743a43bee2..20fbb084830e 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -45,10 +45,19 @@ static enum phy_fia tc_port_to_fia(struct drm_i915_private *i915,
> return tc_port / 2;
> }
>
> +static u8
> +tc_port_to_fia_idx(struct drm_i915_private *i915, enum tc_port tc_port)
> +{
> + if (!has_modular_fia(i915))
> + return tc_port;
> +
> + /* See tc_port_to_fia() comment */
> + return tc_port % 2;
> +}
> +
> u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
> {
> 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);
> struct intel_uncore *uncore = &i915->uncore;
> u32 lane_mask;
>
> @@ -57,8 +66,8 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>
> WARN_ON(lane_mask == 0xffffffff);
>
> - return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> - DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> + return (lane_mask & DP_LANE_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >>
> + DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> }
>
> int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
> @@ -95,7 +104,6 @@ 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;
> @@ -104,19 +112,21 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
>
> val = intel_uncore_read(uncore,
> PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
> - val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> + val &= ~DFLEXDPMLE1_DPMLETC_MASK(dig_port->tc_phy_fia_idx);
>
> switch (required_lanes) {
> case 1:
> - val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> - DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> + val |= lane_reversal ?
> + DFLEXDPMLE1_DPMLETC_ML3(dig_port->tc_phy_fia_idx) :
> + DFLEXDPMLE1_DPMLETC_ML0(dig_port->tc_phy_fia_idx);
> break;
> case 2:
> - val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> - DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> + val |= lane_reversal ?
> + DFLEXDPMLE1_DPMLETC_ML3_2(dig_port->tc_phy_fia_idx) :
> + DFLEXDPMLE1_DPMLETC_ML1_0(dig_port->tc_phy_fia_idx);
> break;
> case 4:
> - val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
> + val |= DFLEXDPMLE1_DPMLETC_ML3_0(dig_port->tc_phy_fia_idx);
> break;
> default:
> MISSING_CASE(required_lanes);
> @@ -164,9 +174,9 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
> return mask;
> }
>
> - if (val & TC_LIVE_STATE_TBT(tc_port))
> + if (val & TC_LIVE_STATE_TBT(dig_port->tc_phy_fia_idx))
> mask |= BIT(TC_PORT_TBT_ALT);
> - if (val & TC_LIVE_STATE_TC(tc_port))
> + if (val & TC_LIVE_STATE_TC(dig_port->tc_phy_fia_idx))
> mask |= BIT(TC_PORT_DP_ALT);
>
> if (intel_uncore_read(uncore, SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
> @@ -182,7 +192,6 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
> static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
> {
> 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);
> struct intel_uncore *uncore = &i915->uncore;
> u32 val;
>
> @@ -194,14 +203,13 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
> return false;
> }
>
> - return val & DP_PHY_MODE_STATUS_COMPLETED(tc_port);
> + return val & DP_PHY_MODE_STATUS_COMPLETED(dig_port->tc_phy_fia_idx);
> }
>
> static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
> bool enable)
> {
> 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);
> struct intel_uncore *uncore = &i915->uncore;
> u32 val;
>
> @@ -215,9 +223,9 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
> return false;
> }
>
> - val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> + val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx);
> if (!enable)
> - val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> + val |= DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx);
>
> intel_uncore_write(uncore,
> PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia), val);
> @@ -232,7 +240,6 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
> static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
> {
> 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);
> struct intel_uncore *uncore = &i915->uncore;
> u32 val;
>
> @@ -244,7 +251,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
> return true;
> }
>
> - return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
> + return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx));
> }
>
> /*
> @@ -541,4 +548,5 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
> dig_port->tc_legacy_port = is_legacy;
> dig_port->tc_link_refcount = 0;
> dig_port->tc_phy_fia = tc_port_to_fia(i915, tc_port);
> + dig_port->tc_phy_fia_idx = tc_port_to_fia_idx(i915, tc_port);
I think we could actually embed here tc_port_to_fia() and
tc_port_to_fia_idx() under a single has_modular_fia().
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> index 783d75531435..944d84c8cce1 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -10,6 +10,7 @@
> #include <linux/types.h>
>
> struct intel_digital_port;
> +struct drm_i915_private;
not needed anymore.
With that, Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Lucas De Marchi
>
> bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bf37ecebc82f..ee5626579263 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2166,13 +2166,13 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define _MMIO_FIA(fia, off) _MMIO(_FIA(fia) + (off))
>
> /* ICL PHY DFLEX registers */
> -#define PORT_TX_DFLEXDPMLE1(fia) _MMIO_FIA((fia), 0x008C0)
> -#define DFLEXDPMLE1_DPMLETC_MASK(tc_port) (0xf << (4 * (tc_port)))
> -#define DFLEXDPMLE1_DPMLETC_ML0(tc_port) (1 << (4 * (tc_port)))
> -#define DFLEXDPMLE1_DPMLETC_ML1_0(tc_port) (3 << (4 * (tc_port)))
> -#define DFLEXDPMLE1_DPMLETC_ML3(tc_port) (8 << (4 * (tc_port)))
> -#define DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) (12 << (4 * (tc_port)))
> -#define DFLEXDPMLE1_DPMLETC_ML3_0(tc_port) (15 << (4 * (tc_port)))
> +#define PORT_TX_DFLEXDPMLE1(fia) _MMIO_FIA((fia), 0x008C0)
> +#define DFLEXDPMLE1_DPMLETC_MASK(idx) (0xf << (4 * (idx)))
> +#define DFLEXDPMLE1_DPMLETC_ML0(idx) (1 << (4 * (idx)))
> +#define DFLEXDPMLE1_DPMLETC_ML1_0(idx) (3 << (4 * (idx)))
> +#define DFLEXDPMLE1_DPMLETC_ML3(idx) (8 << (4 * (idx)))
> +#define DFLEXDPMLE1_DPMLETC_ML3_2(idx) (12 << (4 * (idx)))
> +#define DFLEXDPMLE1_DPMLETC_ML3_0(idx) (15 << (4 * (idx)))
>
> /* BXT PHY Ref registers */
> #define _PORT_REF_DW3_A 0x16218C
> @@ -11671,16 +11671,16 @@ enum skl_power_gate {
>
> #define PORT_TX_DFLEXDPSP(fia) _MMIO_FIA((fia), 0x008A0)
> #define MODULAR_FIA_MASK (1 << 4)
> -#define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6))
> -#define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5))
> -#define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8)
> -#define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8))
> -#define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8))
> +#define TC_LIVE_STATE_TBT(idx) (1 << ((idx) * 8 + 6))
> +#define TC_LIVE_STATE_TC(idx) (1 << ((idx) * 8 + 5))
> +#define DP_LANE_ASSIGNMENT_SHIFT(idx) ((idx) * 8)
> +#define DP_LANE_ASSIGNMENT_MASK(idx) (0xf << ((idx) * 8))
> +#define DP_LANE_ASSIGNMENT(idx, x) ((x) << ((idx) * 8))
>
> #define PORT_TX_DFLEXDPPMS(fia) _MMIO_FIA((fia), 0x00890)
> -#define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port))
> +#define DP_PHY_MODE_STATUS_COMPLETED(idx) (1 << (idx))
>
> #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia), 0x00894)
> -#define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port))
> +#define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx))
>
> #endif /* _I915_REG_H_ */
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
