[Intel-gfx] [PATCH 3/3] drm/i915: add i915.dp_link_train_policy option

[Daniel Vetter]

On Thu, Apr 24, 2014 at 06:22:59PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> We still have way too many bugs with DP link training. We keep
> switching between "narrow and fast" and "wide and slow", we recently
> added 5GHz support, and whenever there's a bug report, we have to ask
> people to apply patches and test them.
> 
> Wouldn't it be so much better if we could just ask them to boot with
> some specific Kernel boot option instead of applying a patch? This
> will move the situation from "i915.ko is completely broken!" to
> "i915.ko's default values are broken, but there's an option I can set
> to fix it, so I won't need to learn how to compile a Kernel!".
> 
> Some useful values:
>  - i915.dp_link_train_policy=1 for "wide and slow"
>  - i915.dp_link_train_policy=0x120 for DP_LINK_BW_2_7 and 2 lanes,
>    which should be able to fit 1920x1080 at 60Hz and 24bpp
>  - i915.dp_link_train_policy=0x210 to force DP 5GHz testing on
>    not-so-huge modes
> 
> The default behavior is still the same.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Yeah, I like this. I'll sign up Todd to review this all.

> ---
>  drivers/gpu/drm/i915/i915_drv.h    |  1 +
>  drivers/gpu/drm/i915/i915_params.c |  8 ++++++++
>  drivers/gpu/drm/i915/intel_dp.c    | 39 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7d6acb4..d5ae8dc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1941,6 +1941,7 @@ struct i915_params {
>  	bool reset;
>  	bool disable_display;
>  	bool disable_vtd_wa;
> +	int dp_link_train_policy;

Small nit: bitfield-y stuff should be unsinged.

>  };
>  extern struct i915_params i915 __read_mostly;
>  
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index d05a2af..8c358e7 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -48,6 +48,7 @@ struct i915_params i915 __read_mostly = {
>  	.disable_display = 0,
>  	.enable_cmd_parser = 1,
>  	.disable_vtd_wa = 0,
> +	.dp_link_train_policy = 0,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -156,3 +157,10 @@ MODULE_PARM_DESC(disable_vtd_wa, "Disable all VT-d workarounds (default: false)"
>  module_param_named(enable_cmd_parser, i915.enable_cmd_parser, int, 0600);
>  MODULE_PARM_DESC(enable_cmd_parser,
>  		 "Enable command parsing (1=enabled [default], 0=disabled)");
> +
> +module_param_named(dp_link_train_policy, i915.dp_link_train_policy, int, 0600);
> +MODULE_PARM_DESC(dp_link_train_policy,
> +	"Choose strategy for DP link training "
> +	"(0=narrow and fast [default], 1=wide and slow. For other values, "
> +	"bits 11:8 are for the BW and bits 7:4 are for the nubmer of lanes, "
> +	"check intel_dp_compute_link_config() for more details.)");
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 884166b..25f7e1c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -800,11 +800,42 @@ static bool intel_dp_compute_link_config(int mode_clock, int bpp,
>  
>  	mode_rate = intel_dp_link_required(mode_clock, bpp);
>  
> -	for (lanes = min_lanes; lanes <= max_lanes; lanes <<= 1)
> +	switch (i915.dp_link_train_policy) {
> +	case 0: /* Narrow and fast. */
> +		for (lanes = min_lanes; lanes <= max_lanes; lanes <<= 1)
> +			for (bw_it = min_bw_index; bw_it <= max_bw_index;
> +			     bw_it++)
> +				if (link_config_is_possible(mode_rate, lanes,
> +							    bws[bw_it]))
> +					goto found;
> +	case 1: /* Wide and slow. */
>  		for (bw_it = min_bw_index; bw_it <= max_bw_index; bw_it++)
> -			if (link_config_is_possible(mode_rate, lanes,
> -						    bws[bw_it]))
> -				goto found;
> +			for (lanes = min_lanes; lanes <= max_lanes; lanes <<= 1)
> +				if (link_config_is_possible(mode_rate, lanes,
> +							    bws[bw_it]))
> +					goto found;
> +	default: /* Bits 11:8 contain the index of the bws array.
> +		  * Bits 7:4 contain the number of lanes.
> +		  * Example: i915.dp_link_train_policy=0x140 uses
> +		  * DP_LINK_BW_2_7 and 4 lanes. */
> +		bw_it = (i915.dp_link_train_policy >> 8) & 0xF;
> +		lanes = (i915.dp_link_train_policy >> 4) & 0xF;
> +
> +		if (bw_it < min_bw_index || bw_it > max_bw_index) {
> +			DRM_ERROR("Invalid BW index\n");
> +			return false;
> +		}
> +
> +		if (lanes < min_lanes || lanes > max_lanes ||
> +		    /* Not a power of two. */
> +		    !(lanes != 0 && (lanes & (lanes - 1)) == 0)) {
> +			DRM_ERROR("Invalid number of lanes.\n");
> +			return false;
> +		}
> +
> +		if (link_config_is_possible(mode_rate, lanes, bws[bw_it]))
> +			goto found;
> +	}
>  
>  	return false;
>  
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch