Re: [PATCH 3/3] drm/i915/dp: limit DP link rate based on VBT on CNL+

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

On Thu, 01 Feb 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Feb 01, 2018 at 01:03:43PM +0200, Jani Nikula wrote:
>> We have the max DP link rate info available in VBT since BDB version
>> 216, included in child device config since commit c4fb60b9aba9
>> ("drm/i915/bios: add DP max link rate to VBT child device
>> struct"). Parse it and use it.
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>>  drivers/gpu/drm/i915/intel_bios.c     | 21 +++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_dp.c       |  9 ++++++++-
>>  drivers/gpu/drm/i915/intel_vbt_defs.h |  5 +++++
>>  4 files changed, 35 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c676269ed843..26b91c25b8a0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1282,6 +1282,7 @@ struct ddi_vbt_port_info {
>>  
>>  	uint8_t dp_boost_level;
>>  	uint8_t hdmi_boost_level;
>> +	int dp_max_link_rate;		/* 0 for not limited by VBT */
>>  };
>>  
>>  enum psr_lines_to_wait {
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index cf3f8f1ba6f7..4e74aa2f16bc 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1274,6 +1274,27 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>>  		DRM_DEBUG_KMS("VBT HDMI boost level for port %c: %d\n",
>>  			      port_name(port), info->hdmi_boost_level);
>>  	}
>> +
>> +	/* DP max link rate for CNL+ */
>> +	if (bdb_version >= 216) {
>> +		switch (child->dp_max_link_rate) {
>> +		default:
>> +		case VBT_DP_MAX_LINK_RATE_HBR3:
>> +			info->dp_max_link_rate = 810000;
>> +			break;
>> +		case VBT_DP_MAX_LINK_RATE_HBR2:
>> +			info->dp_max_link_rate = 540000;
>> +			break;
>> +		case VBT_DP_MAX_LINK_RATE_HBR:
>> +			info->dp_max_link_rate = 270000;
>> +			break;
>> +		case VBT_DP_MAX_LINK_RATE_LBR:
>> +			info->dp_max_link_rate = 162000;
>> +			break;
>> +		}
>> +		DRM_DEBUG_KMS("VBT DP max link rate for port %c: %d\n",
>> +			      port_name(port), info->dp_max_link_rate);
>> +	}
>>  }
>>  
>>  static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 8bef858919c8..9a610d4783d8 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -270,8 +270,10 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>>  {
>>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> +	const struct ddi_vbt_port_info *info =
>> +		&dev_priv->vbt.ddi_port_info[dig_port->base.port];
>>  	const int *source_rates;
>> -	int size, max_rate = 0;
>> +	int size, max_rate = 0, vbt_max_rate = info->dp_max_link_rate;
>>  
>>  	/* This should only be done once */
>>  	WARN_ON(intel_dp->source_rates || intel_dp->num_source_rates);
>> @@ -295,6 +297,11 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>>  		size = ARRAY_SIZE(default_rates) - 1;
>>  	}
>>  
>> +	if (max_rate && vbt_max_rate)
>> +		max_rate = min(max_rate, vbt_max_rate);
>> +	else if (vbt_max_rate)
>> +		max_rate = vbt_max_rate;
>
> I kinda wish this would look exactly like the hdmi version, but
> can't do that without populating max_rate for every platform. So
> I guess we'll go with this.

I thought about that, but it's not as useful here as in hdmi because of
the varying number of intermediate rates. So you'll need several arrays
anyway. Some of it could be done, and later on I presume default_rates
will need to get updated for HBR3, so there's room for follow-up.

Pushed this series for now, thanks for the reviews.

BR,
Jani.


>
> Series looks good to me:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>> +
>>  	if (max_rate)
>>  		size = intel_dp_rate_limit_len(source_rates, size, max_rate);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> index 3d3feee9b5dd..458468237b5f 100644
>> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
>> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> @@ -320,6 +320,11 @@ enum vbt_gmbus_ddi {
>>  	DDC_BUS_DDI_F,
>>  };
>>  
>> +#define VBT_DP_MAX_LINK_RATE_HBR3	0
>> +#define VBT_DP_MAX_LINK_RATE_HBR2	1
>> +#define VBT_DP_MAX_LINK_RATE_HBR	2
>> +#define VBT_DP_MAX_LINK_RATE_LBR	3
>> +
>>  /*
>>   * The child device config, aka the display device data structure, provides a
>>   * description of a port and its configuration on the platform.
>> -- 
>> 2.11.0

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