Re: [PATCH v2 02/13] drm/i915/tgl: Finish modular FIA support on registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux