diff mbox

[17/17] drm/dp: Add drm_dp_link_choose() helper

Message ID 20180205193827.20374-18-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding Feb. 5, 2018, 7:38 p.m. UTC
From: Thierry Reding <treding@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@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  5 ++++
 2 files changed, 60 insertions(+)

Comments

Jani Nikula Feb. 7, 2018, 12:53 p.m. UTC | #1
On Mon, 05 Feb 2018, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@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@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 c8b18c0161d7..fb6ee3ebc37d 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -557,6 +557,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.

Just a couple of notes here:

Recent eDP allows more rates than just the ones mentioned. So you'll
actually have a number of source and sink rates, and you'll have to
intersect them to find the common rates. We have this in i915.

Although the spec says use the "smallest" link parameters possible,
we've found that many panels out in the wild only work at the maximum
sink parameters. Presumably the sink max rate and width correspond to
the native resolution, and not much testing happens using other
parameters. :(


BR,
Jani.


> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +int drm_dp_link_choose(struct drm_dp_link *link,
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_display_info *info)
> +{
> +	/* available link symbol clock rates */
> +	static const unsigned int rates[3] = { 162000, 270000, 540000 };
> +	/* available number of lanes */
> +	static const unsigned int lanes[3] = { 1, 2, 4 };
> +	unsigned long requirement, capacity;
> +	unsigned int rate = link->max_rate;
> +	unsigned int i, j;
> +
> +	/* bandwidth requirement */
> +	requirement = mode->clock * info->bpc * 3;
> +
> +	for (i = 0; i < ARRAY_SIZE(lanes) && lanes[i] <= link->max_lanes; i++) {
> +		for (j = 0; j < ARRAY_SIZE(rates) && rates[j] <= rate; j++) {
> +			/*
> +			 * Capacity for this combination of lanes and rate,
> +			 * factoring in the ANSI 8B/10B encoding.
> +			 *
> +			 * Link rates in the DRM DP helpers are really link
> +			 * symbol frequencies, so a tenth of the actual rate
> +			 * of the link.
> +			 */
> +			capacity = lanes[i] * (rates[j] * 10) * 8 / 10;
> +
> +			if (capacity >= requirement) {
> +				DRM_DEBUG_KMS("using %u lanes at %u kHz (%lu/%lu kbps)\n",
> +					      lanes[i], rates[j], requirement,
> +					      capacity);
> +				link->lanes = lanes[i];
> +				link->rate = rates[j];
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -ERANGE;
> +}
> +EXPORT_SYMBOL(drm_dp_link_choose);
> +
>  /**
>   * drm_dp_downstream_max_clock() - extract branch device max
>   *                                 pixel rate for legacy VGA
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 4c7badcde945..39d134f9a954 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -27,6 +27,8 @@
>  #include <linux/i2c.h>
>  #include <linux/delay.h>
>  
> +#include <drm/drm_crtc.h>
> +
>  /*
>   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
>   * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
> @@ -1194,6 +1196,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
> +int drm_dp_link_choose(struct drm_dp_link *link,
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_display_info *info);
>  int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  				const u8 port_cap[4]);
>  int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
Thierry Reding March 13, 2018, 12:36 p.m. UTC | #2
On Wed, Feb 07, 2018 at 02:53:19PM +0200, Jani Nikula wrote:
> On Mon, 05 Feb 2018, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@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@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 c8b18c0161d7..fb6ee3ebc37d 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -557,6 +557,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.
> 
> Just a couple of notes here:

Sorry, this got burried under too much email.

> Recent eDP allows more rates than just the ones mentioned. So you'll
> actually have a number of source and sink rates, and you'll have to
> intersect them to find the common rates. We have this in i915.

I'm aware of this and I have a local patch to implement this. However I
currently don't have an eDP setup where I can test it, so I didn't think
it right to submit the patch.

> Although the spec says use the "smallest" link parameters possible,
> we've found that many panels out in the wild only work at the maximum
> sink parameters. Presumably the sink max rate and width correspond to
> the native resolution, and not much testing happens using other
> parameters. :(

I suppose I could just drop this helper. Or perhaps add a note about the
potential pitfalls. It works fine for my particular use-case, so I could
move it into the Tegra driver.

How about the other patches? It's getting really late for v4.17, but I'd
like to still get these in if possible so I can reduce my local patch
count and merge DP support for Tegra186.

Thierry
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c8b18c0161d7..fb6ee3ebc37d 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -557,6 +557,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.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info)
+{
+	/* available link symbol clock rates */
+	static const unsigned int rates[3] = { 162000, 270000, 540000 };
+	/* available number of lanes */
+	static const unsigned int lanes[3] = { 1, 2, 4 };
+	unsigned long requirement, capacity;
+	unsigned int rate = link->max_rate;
+	unsigned int i, j;
+
+	/* bandwidth requirement */
+	requirement = mode->clock * info->bpc * 3;
+
+	for (i = 0; i < ARRAY_SIZE(lanes) && lanes[i] <= link->max_lanes; i++) {
+		for (j = 0; j < ARRAY_SIZE(rates) && rates[j] <= rate; j++) {
+			/*
+			 * Capacity for this combination of lanes and rate,
+			 * factoring in the ANSI 8B/10B encoding.
+			 *
+			 * Link rates in the DRM DP helpers are really link
+			 * symbol frequencies, so a tenth of the actual rate
+			 * of the link.
+			 */
+			capacity = lanes[i] * (rates[j] * 10) * 8 / 10;
+
+			if (capacity >= requirement) {
+				DRM_DEBUG_KMS("using %u lanes at %u kHz (%lu/%lu kbps)\n",
+					      lanes[i], rates[j], requirement,
+					      capacity);
+				link->lanes = lanes[i];
+				link->rate = rates[j];
+				return 0;
+			}
+		}
+	}
+
+	return -ERANGE;
+}
+EXPORT_SYMBOL(drm_dp_link_choose);
+
 /**
  * drm_dp_downstream_max_clock() - extract branch device max
  *                                 pixel rate for legacy VGA
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 4c7badcde945..39d134f9a954 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -27,6 +27,8 @@ 
 #include <linux/i2c.h>
 #include <linux/delay.h>
 
+#include <drm/drm_crtc.h>
+
 /*
  * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
  * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
@@ -1194,6 +1196,9 @@  int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info);
 int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 				const u8 port_cap[4]);
 int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],