Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
> -Original Message-
> From: Luca Coelho
> Sent: Tuesday, May 30, 2023 1:08 PM
> To: Kahola, Mika ; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of
> MFD
>
> On Tue, 2023-05-30 at 09:30 +, Kahola, Mika wrote:
> > > -Original Message-
> > > From: Luca Coelho
> > > Sent: Tuesday, May 30, 2023 11:38 AM
> > > To: Kahola, Mika ; intel-
> > > g...@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane
> > > in case of MFD
> > >
> > > Looks good, I only have some nitpicks.
> > >
> > > On Wed, 2023-05-24 at 18:01 +0300, Mika Kahola wrote:
> > > > In case when only two or less lanes are owned such as MFD (DP-alt
> > > > with
> > > > x2 lanes) we need to reset only one lane (lane0). With only x2
> > > > lanes we don't need to poll for the phy current status on both
> > > > lanes since only the owned lane will respond.
> > >
> > > It would be nice to explain why it is so.
> > >
> > >
> > > > Signed-off-by: Mika Kahola
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> > > >
> > > > 1 file changed, 23 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > index ee6902118860..b8c812c5b33f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > @@ -2528,13 +2528,23 @@ static u32
> > > > intel_cx0_get_pclk_refclk_ack(u8 lane_mask)
> > > > return val;
> > > > }
> > > >
> > > > -/* FIXME: Some Type-C cases need not reset both the lanes.
> > > > Handle
> > > > those cases. */ -static void intel_cx0_phy_lane_reset(struct
> > > > drm_i915_private *i915, enum port port,
> > > > +static void intel_cx0_phy_lane_reset(struct drm_i915_private
> > > > *i915,
> > > > +struct intel_encoder
> > > > *encoder,
> > > > bool lane_reversal)
> > > > {
> > > > + enum port port = encoder->port;
> > > > enum phy phy = intel_port_to_phy(i915, port);
> > > > + u8 fia_max =
> > > > +intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
> > >
> > > Logically, we don't care about "fia_max" in this function, we only
> > > care whether one or two lanes should be handled. In all places we
> > > use fia_max, we only check if it is > 2. So I think it would be
> > > clearer to have the > 2 here already and call the variable something
> > > else.
> > >
> > > Additionally, "> 2" looks slightly magic (without knowing the
> > > reason, as stated in my first comment). Is there any more self-
> > > explanatory symbol we could used?
> > I admit, it's not that clear when you look at it first time. It only
> > means that all lanes are in use and we should in that case reset all
> > lanes. Maybe switching to Boolean instead something like this
> >
> > bool all_lanes =
> > intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder)) > 2;
> >
> > And do the reset routines based on this?
>
> Sounds good.
>
>
> > > > u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> > > > INTEL_CX0_LANE0;
> > > > + u32 lane_pipe_reset = fia_max > 2 ?
> > > > + XELPDP_LANE_PIPE_RESET(0) |
> > > > + XELPDP_LANE_PIPE_RESET(1) :
> > > > + XELPDP_LANE_PIPE_RESET(0);
> > > > + u32 lane_phy_current_status = fia_max > 2 ?
> > > > +
> > > > XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> > > > +
> > > > XELPDP_LANE_PHY_CURRENT_STATUS(1) :
> > > > +
> > > > XELPDP_LANE_PHY_CURRENT_STATUS(0);
> > >
> > > It was already logically like this in the code, so not directly
> > > related to this patch, but is there any reason why we don't need to
> > > include more lanes in the reset? I mean, we're only handling lanes
> > > 0 and 1. If we have 4 lanes, the other two never need to be reset?
> > There are lanes and then there are lanes. FIA has 4 main lanes with
> > are muxed into 2 data lanes and here we only reset these data lanes.
> > It's confusing as we have lanes defined for two different meanings.
>
> Okay, that clarifies it! We should probably have been calling them fia_lanes
> and data_lanes to distinguish, but make these
> changes now.
I will try to clarify these naming conventions. I spin another round of this
patch which hopefully is more self-explanatory.
-Mika-
>
> This also clarifies why we reset only one or both data lanes. A small
> paragraph explaining this would be nice in the commit log
> and/or as a comment in the function.
>
>
> > Thanks for the review and comments!
>
> You're welcome! You're helping me learn.
>
> --
> Cheers,
> Luca.
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
On Tue, 2023-05-30 at 09:30 +, Kahola, Mika wrote:
> > -Original Message-
> > From: Luca Coelho
> > Sent: Tuesday, May 30, 2023 11:38 AM
> > To: Kahola, Mika ; intel-
> > g...@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane
> > in case of MFD
> >
> > Looks good, I only have some nitpicks.
> >
> > On Wed, 2023-05-24 at 18:01 +0300, Mika Kahola wrote:
> > > In case when only two or less lanes are owned such as MFD (DP-alt
> > > with
> > > x2 lanes) we need to reset only one lane (lane0). With only x2
> > > lanes
> > > we don't need to poll for the phy current status on both lanes
> > > since
> > > only the owned lane will respond.
> >
> > It would be nice to explain why it is so.
> >
> >
> > > Signed-off-by: Mika Kahola
> > > ---
> > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> > >
> > > 1 file changed, 23 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index ee6902118860..b8c812c5b33f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -2528,13 +2528,23 @@ static u32
> > > intel_cx0_get_pclk_refclk_ack(u8 lane_mask)
> > > return val;
> > > }
> > >
> > > -/* FIXME: Some Type-C cases need not reset both the lanes.
> > > Handle
> > > those cases. */ -static void intel_cx0_phy_lane_reset(struct
> > > drm_i915_private *i915, enum port port,
> > > +static void intel_cx0_phy_lane_reset(struct drm_i915_private
> > > *i915,
> > > + struct intel_encoder
> > > *encoder,
> > > bool lane_reversal)
> > > {
> > > + enum port port = encoder->port;
> > > enum phy phy = intel_port_to_phy(i915, port);
> > > + u8 fia_max =
> > > +intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
> >
> > Logically, we don't care about "fia_max" in this function, we only
> > care whether one or two lanes should be handled. In all places
> > we use fia_max, we only check if it is > 2. So I think it would be
> > clearer to have the > 2 here already and call the variable
> > something else.
> >
> > Additionally, "> 2" looks slightly magic (without knowing the
> > reason, as stated in my first comment). Is there any more self-
> > explanatory symbol we could used?
> I admit, it's not that clear when you look at it first time. It only
> means that all lanes are in use and we should in that case reset all
> lanes. Maybe switching to Boolean instead something like this
>
> bool all_lanes =
> intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder)) > 2;
>
> And do the reset routines based on this?
Sounds good.
> > > u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> > > INTEL_CX0_LANE0;
> > > + u32 lane_pipe_reset = fia_max > 2 ?
> > > + XELPDP_LANE_PIPE_RESET(0) |
> > > + XELPDP_LANE_PIPE_RESET(1) :
> > > + XELPDP_LANE_PIPE_RESET(0);
> > > + u32 lane_phy_current_status = fia_max > 2 ?
> > > +
> > > XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> > > +
> > > XELPDP_LANE_PHY_CURRENT_STATUS(1) :
> > > +
> > > XELPDP_LANE_PHY_CURRENT_STATUS(0);
> >
> > It was already logically like this in the code, so not directly
> > related to this patch, but is there any reason why we don't need to
> > include more lanes in the reset? I mean, we're only handling lanes
> > 0 and 1. If we have 4 lanes, the other two never need to be
> > reset?
> There are lanes and then there are lanes. FIA has 4 main lanes with
> are muxed into 2 data lanes and here we only reset these data lanes.
> It's confusing as we have lanes defined for two different meanings.
Okay, that clarifies it! We should probably have been calling them
fia_lanes and data_lanes to distinguish, but make these changes now.
This also clarifies why we reset only one or both data lanes. A small
paragraph explaining this would be nice in the commit log and/or as a
comment in the function.
> Thanks for the review and comments!
You're welcome! You're helping me learn.
--
Cheers,
Luca.
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
> -Original Message-
> From: Luca Coelho
> Sent: Tuesday, May 30, 2023 11:38 AM
> To: Kahola, Mika ; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of
> MFD
>
> Looks good, I only have some nitpicks.
>
> On Wed, 2023-05-24 at 18:01 +0300, Mika Kahola wrote:
> > In case when only two or less lanes are owned such as MFD (DP-alt with
> > x2 lanes) we need to reset only one lane (lane0). With only x2 lanes
> > we don't need to poll for the phy current status on both lanes since
> > only the owned lane will respond.
>
> It would be nice to explain why it is so.
>
>
> > Signed-off-by: Mika Kahola
> > ---
> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> >
> > 1 file changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index ee6902118860..b8c812c5b33f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -2528,13 +2528,23 @@ static u32 intel_cx0_get_pclk_refclk_ack(u8
> > lane_mask)
> > return val;
> > }
> >
> > -/* FIXME: Some Type-C cases need not reset both the lanes. Handle
> > those cases. */ -static void intel_cx0_phy_lane_reset(struct
> > drm_i915_private *i915, enum port port,
> > +static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
> > +struct intel_encoder *encoder,
> > bool lane_reversal)
> > {
> > + enum port port = encoder->port;
> > enum phy phy = intel_port_to_phy(i915, port);
> > + u8 fia_max =
> > +intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
>
> Logically, we don't care about "fia_max" in this function, we only care
> whether one or two lanes should be handled. In all places
> we use fia_max, we only check if it is > 2. So I think it would be clearer
> to have the > 2 here already and call the variable
> something else.
>
> Additionally, "> 2" looks slightly magic (without knowing the reason, as
> stated in my first comment). Is there any more self-
> explanatory symbol we could used?
I admit, it's not that clear when you look at it first time. It only means that
all lanes are in use and we should in that case reset all lanes. Maybe
switching to Boolean instead something like this
bool all_lanes = intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder)) > 2;
And do the reset routines based on this?
>
> > u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> > INTEL_CX0_LANE0;
> > + u32 lane_pipe_reset = fia_max > 2 ?
> > + XELPDP_LANE_PIPE_RESET(0) |
> > + XELPDP_LANE_PIPE_RESET(1) :
> > + XELPDP_LANE_PIPE_RESET(0);
> > + u32 lane_phy_current_status = fia_max > 2 ?
> > + XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> > + XELPDP_LANE_PHY_CURRENT_STATUS(1) :
> > + XELPDP_LANE_PHY_CURRENT_STATUS(0);
>
> It was already logically like this in the code, so not directly related to
> this patch, but is there any reason why we don't need to
> include more lanes in the reset? I mean, we're only handling lanes 0 and 1.
> If we have 4 lanes, the other two never need to be
> reset?
There are lanes and then there are lanes. FIA has 4 main lanes with are muxed
into 2 data lanes and here we only reset these data lanes. It's confusing as we
have lanes defined for two different meanings.
Thanks for the review and comments!
-Mika-
>
> --
> Cheers,
> Luca.
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
> -Original Message-
> From: Intel-gfx On Behalf Of Mika
> Kahola
> Sent: Wednesday, May 24, 2023 8:32 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
>
> In case when only two or less lanes are owned such as MFD (DP-alt with x2
> lanes) we need to reset only one lane (lane0). With only x2 lanes we don't
> need to poll for the phy current status on both lanes since only the owned
> lane will respond.
>
> Signed-off-by: Mika Kahola
> ---
Reviewed-by: Arun R Murthy
Thanks and Regards,
Arun R Murthy
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index ee6902118860..b8c812c5b33f 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2528,13 +2528,23 @@ static u32 intel_cx0_get_pclk_refclk_ack(u8
> lane_mask)
> return val;
> }
>
> -/* FIXME: Some Type-C cases need not reset both the lanes. Handle those
> cases. */ -static void intel_cx0_phy_lane_reset(struct drm_i915_private
> *i915, enum port port,
> +static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
> + struct intel_encoder *encoder,
>bool lane_reversal)
> {
> + enum port port = encoder->port;
> enum phy phy = intel_port_to_phy(i915, port);
> + u8 fia_max =
> +intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
> u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> INTEL_CX0_LANE0;
> + u32 lane_pipe_reset = fia_max > 2 ?
> + XELPDP_LANE_PIPE_RESET(0) |
> + XELPDP_LANE_PIPE_RESET(1) :
> + XELPDP_LANE_PIPE_RESET(0);
> + u32 lane_phy_current_status = fia_max > 2 ?
> + 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,
> @@ -2545,23 +2555,24 @@ static void intel_cx0_phy_lane_reset(struct
> drm_i915_private *i915, enum port po
>
> intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
>XELPDP_LANE_PIPE_RESET(0) |
> XELPDP_LANE_PIPE_RESET(1),
> - XELPDP_LANE_PIPE_RESET(0) |
> XELPDP_LANE_PIPE_RESET(1));
> + lane_pipe_reset);
>
> if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),
> -
> XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> -
> XELPDP_LANE_PHY_CURRENT_STATUS(1),
> -
> XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> -
> XELPDP_LANE_PHY_CURRENT_STATUS(1),
> + lane_phy_current_status,
> lane_phy_current_status,
>
> XELPDP_PORT_RESET_START_TIMEOUT_US, 0, NULL))
> drm_warn(>drm, "PHY %c failed to bring out of Lane
> reset after %dus.\n",
>phy_name(phy),
> XELPDP_PORT_RESET_START_TIMEOUT_US);
>
> intel_de_rmw(i915, XELPDP_PORT_CLOCK_CTL(port),
> -
> intel_cx0_get_pclk_refclk_request(INTEL_CX0_BOTH_LANES),
> + intel_cx0_get_pclk_refclk_request(fia_max > 2 ?
> +INTEL_CX0_BOTH_LANES :
> +INTEL_CX0_LANE0),
>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(INTEL_CX0_BOTH_LANES),
> +
> intel_cx0_get_pclk_refclk_ack(fia_max > 2 ?
> +
> INTEL_CX0_BOTH_LANES :
> +
> INTEL_CX0_LANE0),
>
> intel_cx0_get_pclk_refclk_ack(lane_mask),
>
> XELPDP_REFCLK_ENABLE_TIMEOUT_US, 0, NULL))
> drm_warn(>drm, "PHY %c failed to request refclk after
> %dus.\n", @@ -2571,13 +2582,9 @@ static void
> intel_cx0_phy_lane_reset(struct drm_i915_private *i915, enum port po
> CX0_P2_STATE_RESET);
> intel_cx0_setup_powerdown(i915, port);
>
> - intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
> - XELPDP_LANE_PIPE_RESET(0) |
> XELPDP_LANE_PIPE_RESET(1),
> - 0);
> + intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset,
> 0);
>
> - if (inte
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
Looks good, I only have some nitpicks.
On Wed, 2023-05-24 at 18:01 +0300, Mika Kahola wrote:
> In case when only two or less lanes are owned such as MFD (DP-alt with x2
> lanes)
> we need to reset only one lane (lane0). With only x2 lanes we don't need
> to poll for the phy current status on both lanes since only the owned lane
> will respond.
It would be nice to explain why it is so.
> Signed-off-by: Mika Kahola
> ---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index ee6902118860..b8c812c5b33f 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2528,13 +2528,23 @@ static u32 intel_cx0_get_pclk_refclk_ack(u8 lane_mask)
> return val;
> }
>
> -/* FIXME: Some Type-C cases need not reset both the lanes. Handle those
> cases. */
> -static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915, enum
> port port,
> +static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
> + struct intel_encoder *encoder,
>bool lane_reversal)
> {
> + enum port port = encoder->port;
> enum phy phy = intel_port_to_phy(i915, port);
> + u8 fia_max =
> intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
Logically, we don't care about "fia_max" in this function, we only care
whether one or two lanes should be handled. In all places we use
fia_max, we only check if it is > 2. So I think it would be clearer to
have the > 2 here already and call the variable something else.
Additionally, "> 2" looks slightly magic (without knowing the reason,
as stated in my first comment). Is there any more self-explanatory
symbol we could used?
> u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
> INTEL_CX0_LANE0;
> + u32 lane_pipe_reset = fia_max > 2 ?
> + XELPDP_LANE_PIPE_RESET(0) |
> + XELPDP_LANE_PIPE_RESET(1) :
> + XELPDP_LANE_PIPE_RESET(0);
> + u32 lane_phy_current_status = fia_max > 2 ?
> + XELPDP_LANE_PHY_CURRENT_STATUS(0) |
> + XELPDP_LANE_PHY_CURRENT_STATUS(1) :
> + XELPDP_LANE_PHY_CURRENT_STATUS(0);
It was already logically like this in the code, so not directly related
to this patch, but is there any reason why we don't need to include
more lanes in the reset? I mean, we're only handling lanes 0 and 1. If
we have 4 lanes, the other two never need to be reset?
--
Cheers,
Luca.
[Intel-gfx] [PATCH] drm/i915/mtl: Reset only one lane in case of MFD
In case when only two or less lanes are owned such as MFD (DP-alt with x2 lanes)
we need to reset only one lane (lane0). With only x2 lanes we don't need
to poll for the phy current status on both lanes since only the owned lane
will respond.
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index ee6902118860..b8c812c5b33f 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -2528,13 +2528,23 @@ static u32 intel_cx0_get_pclk_refclk_ack(u8 lane_mask)
return val;
}
-/* FIXME: Some Type-C cases need not reset both the lanes. Handle those cases.
*/
-static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915, enum port
port,
+static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
+struct intel_encoder *encoder,
bool lane_reversal)
{
+ enum port port = encoder->port;
enum phy phy = intel_port_to_phy(i915, port);
+ u8 fia_max =
intel_tc_port_fia_max_lane_count(enc_to_dig_port(encoder));
u8 lane_mask = lane_reversal ? INTEL_CX0_LANE1 :
INTEL_CX0_LANE0;
+ u32 lane_pipe_reset = fia_max > 2 ?
+ XELPDP_LANE_PIPE_RESET(0) |
+ XELPDP_LANE_PIPE_RESET(1) :
+ XELPDP_LANE_PIPE_RESET(0);
+ u32 lane_phy_current_status = fia_max > 2 ?
+ 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,
@@ -2545,23 +2555,24 @@ static void intel_cx0_phy_lane_reset(struct
drm_i915_private *i915, enum port po
intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
-XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1));
+lane_pipe_reset);
if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),
-XELPDP_LANE_PHY_CURRENT_STATUS(0) |
-XELPDP_LANE_PHY_CURRENT_STATUS(1),
-XELPDP_LANE_PHY_CURRENT_STATUS(0) |
-XELPDP_LANE_PHY_CURRENT_STATUS(1),
+lane_phy_current_status,
lane_phy_current_status,
XELPDP_PORT_RESET_START_TIMEOUT_US, 0,
NULL))
drm_warn(>drm, "PHY %c failed to bring out of Lane reset
after %dus.\n",
phy_name(phy), XELPDP_PORT_RESET_START_TIMEOUT_US);
intel_de_rmw(i915, XELPDP_PORT_CLOCK_CTL(port),
-intel_cx0_get_pclk_refclk_request(INTEL_CX0_BOTH_LANES),
+intel_cx0_get_pclk_refclk_request(fia_max > 2 ?
+ INTEL_CX0_BOTH_LANES :
+ INTEL_CX0_LANE0),
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(INTEL_CX0_BOTH_LANES),
+intel_cx0_get_pclk_refclk_ack(fia_max
> 2 ?
+
INTEL_CX0_BOTH_LANES :
+
INTEL_CX0_LANE0),
intel_cx0_get_pclk_refclk_ack(lane_mask),
XELPDP_REFCLK_ENABLE_TIMEOUT_US, 0,
NULL))
drm_warn(>drm, "PHY %c failed to request refclk after
%dus.\n",
@@ -2571,13 +2582,9 @@ static void intel_cx0_phy_lane_reset(struct
drm_i915_private *i915, enum port po
CX0_P2_STATE_RESET);
intel_cx0_setup_powerdown(i915, port);
- intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
-XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
-0);
+ intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset, 0);
- if (intel_de_wait_for_clear(i915, XELPDP_PORT_BUF_CTL2(port),
- XELPDP_LANE_PHY_CURRENT_STATUS(0) |
- XELPDP_LANE_PHY_CURRENT_STATUS(1),
+ if (intel_de_wait_for_clear(i915, XELPDP_PORT_BUF_CTL2(port),
