[Intel-gfx] [PATCH 2/5] drm/i915: use the HDMI DDI buffer translations from VBT

[Chris Wilson]

On Wed, Sep 11, 2013 at 06:02:48PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> We currently use the recommended values from BSpec, but the VBT
> specifies the correct value to use for the hardware we have, so use
> it. We also fall back to the recommended value in case we can't find
> the VBT.
> 
> In addition, this code also provides some infrastructure to parse more
> information about the DDI ports. There's a lot more information we
> could extract and use in the future.
> 
> v2: - Move some code to init_vbt_defaults.
> v3: - Rebase
>     - Clarify the "DVO Port" matching code
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  6 +++
>  drivers/gpu/drm/i915/intel_bios.c | 78 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h | 13 +++++++
>  drivers/gpu/drm/i915/intel_ddi.c  | 24 +++++++++++-
>  4 files changed, 119 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ca8856a..298c671 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1052,6 +1052,10 @@ enum modeset_restore {
>  	MODESET_SUSPENDED,
>  };
>  
> +struct ddi_vbt_port_info {
> +	uint8_t hdmi_level_shift;
> +};
> +
>  struct intel_vbt_data {
>  	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
>  	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
> @@ -1086,6 +1090,8 @@ struct intel_vbt_data {
>  
>  	int child_dev_num;
>  	union child_device_config *child_dev;
> +
> +	struct ddi_vbt_port_info ddi_port_info[5];

s/5/PORT_E+1/ or better #define NUM_DDI_PORTS PORT_E+1.

>  };
>  
>  enum intel_ddb_partitioning {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 33003b9..d7ff8fc 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -583,6 +583,72 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	dev_priv->vbt.dsi.panel_id = mipi->panel_id;
>  }
>  
> +static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> +			   struct bdb_header *bdb)
> +{
> +	union child_device_config *it, *child = NULL;
> +	struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
> +	uint8_t hdmi_level_shift;
> +	int i, j;
> +	/* Each DDI port can have more than one value on the "DVO Port" field,
> +	 * so look for all the possible values for each port and abort if more
> +	 * than one is found. */
> +	int dvo_ports[][2] = {
> +		{DVO_PORT_HDMIA, DVO_PORT_DPA},
> +		{DVO_PORT_HDMIB, DVO_PORT_DPB},
> +		{DVO_PORT_HDMIC, DVO_PORT_DPC},
> +		{DVO_PORT_HDMID, DVO_PORT_DPD},
> +		{DVO_PORT_CRT, -1 /* Port E can only be DVO_PORT_CRT */ },
> +	};
> +

I don't see the point of calling this function if bdb->version < 158 ?

> +	/* Find the child device to use, abort if more than one found. */
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		it = dev_priv->vbt.child_dev + i;
> +
> +		for (j = 0; j < 2; j++) {
> +			if (dvo_ports[port][j] == -1)
> +				break;
> +
> +			if (it->common.dvo_port == dvo_ports[port][j]) {
> +				if (child) {
> +					DRM_ERROR("More than one child device for port %c in VBT.\n",
> +						   port_name(port));
> +					return;
> +				}
> +				child = it;
> +			}
> +		}
> +	}
> +	if (!child)
> +		return;
> +
> +	if (bdb->version >= 158) {
> +		/* The VBT HDMI level shift values match the table we have. */
> +		hdmi_level_shift = child->raw[7] & 0xF;
> +		if (hdmi_level_shift < 0xC) {
> +			DRM_DEBUG_KMS("VBT HDMI level shift for port %c: %d\n",
> +				      port_name(port),
> +				      hdmi_level_shift);
> +			info->hdmi_level_shift = hdmi_level_shift;
> +		}
> +	}
> +}
> +
> +static void parse_ddi_ports(struct drm_i915_private *dev_priv,
> +			    struct bdb_header *bdb)
> +{
> +	enum port port;
> +
> +	if (!dev_priv->vbt.child_dev_num)
> +		return;
> +
> +	if (bdb->version < 155)
> +		return;
> +
> +	for (port = PORT_A; port <= PORT_E; port++)
> +		parse_ddi_port(dev_priv, port, bdb);
> +}
> +
>  static void
>  parse_device_mapping(struct drm_i915_private *dev_priv,
>  		       struct bdb_header *bdb)
> @@ -670,6 +736,16 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  	dev_priv->vbt.lvds_use_ssc = 1;
>  	dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
>  	DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->vbt.lvds_ssc_freq);
> +
> +	if (HAS_DDI(dev)) {

Might as well just set the defaults anyway. I only really care if the
recommendations change between generations.

> +		enum port port;
> +
> +		for (port = PORT_A; port <= PORT_E; port++) {
> +			/* Recommended BSpec default: 800mV 0dB. */
> +			dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
> +		}
> +	}
> +
>  }
>  
>  static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
> @@ -761,6 +837,8 @@ intel_parse_bios(struct drm_device *dev)
>  	parse_driver_features(dev_priv, bdb);
>  	parse_edp(dev_priv, bdb);
>  	parse_mipi(dev_priv, bdb);
> +	if (HAS_DDI(dev))
> +		parse_ddi_ports(dev_priv, bdb);

Move the check here into parse_ddi_ports() so the calling sequence is
clear.

-- 
Chris Wilson, Intel Open Source Technology Centre