[PATCHv2 0/3] Some correction in the DP Link Training sequence

[PATCHv2 0/3] Some correction in the DP Link Training sequence

[Arun R Murthy]

Arun R Murthy (3):
  drm/i915/dp: use fsleep instead of usleep_range for LT
  drm/i915/dp: read Aux RD interval just before reading the FFE preset
  drm/i915/dp: Include the time taken by AUX Tx for timeout

 .../drm/i915/display/intel_dp_link_training.c | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCHv2 1/3] drm/i915/dp: use fsleep instead of usleep_range for LT

[Arun R Murthy]

Sleeping for < 10us use udelay, for 10us to 20ms use usleep_range() and
for > 10ms use msleep. flseep() will call the particular API based on
the above condition. (Documentation/timers/timers-howto.rst)
Aux RD Interval value depends on the value read from the dpcd register
which is updated from the sink device, hence use flseep.

Co-developed-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 397cc4ebae52..f41b69840ad9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -898,7 +898,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 
 	voltage_tries = 1;
 	for (cr_tries = 0; cr_tries < max_cr_tries; ++cr_tries) {
-		usleep_range(delay_us, 2 * delay_us);
+		fsleep(delay_us);
 
 		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
 						     link_status) < 0) {
@@ -1040,7 +1040,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 	}
 
 	for (tries = 0; tries < 5; tries++) {
-		usleep_range(delay_us, 2 * delay_us);
+		fsleep(delay_us);
 
 		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
 						     link_status) < 0) {
@@ -1417,7 +1417,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 	deadline = jiffies + msecs_to_jiffies_timeout(400);
 
 	for (try = 0; try < max_tries; try++) {
-		usleep_range(delay_us, 2 * delay_us);
+		fsleep(delay_us);
 
 		/*
 		 * The delay may get updated. The transmitter shall read the
-- 
2.25.1

[PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading the FFE preset

[Arun R Murthy]

Figure 3-52: 128b132b DP DPTC LANEx_CHANNEL_EQ_DONE Sequence of
DP2.1a spec.
During LT, the transmitter shall read DPCD 02216h before DPCD
00202h through 00207h, and 0200Ch through 0200Fh

Co-developed-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 .../gpu/drm/i915/display/intel_dp_link_training.c    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index f41b69840ad9..460426a3b506 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1419,12 +1419,6 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 	for (try = 0; try < max_tries; try++) {
 		fsleep(delay_us);
 
-		/*
-		 * The delay may get updated. The transmitter shall read the
-		 * delay before link status during link training.
-		 */
-		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
-
 		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
 			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
 			return false;
@@ -1451,6 +1445,12 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 		if (time_after(jiffies, deadline))
 			timeout = true; /* try one last time after deadline */
 
+		/*
+		 * During LT, Tx shall read DPCD 02216h before DPCD 00202h to 00207h and
+		 * 0200Ch through 0200Fh.
+		 */
+		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
+
 		/* Update signal levels and training set as requested. */
 		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
 		if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
-- 
2.25.1

RE: [PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading the FFE preset

[Kandpal, Suraj]

> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Arun R
> Murthy
> Sent: Tuesday, September 24, 2024 11:44 AM
> To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>; Srikanth V, NagaVenkata
> <nagavenkata.srikanth.v@intel.com>
> Subject: [PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading
> the FFE preset
> 
> Figure 3-52: 128b132b DP DPTC LANEx_CHANNEL_EQ_DONE Sequence of
> DP2.1a spec.
> During LT, the transmitter shall read DPCD 02216h before DPCD 00202h
> through 00207h, and 0200Ch through 0200Fh
> 
> Co-developed-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  .../gpu/drm/i915/display/intel_dp_link_training.c    | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index f41b69840ad9..460426a3b506 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1419,12 +1419,6 @@ intel_dp_128b132b_lane_eq(struct intel_dp
> *intel_dp,
>  	for (try = 0; try < max_tries; try++) {
>  		fsleep(delay_us);
> 
> -		/*
> -		 * The delay may get updated. The transmitter shall read the
> -		 * delay before link status during link training.
> -		 */
> -		delay_us =
> drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> -
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status)
> < 0) {
>  			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link
> status\n");
>  			return false;
> @@ -1451,6 +1445,12 @@ intel_dp_128b132b_lane_eq(struct intel_dp
> *intel_dp,
>  		if (time_after(jiffies, deadline))
>  			timeout = true; /* try one last time after deadline */
> 
> +		/*
> +		 * During LT, Tx shall read DPCD 02216h before DPCD 00202h
> to 00207h and
> +		 * 0200Ch through 0200Fh.
> +		 */
> +		delay_us =
> drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> +
>  		/* Update signal levels and training set as requested. */
>  		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX,
> link_status);
>  		if (!intel_dp_update_link_train(intel_dp, crtc_state,
> DP_PHY_DPRX)) {
> --
> 2.25.1

Re: [PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading the FFE preset

[Jani Nikula]

On Tue, 24 Sep 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> Figure 3-52: 128b132b DP DPTC LANEx_CHANNEL_EQ_DONE Sequence of
> DP2.1a spec.
> During LT, the transmitter shall read DPCD 02216h before DPCD
> 00202h through 00207h, and 0200Ch through 0200Fh
>
> Co-developed-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_dp_link_training.c    | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index f41b69840ad9..460426a3b506 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1419,12 +1419,6 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  	for (try = 0; try < max_tries; try++) {
>  		fsleep(delay_us);
>  
> -		/*
> -		 * The delay may get updated. The transmitter shall read the
> -		 * delay before link status during link training.
> -		 */
> -		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> -
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>  			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>  			return false;
> @@ -1451,6 +1445,12 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  		if (time_after(jiffies, deadline))
>  			timeout = true; /* try one last time after deadline */
>  
> +		/*
> +		 * During LT, Tx shall read DPCD 02216h before DPCD 00202h to 00207h and
> +		 * 0200Ch through 0200Fh.
> +		 */

I really like comments that are actual helpful sentences. Why do I I
need to look up what 02216h and 00202h-00207h are? What's wrong with the
original comment?

BR,
Jani.


> +		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> +
>  		/* Update signal levels and training set as requested. */
>  		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
>  		if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {

-- 
Jani Nikula, Intel

RE: [PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading the FFE preset

[Murthy, Arun R]

> > +		/*
> > +		 * During LT, Tx shall read DPCD 02216h before DPCD 00202h
> to 00207h and
> > +		 * 0200Ch through 0200Fh.
> > +		 */
> 
> I really like comments that are actual helpful sentences. Why do I I need to look
> up what 02216h and 00202h-00207h are? What's wrong with the original
> comment?
> 
Original comment was wrong "read delay before reading link status"
In the updated comment will replace the registers with meaningful name.

Thanks and Regards,
Arun R Murthy
--------------------

RE: [PATCHv2 2/3] drm/i915/dp: read Aux RD interval just before reading the FFE preset

[Jani Nikula]

On Wed, 25 Sep 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> > +		/*
>> > +		 * During LT, Tx shall read DPCD 02216h before DPCD 00202h
>> to 00207h and
>> > +		 * 0200Ch through 0200Fh.
>> > +		 */
>> 
>> I really like comments that are actual helpful sentences. Why do I I need to look
>> up what 02216h and 00202h-00207h are? What's wrong with the original
>> comment?
>> 
> Original comment was wrong "read delay before reading link status"
> In the updated comment will replace the registers with meaningful name.

If you replace the registers with meaningful names, won't you end up
with "DPCD 02216h" -> "delay" and "DPCD 00202h to 00207h" -> "link
status"?

BR,
Jani.




>
> Thanks and Regards,
> Arun R Murthy
> --------------------

-- 
Jani Nikula, Intel

[PATCHv2 3/3] drm/i915/dp: Include the time taken by AUX Tx for timeout

[Arun R Murthy]

As per DP spec the timeout for LANE_CHANNEL_EQ_DONE is 400ms. But this
timeout value is exclusively for the Aux RD Interval and excludes the
time consumed for the AUX Tx (i.e reading/writing FFE presets). Add
another 50ms for these AUX Tx to the 400ms timeout.
Ref: "Figure 3-52: 128b132b DP DPTC LANEx_CHANNEL_EQ_DONE Sequence" of
DP2.1a spec.

Co-developed-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 460426a3b506..60bf375b9aec 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1414,7 +1414,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 	}
 
 	/* Time budget for the LANEx_EQ_DONE Sequence */
-	deadline = jiffies + msecs_to_jiffies_timeout(400);
+	deadline = jiffies + msecs_to_jiffies_timeout(450);
 
 	for (try = 0; try < max_tries; try++) {
 		fsleep(delay_us);
-- 
2.25.1