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

[Jani Nikula <jani.nikula@linux.intel.com>]

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],

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel