| Message ID | 20230725212716.3060259-2-gustavo.sousa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix C10/C20 implementation w.r.t. owned PHY lanes | expand |
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@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? -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",
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@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. 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",
> -----Original Message----- > From: Sousa, Gustavo <gustavo.sousa@intel.com> > Sent: Thursday, August 3, 2023 5:03 PM > To: Taylor, Clinton A <clinton.a.taylor@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; Kahola, Mika <mika.kahola@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@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@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",
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. + */ + 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_LANES : - 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",
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@intel.com> --- drivers/gpu/drm/i915/display/intel_cx0_phy.c | 44 ++++++++++++-------- 1 file changed, 27 insertions(+), 17 deletions(-)