Re: [PATCH 08/15] drm/dp: Use drm_dp_aux_rd_interval()

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3155 bytes --]

On Fri, Oct 18, 2019 at 05:33:12PM -0400, Lyude Paul wrote:
> This also seems like maybe it should just go into the previous patch?

I suppose they could both be merged, but I think it's better to keep
them separate. In fact, I'm having second thoughts about the new helper
because it doesn't really take into account all the special cases. For
example, the patch below will use the value returned from the helper
independent of context, whereas according to the specification the value
is different if used for clock recovery (100 us) or if it is used for
channel equalization (400 us).

Perhaps a better order would be for the "do not busy loop" patch to go
first and then introduce the new helper and finally use the new helper
(along with the signed -> unsigned change) in a third patch while taking
care of using the right values all the time.

I'll respin these patches and send out the fixes in a v2.

Thierry

> On Tue, 2019-10-15 at 16:35 +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Make use of the newly added drm_dp_aux_rd_interval() helper in existing
> > DP link training helpers.
> > 
> > v2: drop stale sentence from commit message (Philipp Zabel)
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 26 +++-----------------------
> >  1 file changed, 3 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index ad2671d2ee8f..4b66010771fa 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -122,17 +122,7 @@ EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> >  
> >  void drm_dp_link_train_clock_recovery_delay(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE])
> >  {
> > -	unsigned int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > -					DP_TRAINING_AUX_RD_MASK;
> > -
> > -	if (rd_interval > 4)
> > -		DRM_DEBUG_KMS("AUX interval %u, out of range (max 4)\n",
> > -			      rd_interval);
> > -
> > -	if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14)
> > -		rd_interval = 100;
> > -	else
> > -		rd_interval *= 4 * USEC_PER_MSEC;
> > +	unsigned int rd_interval = drm_dp_aux_rd_interval(dpcd);
> >  
> >  	usleep_range(rd_interval, rd_interval * 2);
> >  }
> > @@ -140,19 +130,9 @@ EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> >  
> >  void drm_dp_link_train_channel_eq_delay(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE])
> >  {
> > -	unsigned int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > -					DP_TRAINING_AUX_RD_MASK;
> > -
> > -	if (rd_interval > 4)
> > -		DRM_DEBUG_KMS("AUX interval %u, out of range (max 4)\n",
> > -			      rd_interval);
> > +	unsigned int min = drm_dp_aux_rd_interval(dpcd);
> >  
> > -	if (rd_interval == 0)
> > -		rd_interval = 400;
> > -	else
> > -		rd_interval *= 4 * USEC_PER_MSEC;
> > -
> > -	usleep_range(rd_interval, rd_interval * 2);
> > +	usleep_range(min, min * 2);
> >  }
> >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> >  
> -- 
> Cheers,
> 	Lyude Paul
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel