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@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > > 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:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
