[PATCH 12/12] drm/dp: Add drm_dp_link_choose() helper

[Jani Nikula]

On Mon, 29 Feb 2016, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Sun, Jan 31, 2016 at 04:39:51PM +0200, Jani Nikula wrote:
>> On Mon, 14 Dec 2015, Thierry Reding <thierry.reding at gmail.com> wrote:
>> > From: Thierry Reding <treding at nvidia.com>
>> >
>> > This helper chooses an appropriate configuration, according to the
>> > bitrate requirements of the video mode and the capabilities of the
>> > DisplayPort sink.
>> >
>> > Signed-off-by: Thierry Reding <treding at nvidia.com>
>> > ---
>> >  drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
>> >  include/drm/drm_dp_helper.h     |  5 ++++
>> >  2 files changed, 60 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> > index da519acfeba7..95825155dc89 100644
>> > --- a/drivers/gpu/drm/drm_dp_helper.c
>> > +++ b/drivers/gpu/drm/drm_dp_helper.c
>> > @@ -512,6 +512,61 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
>> >  }
>> >  EXPORT_SYMBOL(drm_dp_link_configure);
>> >  
>> > +/**
>> > + * drm_dp_link_choose() - choose the lowest possible configuration for a mode
>> > + * @link: DRM DP link object
>> > + * @mode: DRM display mode
>> > + * @info: DRM display information
>> > + *
>> > + * According to the eDP specification, a source should select a configuration
>> > + * with the lowest number of lanes and the lowest possible link rate that can
>> > + * match the bitrate requirements of a video mode. However it must ensure not
>> > + * to exceed the capabilities of the sink.
>> 
>> Eventually this would have to take into account the intersection of
>> per-sink and per-source supported rates, including the intermediate
>> frequencies. Until then, i915 couldn't switch over.
>
> I'm not sure I understand what you're saying. The idea is that drivers
> will call the drm_dp_link_probe() helper to probe the sink for supported
> frequencies, number of lanes and capabilities.
>
> After that, drivers are supposed to adjust the maximum values to account
> for their limitations. Hence the name for struct drm_dp_*link*, because
> it contains the negotiated parameters for the link between source and
> sink.
>
> That seems to me like the logical procedure when following the spec. Is
> that not how i915 works?

eDP v1.4 introduces more link rates. DPCD 10h through 1fh may contain a
table of the supported link rates, AFAICT the spec is written in a way
to allow arbitrary rates, but at least 1.62, 2.16, 2.43, 2.7, 3.24,
4.32, and 5.4 Gbps are cited and more details are provided for
those. The support is not limited by the maximum only, so an
intersection of the rates supported by source and sink will be required.

Obviously _link_probe() needs to be updated to figure out the supported
rates, not just the max, and _link_choose() can't assume a fixed set of
rates either.

i915 supports this. I'm not saying that what you're doing must support
all of this from the start, but I think you should get the eDP v1.4 spec
to make sure adding the support later on will be as straightforward as
possible.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center