Re: [PATCH] drm/dp: Avoid needless delay while link training

From: Jani Nikula <jani.nikula@intel.com>
To: "Lee, Shawn C" <shawn.c.lee@intel.com>, intel-gfx@lists.freedesktop.org
Cc: Cooper Chiou <cooper.chiou@intel.com>
Subject: Re: [PATCH] drm/dp: Avoid needless delay while link training
Date: Tue, 26 Sep 2017 12:45:38 +0300	[thread overview]
Message-ID: <87o9px94jh.fsf@intel.com> (raw)
In-Reply-To: <1506419372-18734-1-git-send-email-shawn.c.lee@intel.com>

On Tue, 26 Sep 2017, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> DP v1.3 spec reserved DPCD TRAINING_AUX_RD_INTERVAL (0000Eh)
> bit7 to indicate Extended Receiver Capability. A DPRX with DPCD
> Rev. 1.4 (or higher) must have an Extended Receiver Capability field.
> Driver have to clear bit7 when retrieve interval value and avoid to
> wait for needless delay.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
>
> Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++------
>  include/drm/drm_dp_helper.h     |  1 +
>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 08af8d6b844b..5a01c84b7f8f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -118,19 +118,25 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
>  }
>  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
>  
> -void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> +void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> +{
> +	u8 interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_INTERVAL_MASK;
> +
> +	if (interval == 0)
>  		udelay(100);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay(interval * 4);

DP 1.4 seems to change this to 100 us for main-link clock recovery, but
let's leave that to another patch, another day...

>  }
>  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
>  
> -void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
> -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> +void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> +{
> +	u8 interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_INTERVAL_MASK;
> +
> +	if (interval == 0)
>  		udelay(400);
>  	else
> -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> +		mdelay(interval * 4);
>  }
>  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 2c412a15cfa1..12d4cf48096c 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -118,6 +118,7 @@
>  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2 or higher */
>  
>  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> +#define DP_TRAINING_AUX_RD_INTERVAL_MASK    (0x7f << 0)

Please indent this like all other content definitions in this file,
i.e. space between "#" and "define".

Please add

# define DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT (1 << 7) /* 1.4 */

while at it.

Other than that, LGTM.

BR,
Jani.

>  
>  #define DP_ADAPTER_CAP			    0x00f   /* 1.2 */
>  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx