diff mbox series

[v2] drm/i915/display: Wait for PHY readiness not needed for disabling sequence

Message ID 20231212115130.485911-1-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915/display: Wait for PHY readiness not needed for disabling sequence | expand

Commit Message

Kahola, Mika Dec. 12, 2023, 11:51 a.m. UTC
When going through the disconnection flow we don't need to wait for PHY
readiness and hence we can skip the wait part. For disabling the function
returns false as an indicator that the power is not enabled. After all,
we are not even using the return value when Type-C is disconnecting.

v2: Cleanup for increased readibility (Imre)

BSpec: 65380

For VLK-53734

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/display/intel_tc.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Comments

Imre Deak Dec. 13, 2023, 7:13 a.m. UTC | #1
On Tue, Dec 12, 2023 at 01:51:30PM +0200, Mika Kahola wrote:
> When going through the disconnection flow we don't need to wait for PHY
> readiness and hence we can skip the wait part. For disabling the function
> returns false as an indicator that the power is not enabled. After all,
> we are not even using the return value when Type-C is disconnecting.
> 
> v2: Cleanup for increased readibility (Imre)
> 
> BSpec: 65380
> 
> For VLK-53734
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

For next time: it would've been better to separate to refactor + fix
patches. The change looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index f64d348a969e..dcf05e00e505 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -1030,18 +1030,25 @@ static bool xelpdp_tc_phy_enable_tcss_power(struct intel_tc_port *tc, bool enabl
>  
>  	__xelpdp_tc_phy_enable_tcss_power(tc, enable);
>  
> -	if ((!tc_phy_wait_for_ready(tc) ||
> -	     !xelpdp_tc_phy_wait_for_tcss_power(tc, enable)) &&
> -	    !drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY)) {
> -		if (enable) {
> -			__xelpdp_tc_phy_enable_tcss_power(tc, false);
> -			xelpdp_tc_phy_wait_for_tcss_power(tc, false);
> -		}
> +	if (enable && !tc_phy_wait_for_ready(tc))
> +		goto out_disable;
>  
> -		return false;
> -	}
> +	if (!xelpdp_tc_phy_wait_for_tcss_power(tc, enable))
> +		goto out_disable;
>  
>  	return true;
> +
> +out_disable:
> +	if (drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY))
> +		return false;
> +
> +	if (!enable)
> +		return false;
> +
> +	__xelpdp_tc_phy_enable_tcss_power(tc, false);
> +	xelpdp_tc_phy_wait_for_tcss_power(tc, false);
> +
> +	return false;
>  }
>  
>  static void xelpdp_tc_phy_take_ownership(struct intel_tc_port *tc, bool take)
> -- 
> 2.34.1
>
Kahola, Mika Dec. 13, 2023, 7:20 a.m. UTC | #2
> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Wednesday, December 13, 2023 9:14 AM
> To: Kahola, Mika <mika.kahola@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/i915/display: Wait for PHY readiness not needed for disabling sequence
> 
> On Tue, Dec 12, 2023 at 01:51:30PM +0200, Mika Kahola wrote:
> > When going through the disconnection flow we don't need to wait for
> > PHY readiness and hence we can skip the wait part. For disabling the
> > function returns false as an indicator that the power is not enabled.
> > After all, we are not even using the return value when Type-C is disconnecting.
> >
> > v2: Cleanup for increased readibility (Imre)
> >
> > BSpec: 65380
> >
> > For VLK-53734
> >
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> 
> For next time: it would've been better to separate to refactor + fix patches. The change looks ok:

That's true that this really contains two parts, the refactoring and a small fix.

Thanks for the review!

-Mika-

> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_tc.c | 25
> > ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index f64d348a969e..dcf05e00e505 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -1030,18 +1030,25 @@ static bool
> > xelpdp_tc_phy_enable_tcss_power(struct intel_tc_port *tc, bool enabl
> >
> >  	__xelpdp_tc_phy_enable_tcss_power(tc, enable);
> >
> > -	if ((!tc_phy_wait_for_ready(tc) ||
> > -	     !xelpdp_tc_phy_wait_for_tcss_power(tc, enable)) &&
> > -	    !drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY)) {
> > -		if (enable) {
> > -			__xelpdp_tc_phy_enable_tcss_power(tc, false);
> > -			xelpdp_tc_phy_wait_for_tcss_power(tc, false);
> > -		}
> > +	if (enable && !tc_phy_wait_for_ready(tc))
> > +		goto out_disable;
> >
> > -		return false;
> > -	}
> > +	if (!xelpdp_tc_phy_wait_for_tcss_power(tc, enable))
> > +		goto out_disable;
> >
> >  	return true;
> > +
> > +out_disable:
> > +	if (drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY))
> > +		return false;
> > +
> > +	if (!enable)
> > +		return false;
> > +
> > +	__xelpdp_tc_phy_enable_tcss_power(tc, false);
> > +	xelpdp_tc_phy_wait_for_tcss_power(tc, false);
> > +
> > +	return false;
> >  }
> >
> >  static void xelpdp_tc_phy_take_ownership(struct intel_tc_port *tc,
> > bool take)
> > --
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index f64d348a969e..dcf05e00e505 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -1030,18 +1030,25 @@  static bool xelpdp_tc_phy_enable_tcss_power(struct intel_tc_port *tc, bool enabl
 
 	__xelpdp_tc_phy_enable_tcss_power(tc, enable);
 
-	if ((!tc_phy_wait_for_ready(tc) ||
-	     !xelpdp_tc_phy_wait_for_tcss_power(tc, enable)) &&
-	    !drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY)) {
-		if (enable) {
-			__xelpdp_tc_phy_enable_tcss_power(tc, false);
-			xelpdp_tc_phy_wait_for_tcss_power(tc, false);
-		}
+	if (enable && !tc_phy_wait_for_ready(tc))
+		goto out_disable;
 
-		return false;
-	}
+	if (!xelpdp_tc_phy_wait_for_tcss_power(tc, enable))
+		goto out_disable;
 
 	return true;
+
+out_disable:
+	if (drm_WARN_ON(&i915->drm, tc->mode == TC_PORT_LEGACY))
+		return false;
+
+	if (!enable)
+		return false;
+
+	__xelpdp_tc_phy_enable_tcss_power(tc, false);
+	xelpdp_tc_phy_wait_for_tcss_power(tc, false);
+
+	return false;
 }
 
 static void xelpdp_tc_phy_take_ownership(struct intel_tc_port *tc, bool take)