Re: [PATCH 11/12] drm/dp: Read AUX read interval from DPCD

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

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

On Tue, Dec 15, 2015 at 11:38:10AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 01:56:03PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Store the AUX read interval from DPCD, so that it can be used to wait
> > for the durations given in the specification during link training.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c |  4 ++++
> >  include/drm/drm_dp_helper.h     | 17 +++++++++++++++++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 76ac68bc1042..da519acfeba7 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
> >  	link->max_lanes = 0;
> >  
> >  	drm_dp_link_caps_reset(&link->caps);
> > +	link->aux_rd_interval = 0;
> >  	link->edp = 0;
> >  
> >  	link->rate = 0;
> > @@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> >  			link->edp = edp_revs[value];
> >  	}
> >  
> > +	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
> > +	link->aux_rd_interval = drm_dp_aux_rd_interval(values);
> 
> Hm, just wondering a bit of the relationship between link and cap. Is link
> all about sink really, and not the source? At least in my experience it
> makes a lot of sense to strictly keep these two separate, since otherwise
> you'll have lots of fun aligning things in generic code. Anyway, just a
> thougth.

The idea is that the link is the intersection between sink and source
capabilities. Drivers are supposed to call drm_dp_link_probe() to obtain
the capabilities of the sink and then adjust the struct drm_dp_link
according to their limitations (e.g. decrease the maximum rate if they
don't support 5.4 GHz, reduce the number of lanes if they only support
two, ...).

Once that's done the drivers can call drm_dp_link_choose() to select the
"best" set of configuration parameters given the link capabilities.

Note that this is strictly deriven from reading the specification under
the assumption that this is how things work in real life. My, arguably
limited, experience with Tegra shows that this is true. But perhaps that
is overly naive.

But I'd like to better understand what other drivers require so that
these helpers can be improved and be useful by more than a single
driver. Currently every driver implements their own DP stack, which I
think is rather unfortunate because we end up with vastly different
behaviour depending on which driver is in use.

Of course if that's what's desired, I'm more than happy to move this
code into the Tegra driver. I might have to duplicate the code that's
shared with MSM, but it's really not a lot compared to what's coming
up.

> > +
> >  	link->rate = link->max_rate;
> >  	link->lanes = link->max_lanes;
> >  
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 825aaf4e8c71..20ae0e413b64 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
> >  }
> >  
> > +/**
> > + * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
> > + * @dpcd: receiver capacity buffer
> > + *
> > + * Reads the AUX read interval (in microseconds) from the DPCD. Note that the
> > + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds.
> > + *
> > + * Returns:
> > + * The read AUX interval in microseconds.
> > + */
> > +static inline unsigned int
> > +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> 
> We should use this one here in the 2 delay helpers for channel_eq and
> clock_recovery imo.

Agreed.

Thierry

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

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

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