[Intel-gfx] [RFC 09/12] drm/i915/config: Add VBT settings configuration.

Bob Paauwe bob.j.paauwe at intel.com
Tue Feb 24 10:37:10 PST 2015


On Tue, 24 Feb 2015 14:57:48 +0100
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Thu, Feb 12, 2015 at 03:41:35PM -0800, Bob Paauwe wrote:
> > Add a new section with subsections to the ACPI configuration table
> > that mimics much of the information typically stored in the VBT/option
> > ROM. This allows for a way to override incorrect VBT data or to provide
> > the configuration if VBT is not present.  Lack of VBT is common in
> > embedded systems.
> > 
> > Any data found in the configuration tables will replace the driver's
> > vbt structure.
> > 
> > MIPI DSI configuration is not implmemnted at this time.
> > 
> > Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
> 
> I'm not too happy with the duplicate of standards this creates. Is there
> no way to just load a vbt blob into the acpi table somewhere and require
> that it perfectly matches the vbt "standard" for the given platforms,
> warts and all included?

That's actually fairly easy to do. I believe that the opregion code is
just that. So the existing vbt code in the driver does that today.  The
acpi property code can't have an opregion embedded as a property, I did
look into that, but you can still create array properties and one of
those could hold a vbt blob. Another alternative would be to have a
property array for each vbt section. 

What I was trying for here was a way to override the driver defaults
for specific settings in the case where the platform did not have an
ACPI opregion or boot firmware with a vbt table.   I wasn't really
trying to create a mechanism to create an entire vbt table.  But the
parsing of the properties does get pretty messy.

> 
> As Jani points out we already have vbt headaches, it would be good if we
> only have those once ;-)
> -Daniel

I've been trying to think of a better way but not really coming up with
something that scales well. EMGD took the approach of create
configuration for only those values that we had customer requests for.
And that was primarily just the panel power up/down timing sequence and
some backlight control. We never tried to replicate everything that
could be set via vbt.

I've also been looking at splitting up the existing vbt structure in
some way so that both the exiting vbt code and the configuration code
could be used to set component configuration.  Something like 

  set_crt_ddc_pin() in the intel_crt code that gets called by either
  (or both) the vbt parsing code or the configuration framework.

But creating set_<something> functions for all the existing values in
the vbt structure seems like it might be too much overhead.

Bob

> 
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c     |   2 +
> >  drivers/gpu/drm/i915/i915_drv.h     |   1 +
> >  drivers/gpu/drm/i915/intel_config.c | 334 ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h    |   4 +-
> >  4 files changed, 340 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 9501360..9119e9b 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -852,6 +852,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >  	if (intel_vgpu_active(dev))
> >  		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
> >  
> > +	intel_config_vbt(dev_priv);
> > +
> >  	i915_setup_sysfs(dev);
> >  
> >  	if (INTEL_INFO(dev)->num_pipes) {
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1580702..a60511e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1657,6 +1657,7 @@ struct intel_config_info {
> >  	struct list_head connector_list;
> >  	struct list_head plane_list;
> >  	struct list_head wa_list;
> > +	struct list_head vbt_list;
> >  };
> >  
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_config.c b/drivers/gpu/drm/i915/intel_config.c
> > index ab14928..c1b06b7 100644
> > --- a/drivers/gpu/drm/i915/intel_config.c
> > +++ b/drivers/gpu/drm/i915/intel_config.c
> > @@ -215,6 +215,7 @@ void intel_config_init(struct drm_device *dev)
> >  	INIT_LIST_HEAD(&info->connector_list);
> >  	INIT_LIST_HEAD(&info->plane_list);
> >  	INIT_LIST_HEAD(&info->wa_list);
> > +	INIT_LIST_HEAD(&info->vbt_list);
> >  
> >  	handle = ACPI_HANDLE(dev->dev);
> >  	if (!handle) {
> > @@ -241,6 +242,7 @@ void intel_config_init(struct drm_device *dev)
> >  #define i915_COMPONENT_CONNECTOR "CNCT"
> >  #define i915_COMPONENT_PLANE "PLNS"
> >  #define i915_COMPONENT_WORKAROUND "WRKS"
> > +#define i915_COMPONENT_VBIOSTABLE "VBT"
> >  
> >  	/* Build lists */
> >  	list_for_each_entry(component, &info->base.adev->children, node) {
> > @@ -267,6 +269,18 @@ void intel_config_init(struct drm_device *dev)
> >  		} else if (strcmp(cname, i915_COMPONENT_WORKAROUND) == 0) {
> >  			if (!alloc_new_node(cl, &info->wa_list))
> >  				goto bail;
> > +		} else if (strcmp(cname, i915_COMPONENT_VBIOSTABLE) == 0) {
> > +			/* Add the main VBT node */
> > +			if (!alloc_new_node(cl, &info->vbt_list))
> > +				goto bail;
> > +
> > +			/* This adds all the sub device nodes */
> > +			list_for_each_entry(cl, &component->children, node) {
> > +				if (!alloc_new_node(cl, &info->vbt_list))
> > +					goto bail;
> > +			}
> > +		} else {
> > +			DRM_DEBUG_DRIVER("i915/config: Unknown component : %s\n", cname);
> >  		}
> >  	}
> >  
> > @@ -284,6 +298,8 @@ bail:
> >  		kfree(new_node);
> >  	list_for_each_entry_safe(new_node, tmp, &info->wa_list, node)
> >  		kfree(new_node);
> > +	list_for_each_entry_safe(new_node, tmp, &info->vbt_list, node)
> > +		kfree(new_node);
> >  
> >  	kfree(info);
> >  	dev_priv->config_info = NULL;
> > @@ -318,6 +334,8 @@ void intel_config_shutdown(struct drm_device *dev)
> >  		kfree(n);
> >  	list_for_each_entry_safe(n, tmp, &info->wa_list, node)
> >  		kfree(n);
> > +	list_for_each_entry_safe(n, tmp, &info->vbt_list, node)
> > +		kfree(n);
> >  
> >  	/* Unload any dynamically loaded ACPI property table */
> >  	handle = ACPI_HANDLE(dev->dev);
> > @@ -395,6 +413,8 @@ static struct list_head *cfg_type_to_list(struct intel_config_info *info,
> >  		return &info->plane_list;
> >  	case CFG_WORKAROUND:
> >  		return &info->wa_list;
> > +	case CFG_VBT:
> > +		return &info->vbt_list;
> >  	}
> >  	return NULL;
> >  }
> > @@ -433,6 +453,320 @@ bool intel_config_get_integer(struct drm_i915_private *dev_priv,
> >  	return false;
> >  }
> >  
> > +static void parse_port_info(struct drm_i915_private *dev_priv,
> > +			    const union acpi_object *pkg,
> > +			    int port)
> > +{
> > +	u32 supports;
> > +
> > +	if (pkg->type != ACPI_TYPE_PACKAGE)
> > +		return;
> > +
> > +	dev_priv->vbt.ddi_port_info[port].hdmi_level_shift =
> > +		(u32)pkg->package.elements[0].integer.value;
> > +
> > +	supports = (u32)pkg->package.elements[1].integer.value;
> > +	dev_priv->vbt.ddi_port_info[port].supports_dvi  = supports & 0x01;
> > +	dev_priv->vbt.ddi_port_info[port].supports_hdmi = supports & 0x02;
> > +	dev_priv->vbt.ddi_port_info[port].supports_dp   = supports & 0x04;
> > +}
> > +
> > +static struct drm_display_mode *parse_mode_info(const union acpi_object *pkg)
> > +{
> > +	struct drm_display_mode *mode;
> > +
> > +	if (pkg->type != ACPI_TYPE_PACKAGE)
> > +		return NULL;
> > +
> > +	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> > +	if (!mode || pkg->package.count != 10)
> > +		return NULL;
> > +
> > +	mode->hdisplay = (u32)pkg->package.elements[0].integer.value;
> > +	mode->hsync_start = (u32)pkg->package.elements[1].integer.value;
> > +	mode->hsync_end = (u32)pkg->package.elements[3].integer.value;
> > +	mode->htotal = (u32)pkg->package.elements[3].integer.value;
> > +	mode->vdisplay = (u32)pkg->package.elements[4].integer.value;
> > +	mode->vsync_start = (u32)pkg->package.elements[5].integer.value;
> > +	mode->vsync_end = (u32)pkg->package.elements[6].integer.value;
> > +	mode->vtotal = (u32)pkg->package.elements[7].integer.value;
> > +	mode->clock = (u32)pkg->package.elements[8].integer.value;
> > +	mode->flags = (u32)pkg->package.elements[9].integer.value;
> > +	mode->type = DRM_MODE_TYPE_PREFERRED;
> > +
> > +	return mode;
> > +}
> > +
> > +
> > +#define PROPERTY_MATCH(prop, name)\
> > +	(strcasecmp(prop->string.pointer, name) == 0)
> > +
> > +/*
> > + * Set any general vbt features defined in the ACPI table.
> > + */
> > +static void parse_vbt_general(struct drm_i915_private *dev_priv,
> > +			      const union acpi_object *properties,
> > +			      struct acpi_device *adev)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "Internal TV"))
> > +			dev_priv->vbt.int_tv_support =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "Internal CRT"))
> > +			dev_priv->vbt.int_crt_support =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "port info A"))
> > +			parse_port_info(dev_priv, value, PORT_A);
> > +		else if (PROPERTY_MATCH(name, "port info B"))
> > +			parse_port_info(dev_priv, value, PORT_B);
> > +		else if (PROPERTY_MATCH(name, "port info C"))
> > +			parse_port_info(dev_priv, value, PORT_C);
> > +		else if (PROPERTY_MATCH(name, "port info D"))
> > +			parse_port_info(dev_priv, value, PORT_D);
> > +		else if (PROPERTY_MATCH(name, "port info E"))
> > +			parse_port_info(dev_priv, value, PORT_E);
> > +		else if (PROPERTY_MATCH(name, "crt ddc pin"))
> > +			dev_priv->vbt.crt_ddc_pin = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "drrs type"))
> > +			dev_priv->vbt.drrs_type = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "fdi rx polarity inverted"))
> > +			dev_priv->vbt.fdi_rx_polarity_inverted =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "display clock mode"))
> > +			dev_priv->vbt.display_clock_mode =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "mode"))
> > +			dev_priv->vbt.lfp_lvds_vbt_mode =
> > +			    parse_mode_info(value);
> > +	}
> > +}
> > +
> > +static void parse_vbt_lvds(struct drm_i915_private *dev_priv,
> > +			   const union acpi_object *properties,
> > +			   struct acpi_device *adev)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "lvds use ssc"))
> > +			dev_priv->vbt.lvds_use_ssc = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "LVDS SSC Freq"))
> > +			dev_priv->vbt.lvds_ssc_freq =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "lvds downclock avail"))
> > +			dev_priv->lvds_downclock_avail =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "lvds downclock"))
> > +			dev_priv->lvds_downclock = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "Pixel Dither"))
> > +			dev_priv->vbt.lvds_dither = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "bios lvds val"))
> > +			dev_priv->vbt.bios_lvds_val =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "lvds vbt"))
> > +			dev_priv->vbt.lvds_vbt = (u32)value->integer.value;
> > +	}
> > +}
> > +
> > +static void parse_pwsq(struct drm_i915_private *dev_priv,
> > +		       const union acpi_object *properties)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "t1/t3"))
> > +			dev_priv->vbt.edp_pps.t1_t3 =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "t8"))
> > +			dev_priv->vbt.edp_pps.t8 = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "t9"))
> > +			dev_priv->vbt.edp_pps.t9 = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "t10"))
> > +			dev_priv->vbt.edp_pps.t10 = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "t11/t12"))
> > +			dev_priv->vbt.edp_pps.t11_t12 =
> > +			    (u32)value->integer.value;
> > +	}
> > +}
> > +
> > +static void parse_backlight(struct drm_i915_private *dev_priv,
> > +			    const union acpi_object *properties)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "present"))
> > +			dev_priv->vbt.backlight.present =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "pwm freq hz"))
> > +			dev_priv->vbt.backlight.pwm_freq_hz =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "active low pwm"))
> > +			dev_priv->vbt.backlight.active_low_pwm =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "min brightness"))
> > +			dev_priv->vbt.backlight.min_brightness =
> > +			    (u32)value->integer.value;
> > +	}
> > +}
> > +
> > +static void parse_psr(struct drm_i915_private *dev_priv,
> > +		      const union acpi_object *properties)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "full link"))
> > +			dev_priv->vbt.psr.full_link =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "require aux wakeup"))
> > +			dev_priv->vbt.psr.require_aux_wakeup =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "idle frames"))
> > +			dev_priv->vbt.psr.idle_frames =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "lines to wait"))
> > +			dev_priv->vbt.psr.lines_to_wait =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "tp1 wakeup time"))
> > +			dev_priv->vbt.psr.tp1_wakeup_time =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "tp2 tp3 wakeup time"))
> > +			dev_priv->vbt.psr.tp2_tp3_wakeup_time =
> > +			    (u32)value->integer.value;
> > +	}
> > +}
> > +
> > +static void parse_vbt_edp(struct drm_i915_private *dev_priv,
> > +			  const union acpi_object *properties,
> > +			  struct acpi_device *adev)
> > +{
> > +	const union acpi_object *prop, *name, *value;
> > +	int i;
> > +	struct acpi_device *component;
> > +
> > +	/* Child devices for Power sequencing, PSR and Backlight */
> > +	list_for_each_entry(component, &adev->children, node) {
> > +		char *sub_section_name;
> > +		const union acpi_object *properties;
> > +
> > +		if (!component)
> > +			continue;
> > +
> > +		sub_section_name = acpi_device_bid(component);
> > +		properties = component->data.properties;
> > +
> > +		if (!properties)
> > +			continue;
> > +
> > +		if (strcmp(sub_section_name, "PWSQ") == 0)
> > +			parse_pwsq(dev_priv, properties);
> > +		else if (strcmp(sub_section_name, "BLKT") == 0)
> > +			parse_backlight(dev_priv, properties);
> > +		else if (strcmp(sub_section_name, "PSR") == 0)
> > +			parse_psr(dev_priv, properties);
> > +	}
> > +
> > +	/* Process any EDP port configuration  */
> > +	for (i = 0; i < properties->package.count; i++) {
> > +		prop = &properties->package.elements[i];
> > +		name = &prop->package.elements[0];
> > +		value = &prop->package.elements[1];
> > +
> > +		if (PROPERTY_MATCH(name, "edp bpp"))
> > +			dev_priv->vbt.edp_bpp = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp pps")) {
> > +			/*
> > +			 * The power sequence timing values are stored in
> > +			 * an array
> > +			 */
> > +			dev_priv->vbt.edp_pps.t1_t3 =
> > +				(u32)value->package.elements[0].integer.value;
> > +			dev_priv->vbt.edp_pps.t8 =
> > +				(u32)value->package.elements[1].integer.value;
> > +			dev_priv->vbt.edp_pps.t9 =
> > +				(u32)value->package.elements[2].integer.value;
> > +			dev_priv->vbt.edp_pps.t10 =
> > +				(u32)value->package.elements[3].integer.value;
> > +			dev_priv->vbt.edp_pps.t11_t12 =
> > +				(u32)value->package.elements[4].integer.value;
> > +		} else if (PROPERTY_MATCH(name, "edp rate"))
> > +			dev_priv->vbt.edp_rate = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp lanes"))
> > +			dev_priv->vbt.edp_lanes = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp preemphasis"))
> > +			dev_priv->vbt.edp_preemphasis =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp vswing"))
> > +			dev_priv->vbt.edp_vswing = (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp initialized"))
> > +			dev_priv->vbt.edp_initialized =
> > +			    (u32)value->integer.value;
> > +		else if (PROPERTY_MATCH(name, "edp support"))
> > +			dev_priv->vbt.edp_support = (u32)value->integer.value;
> > +	}
> > +}
> > +
> > +/*
> > + * Port device configration based on VBT.
> > + *
> > + * Use the configuration information to overwrite any
> > + * corresponding VBT values or provide initial configuration
> > + * if VBT doens't exist.
> > + */
> > +void intel_config_vbt(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_config_info *info = dev_priv->config_info;
> > +	const union acpi_object *properties;
> > +	struct list_head *list;
> > +	struct intel_config_node *n;
> > +	char *sub_section_name;
> > +
> > +	list = cfg_type_to_list(info, CFG_VBT);
> > +	list_for_each_entry(n, list, node) {
> > +		sub_section_name = acpi_device_bid(n->adev);
> > +		properties = n->adev->data.properties;
> > +		if (!properties)
> > +			continue;
> > +
> > +		/* Call into sub section handlers */
> > +		if (strcmp(sub_section_name, "VBT") == 0)
> > +			parse_vbt_general(dev_priv, properties, n->adev);
> > +		else if (strcmp(sub_section_name, "LVDS") == 0)
> > +			parse_vbt_lvds(dev_priv, properties, n->adev);
> > +		else if (strcmp(sub_section_name, "EDP") == 0)
> > +			parse_vbt_edp(dev_priv, properties, n->adev);
> > +		else if (strcmp(sub_section_name, "DSI") == 0)
> > +			DRM_DEBUG_DRIVER("i915/config/vbt: Process Mipi DSI VBT features\n");
> > +	}
> > +}
> >  
> >  /*
> >   * Use ACPI methods to enumerate the items in the WA list.
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index e2b4e09..cc5da99 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1266,12 +1266,14 @@ enum cfg_type {
> >  	CFG_CRTC,
> >  	CFG_CONNECTOR,
> >  	CFG_PLANE,
> > -	CFG_WORKAROUND
> > +	CFG_WORKAROUND,
> > +	CFG_VBT
> >  };
> >  
> >  void intel_config_init(struct drm_device *dev);
> >  void intel_config_shutdown(struct drm_device *dev);
> >  bool intel_config_wa_add(struct drm_i915_private *dev_priv);
> > +void intel_config_vbt(struct drm_i915_private *dev_priv);
> >  bool intel_config_get_integer(struct drm_i915_private *dev_priv,
> >  			  enum cfg_type cfg_type,
> >  			  const char *name,
> > -- 
> > 2.1.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 



More information about the Intel-gfx mailing list