[Intel-gfx] [PATCH 1/4] drm/i915/cx0: Add intel_cx0_get_owned_lane_mask()

[Kahola, Mika]

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa at intel.com>
> Sent: Thursday, August 3, 2023 5:03 PM
> To: Taylor, Clinton A <clinton.a.taylor at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; Kahola, Mika <mika.kahola at intel.com>
> Subject: Re: [PATCH 1/4] drm/i915/cx0: Add intel_cx0_get_owned_lane_mask()
> 
> Quoting Taylor, Clinton A (2023-08-02 18:41:27-03:00)
> >On Tue, 2023-07-25 at 18:27 -0300, Gustavo Sousa wrote:
> >> There are more parts of C10/C20 programming that need to take owned
> >> lanes into account. Define the function
> >> intel_cx0_get_owned_lane_mask() and use it. There will be new users
> >> of that function in upcoming changes.
> >>
> >> BSpec: 64539
> >> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 44
> >> ++++++++++++--------
> >>  1 file changed, 27 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> index 1b00ef2c6185..b903ceb0b56a 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> @@ -46,6 +46,22 @@ static int lane_mask_to_lane(u8 lane_mask)
> >>          return ilog2(lane_mask);
> >>  }
> >>
> >> +static u8 intel_cx0_get_owned_lane_mask(struct drm_i915_private *i915,
> >> +                                        struct intel_encoder
> >> +*encoder) {
> >> +        struct intel_digital_port *dig_port =
> >> +enc_to_dig_port(encoder);
> >> +
> >> +        if (!intel_tc_port_in_dp_alt_mode(dig_port))
> >> +                return INTEL_CX0_BOTH_LANES;
> >> +
> >> +        /*
> >> +         * In DP-alt with pin assignment D, only PHY lane 0 is owned
> >> +         * by display and lane 1 is owned by USB.
> >> +         */
> > lane_revesal is not being handled here. Do we need to take
> >lane_reversal into account with Pin assignment D is being used?
> 
> According to the BSpec, we should only care about lane reversal on native connections, and both lanes are always owned for
> those cases.

That's correct. The patch looks ok to me.

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

> 
> In Type-C DP-alt mode, FIA handles stuff so that display hardware and software does not need to handle lane reversal.
> 
> --
> Gustavo Sousa
> 
> >
> >-Clint
> >
> >> +        return intel_tc_port_fia_max_lane_count(dig_port) > 2
> >> +                ? INTEL_CX0_BOTH_LANES : INTEL_CX0_LANE0; }
> >> +
> >>  static void
> >>  assert_dc_off(struct drm_i915_private *i915)  { @@ -2534,17 +2550,15
> >> @@ static void intel_cx0_phy_lane_reset(struct drm_i915_private
> >> *i915,  {
> >>          enum port port = encoder->port;
> >>          enum phy phy = intel_port_to_phy(i915, port);
> >> -        bool both_lanes =  intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder)) > 2;
> >> -        u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> >> -                                  INTEL_CX0_LANE0;
> >> -        u32 lane_pipe_reset = both_lanes ?
> >> -                              XELPDP_LANE_PIPE_RESET(0) |
> >> -                              XELPDP_LANE_PIPE_RESET(1) :
> >> -                              XELPDP_LANE_PIPE_RESET(0);
> >> -        u32 lane_phy_current_status = both_lanes ?
> >> -                                      XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> >> -                                      XELPDP_LANE_PHY_CURRENT_STATUS(1) :
> >> -                                      XELPDP_LANE_PHY_CURRENT_STATUS(0);
> >> +        u8 owned_lane_mask = intel_cx0_get_owned_lane_mask(i915, encoder);
> >> +        u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 : INTEL_CX0_LANE0;
> >> +        u32 lane_pipe_reset = owned_lane_mask == INTEL_CX0_BOTH_LANES
> >> +                                ? XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1)
> >> +                                : XELPDP_LANE_PIPE_RESET(0);
> >> +        u32 lane_phy_current_status = owned_lane_mask == INTEL_CX0_BOTH_LANES
> >> +                                        ? (XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> >> +                                           XELPDP_LANE_PHY_CURRENT_STATUS(1))
> >> +                                        :
> >> + XELPDP_LANE_PHY_CURRENT_STATUS(0);
> >>
> >>          if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL1(port),
> >>
> >> XELPDP_PORT_BUF_SOC_PHY_READY, @@ -2564,15 +2578,11 @@ static void
> >> intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
> >>                           phy_name(phy),
> >> XELPDP_PORT_RESET_START_TIMEOUT_US);
> >>
> >>          intel_de_rmw(i915, XELPDP_PORT_CLOCK_CTL(port),
> >> -                     intel_cx0_get_pclk_refclk_request(both_lanes ?
> >> -                                                       INTEL_CX0_BOTH_LANES :
> >> -                                                       INTEL_CX0_LANE0),
> >> +
> >> + intel_cx0_get_pclk_refclk_request(owned_lane_mask),
> >>                       intel_cx0_get_pclk_refclk_request(lane_mask));
> >>
> >>          if (__intel_de_wait_for_register(i915, XELPDP_PORT_CLOCK_CTL(port),
> >> -                                         intel_cx0_get_pclk_refclk_ack(both_lanes ?
> >> -                                                                       INTEL_CX0_BOTH_LANE
> >> S :
> >> -                                                                       INTEL_CX0_LANE0),
> >> +
> >> + intel_cx0_get_pclk_refclk_ack(owned_lane_mask),
> >>                                           intel_cx0_get_pclk_refclk_ack(lane_mask),
> >>                                           XELPDP_REFCLK_ENABLE_TIMEOUT_US, 0, NULL))
> >>                  drm_warn(&i915->drm, "PHY %c failed to request
> >> refclk after %dus.\n",