* [PATCH 1/2] drm/i915: VBT's child_device_config changes over time
@ 2013-08-27 20:22 Paulo Zanoni
2013-08-27 20:22 ` [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT Paulo Zanoni
2013-09-03 14:33 ` [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Rodrigo Vivi
0 siblings, 2 replies; 7+ messages in thread
From: Paulo Zanoni @ 2013-08-27 20:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We currently treat the child_device_config as a simple struct, but
this is not correct: new BDB versions change the meaning of some
offsets, so the struct needs to be adjusted for each version.
Since there are too many changes (today we're in version 170!), making
a big versioned union would be too complicated, so child_device_config
is now a union of 3 things: (i) a "raw" byte array that's safe to use
anywhere; (ii) an "old" structure that's the one we've been using and
should be safe to keep in the SDVO and TV code; and (iii) a "common"
structure that should contain only fields that are common for all the
known VBT versions.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_bios.c | 36 ++++++++++++++++++------------------
drivers/gpu/drm/i915/intel_bios.h | 33 +++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_dp.c | 6 +++---
drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
drivers/gpu/drm/i915/intel_tv.c | 8 ++++----
6 files changed, 59 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 84b95b1..08fe1b6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1067,7 +1067,7 @@ struct intel_vbt_data {
int crt_ddc_pin;
int child_dev_num;
- struct child_device_config *child_dev;
+ union child_device_config *child_dev;
};
enum intel_ddb_partitioning {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 53f2bed..8a27925 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -389,7 +389,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
{
struct sdvo_device_mapping *p_mapping;
struct bdb_general_definitions *p_defs;
- struct child_device_config *p_child;
+ union child_device_config *p_child;
int i, child_device_num, count;
u16 block_size;
@@ -416,36 +416,36 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
count = 0;
for (i = 0; i < child_device_num; i++) {
p_child = &(p_defs->devices[i]);
- if (!p_child->device_type) {
+ if (!p_child->old.device_type) {
/* skip the device block if device type is invalid */
continue;
}
- if (p_child->slave_addr != SLAVE_ADDR1 &&
- p_child->slave_addr != SLAVE_ADDR2) {
+ if (p_child->old.slave_addr != SLAVE_ADDR1 &&
+ p_child->old.slave_addr != SLAVE_ADDR2) {
/*
* If the slave address is neither 0x70 nor 0x72,
* it is not a SDVO device. Skip it.
*/
continue;
}
- if (p_child->dvo_port != DEVICE_PORT_DVOB &&
- p_child->dvo_port != DEVICE_PORT_DVOC) {
+ if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
+ p_child->old.dvo_port != DEVICE_PORT_DVOC) {
/* skip the incorrect SDVO port */
DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
continue;
}
DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
" %s port\n",
- p_child->slave_addr,
- (p_child->dvo_port == DEVICE_PORT_DVOB) ?
+ p_child->old.slave_addr,
+ (p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
"SDVOB" : "SDVOC");
- p_mapping = &(dev_priv->sdvo_mappings[p_child->dvo_port - 1]);
+ p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 1]);
if (!p_mapping->initialized) {
- p_mapping->dvo_port = p_child->dvo_port;
- p_mapping->slave_addr = p_child->slave_addr;
- p_mapping->dvo_wiring = p_child->dvo_wiring;
- p_mapping->ddc_pin = p_child->ddc_pin;
- p_mapping->i2c_pin = p_child->i2c_pin;
+ p_mapping->dvo_port = p_child->old.dvo_port;
+ p_mapping->slave_addr = p_child->old.slave_addr;
+ p_mapping->dvo_wiring = p_child->old.dvo_wiring;
+ p_mapping->ddc_pin = p_child->old.ddc_pin;
+ p_mapping->i2c_pin = p_child->old.i2c_pin;
p_mapping->initialized = 1;
DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
p_mapping->dvo_port,
@@ -457,7 +457,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
"two SDVO device.\n");
}
- if (p_child->slave2_addr) {
+ if (p_child->old.slave2_addr) {
/* Maybe this is a SDVO device with multiple inputs */
/* And the mapping info is not added */
DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
@@ -573,7 +573,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
struct bdb_header *bdb)
{
struct bdb_general_definitions *p_defs;
- struct child_device_config *p_child, *child_dev_ptr;
+ union child_device_config *p_child, *child_dev_ptr;
int i, child_device_num, count;
u16 block_size;
@@ -601,7 +601,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
/* get the number of child device that is present */
for (i = 0; i < child_device_num; i++) {
p_child = &(p_defs->devices[i]);
- if (!p_child->device_type) {
+ if (!p_child->common.device_type) {
/* skip the device block if device type is invalid */
continue;
}
@@ -621,7 +621,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
count = 0;
for (i = 0; i < child_device_num; i++) {
p_child = &(p_defs->devices[i]);
- if (!p_child->device_type) {
+ if (!p_child->common.device_type) {
/* skip the device block if device type is invalid */
continue;
}
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index e088d6f..ae1b423 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -201,7 +201,10 @@ struct bdb_general_features {
#define DEVICE_PORT_DVOB 0x01
#define DEVICE_PORT_DVOC 0x02
-struct child_device_config {
+/* We used to keep this struct but without any version control. We should avoid
+ * using it in the future, but it should be safe to keep using it in the old
+ * code. */
+struct old_child_dev_config {
u16 handle;
u16 device_type;
u8 device_id[10]; /* ascii string */
@@ -223,6 +226,32 @@ struct child_device_config {
u8 dvo_function;
} __attribute__((packed));
+/* This one contains field offsets that are known to be common for all BDB
+ * versions. Notice that the meaning of the contents contents may still change,
+ * but at least the offsets are consistent. */
+struct common_child_dev_config {
+ u16 handle;
+ u16 device_type;
+ u8 not_common1[12];
+ u8 dvo_port;
+ u8 not_common2[2];
+ u8 ddc_pin;
+ u16 edid_ptr;
+} __attribute__((packed));
+
+/* This field changes depending on the BDB version, so the most reliable way to
+ * read it is by checking the BDB version and reading the raw pointer. */
+union child_device_config {
+ /* This one is safe to be used anywhere, but the code should still check
+ * the BDB version. */
+ u8 raw[33];
+ /* This one should only be kept for legacy code. */
+ struct old_child_dev_config old;
+ /* This one should also be safe to use anywhere, even without version
+ * checks. */
+ struct common_child_dev_config common;
+};
+
struct bdb_general_definitions {
/* DDC GPIO */
u8 crt_ddc_gmbus_pin;
@@ -248,7 +277,7 @@ struct bdb_general_definitions {
* number = (block_size - sizeof(bdb_general_definitions))/
* sizeof(child_device_config);
*/
- struct child_device_config devices[0];
+ union child_device_config devices[0];
} __attribute__((packed));
struct bdb_lvds_options {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..6224ea2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3090,7 +3090,7 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
bool intel_dpd_is_edp(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct child_device_config *p_child;
+ union child_device_config *p_child;
int i;
if (!dev_priv->vbt.child_dev_num)
@@ -3099,8 +3099,8 @@ bool intel_dpd_is_edp(struct drm_device *dev)
for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
p_child = dev_priv->vbt.child_dev + i;
- if (p_child->dvo_port == PORT_IDPD &&
- p_child->device_type == DEVICE_TYPE_eDP)
+ if (p_child->common.dvo_port == PORT_IDPD &&
+ p_child->common.device_type == DEVICE_TYPE_eDP)
return true;
}
return false;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 4d33278..dc99e38 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -786,7 +786,8 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
return true;
for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
- struct child_device_config *child = dev_priv->vbt.child_dev + i;
+ union child_device_config *uchild = dev_priv->vbt.child_dev + i;
+ struct old_child_dev_config *child = &uchild->old;
/* If the device type is not LFP, continue.
* We have to check both the new identifiers as well as the
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index f2c6d79..78631a2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1510,7 +1510,7 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
static int tv_is_present_in_vbt(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct child_device_config *p_child;
+ union child_device_config *p_child;
int i, ret;
if (!dev_priv->vbt.child_dev_num)
@@ -1522,13 +1522,13 @@ static int tv_is_present_in_vbt(struct drm_device *dev)
/*
* If the device type is not TV, continue.
*/
- if (p_child->device_type != DEVICE_TYPE_INT_TV &&
- p_child->device_type != DEVICE_TYPE_TV)
+ if (p_child->old.device_type != DEVICE_TYPE_INT_TV &&
+ p_child->old.device_type != DEVICE_TYPE_TV)
continue;
/* Only when the addin_offset is non-zero, it is regarded
* as present.
*/
- if (p_child->addin_offset) {
+ if (p_child->old.addin_offset) {
ret = 1;
break;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
2013-08-27 20:22 [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Paulo Zanoni
@ 2013-08-27 20:22 ` Paulo Zanoni
2013-08-28 15:39 ` Paulo Zanoni
2013-09-03 14:33 ` [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Rodrigo Vivi
1 sibling, 1 reply; 7+ messages in thread
From: Paulo Zanoni @ 2013-08-27 20:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@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.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++++
drivers/gpu/drm/i915/intel_bios.c | 65 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++--
3 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 08fe1b6..645dd74 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1039,6 +1039,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 */
@@ -1068,6 +1072,8 @@ struct intel_vbt_data {
int child_dev_num;
union child_device_config *child_dev;
+
+ struct ddi_vbt_port_info ddi_port_info[5];
};
enum intel_ddb_partitioning {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 8a27925..a5cb665 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -568,6 +568,69 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
}
}
+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;
+ int dvo_ports[][2] = {
+ {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
+ };
+ int n_dvo_ports[] = {2, 2, 2, 2, 1};
+
+ /* 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 < n_dvo_ports[port]; j++) {
+ 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;
+
+ /* Default values, in case something is wrong with the VBT. */
+ for (port = PORT_A; port <= PORT_E; port++) {
+ /* Recommended BSpec default: 800mV 0dB. */
+ dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
+ }
+
+ 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)
@@ -745,6 +808,8 @@ intel_parse_bios(struct drm_device *dev)
parse_device_mapping(dev_priv, bdb);
parse_driver_features(dev_priv, bdb);
parse_edp(dev_priv, bdb);
+ if (HAS_DDI(dev))
+ parse_ddi_ports(dev_priv, bdb);
if (bios)
pci_unmap_rom(pdev, bios);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 63aca49..ece226d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -42,7 +42,6 @@ static const u32 hsw_ddi_translations_dp[] = {
0x80C30FFF, 0x000B0000,
0x00FFFFFF, 0x00040006,
0x80D75FFF, 0x000B0000,
- 0x00FFFFFF, 0x00040006 /* HDMI parameters */
};
static const u32 hsw_ddi_translations_fdi[] = {
@@ -55,7 +54,22 @@ static const u32 hsw_ddi_translations_fdi[] = {
0x00C30FFF, 0x001E0000,
0x00FFFFFF, 0x00060006,
0x00D75FFF, 0x001E0000,
- 0x00FFFFFF, 0x00040006 /* HDMI parameters */
+};
+
+static const u32 hsw_ddi_translations_hdmi[] = {
+ /* Idx NT mV diff T mV diff db */
+ 0x00FFFFFF, 0x0006000E, /* 0: 400 400 0 */
+ 0x00E79FFF, 0x000E000C, /* 1: 400 500 2 */
+ 0x00D75FFF, 0x0005000A, /* 2: 400 600 3.5 */
+ 0x00FFFFFF, 0x0005000A, /* 3: 600 600 0 */
+ 0x00E79FFF, 0x001D0007, /* 4: 600 750 2 */
+ 0x00D75FFF, 0x000C0004, /* 5: 600 900 3.5 */
+ 0x00FFFFFF, 0x00040006, /* 6: 800 800 0 */
+ 0x80E79FFF, 0x00030002, /* 7: 800 1000 2 */
+ 0x00FFFFFF, 0x00140005, /* 8: 850 850 0 */
+ 0x00FFFFFF, 0x000C0004, /* 9: 900 900 0 */
+ 0x00FFFFFF, 0x001C0003, /* 10: 950 950 0 */
+ 0x80FFFFFF, 0x00030002, /* 11: 1000 1000 0 */
};
static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
@@ -92,12 +106,18 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
const u32 *ddi_translations = (port == PORT_E) ?
hsw_ddi_translations_fdi :
hsw_ddi_translations_dp;
+ int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
for (i = 0, reg = DDI_BUF_TRANS(port);
i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
I915_WRITE(reg, ddi_translations[i]);
reg += 4;
}
+ /* Entry 9 is for HDMI: */
+ for (i = 0; i < 2; i++) {
+ I915_WRITE(reg, hsw_ddi_translations_hdmi[hdmi_level * 2 + i]);
+ reg += 4;
+ }
}
/* Program DDI buffers translations for DP. By default, program ports A-D in DP
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
2013-08-27 20:22 ` [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT Paulo Zanoni
@ 2013-08-28 15:39 ` Paulo Zanoni
2013-09-03 14:37 ` Rodrigo Vivi
0 siblings, 1 reply; 7+ messages in thread
From: Paulo Zanoni @ 2013-08-28 15:39 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@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.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++++
drivers/gpu/drm/i915/intel_bios.c | 69 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_ddi.c | 24 ++++++++++++--
3 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 08fe1b6..645dd74 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1039,6 +1039,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 */
@@ -1068,6 +1072,8 @@ struct intel_vbt_data {
int child_dev_num;
union child_device_config *child_dev;
+
+ struct ddi_vbt_port_info ddi_port_info[5];
};
enum intel_ddb_partitioning {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 8a27925..79bb651 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
}
}
+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;
+ int dvo_ports[][2] = {
+ {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
+ };
+ int n_dvo_ports[] = {2, 2, 2, 2, 1};
+
+ /* 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 < n_dvo_ports[port]; j++) {
+ 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)
@@ -655,6 +712,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)) {
+ 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)
@@ -745,6 +812,8 @@ intel_parse_bios(struct drm_device *dev)
parse_device_mapping(dev_priv, bdb);
parse_driver_features(dev_priv, bdb);
parse_edp(dev_priv, bdb);
+ if (HAS_DDI(dev))
+ parse_ddi_ports(dev_priv, bdb);
if (bios)
pci_unmap_rom(pdev, bios);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 63aca49..ece226d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -42,7 +42,6 @@ static const u32 hsw_ddi_translations_dp[] = {
0x80C30FFF, 0x000B0000,
0x00FFFFFF, 0x00040006,
0x80D75FFF, 0x000B0000,
- 0x00FFFFFF, 0x00040006 /* HDMI parameters */
};
static const u32 hsw_ddi_translations_fdi[] = {
@@ -55,7 +54,22 @@ static const u32 hsw_ddi_translations_fdi[] = {
0x00C30FFF, 0x001E0000,
0x00FFFFFF, 0x00060006,
0x00D75FFF, 0x001E0000,
- 0x00FFFFFF, 0x00040006 /* HDMI parameters */
+};
+
+static const u32 hsw_ddi_translations_hdmi[] = {
+ /* Idx NT mV diff T mV diff db */
+ 0x00FFFFFF, 0x0006000E, /* 0: 400 400 0 */
+ 0x00E79FFF, 0x000E000C, /* 1: 400 500 2 */
+ 0x00D75FFF, 0x0005000A, /* 2: 400 600 3.5 */
+ 0x00FFFFFF, 0x0005000A, /* 3: 600 600 0 */
+ 0x00E79FFF, 0x001D0007, /* 4: 600 750 2 */
+ 0x00D75FFF, 0x000C0004, /* 5: 600 900 3.5 */
+ 0x00FFFFFF, 0x00040006, /* 6: 800 800 0 */
+ 0x80E79FFF, 0x00030002, /* 7: 800 1000 2 */
+ 0x00FFFFFF, 0x00140005, /* 8: 850 850 0 */
+ 0x00FFFFFF, 0x000C0004, /* 9: 900 900 0 */
+ 0x00FFFFFF, 0x001C0003, /* 10: 950 950 0 */
+ 0x80FFFFFF, 0x00030002, /* 11: 1000 1000 0 */
};
static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
@@ -92,12 +106,18 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
const u32 *ddi_translations = (port == PORT_E) ?
hsw_ddi_translations_fdi :
hsw_ddi_translations_dp;
+ int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
for (i = 0, reg = DDI_BUF_TRANS(port);
i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
I915_WRITE(reg, ddi_translations[i]);
reg += 4;
}
+ /* Entry 9 is for HDMI: */
+ for (i = 0; i < 2; i++) {
+ I915_WRITE(reg, hsw_ddi_translations_hdmi[hdmi_level * 2 + i]);
+ reg += 4;
+ }
}
/* Program DDI buffers translations for DP. By default, program ports A-D in DP
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/i915: VBT's child_device_config changes over time
2013-08-27 20:22 [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Paulo Zanoni
2013-08-27 20:22 ` [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT Paulo Zanoni
@ 2013-09-03 14:33 ` Rodrigo Vivi
1 sibling, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2013-09-03 14:33 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
As I told you in irc I'm not sure if the best option is to use common,
raw and old, but since I don't have a better idea and while nobody
else has, just to go ahead ;)
And I liked very much the raw part, although I had to count the bytes
amount on VBT doc when reviewing following patches ;)
For this patch:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Still have to continue reviewing the following ones
On Tue, Aug 27, 2013 at 5:22 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> We currently treat the child_device_config as a simple struct, but
> this is not correct: new BDB versions change the meaning of some
> offsets, so the struct needs to be adjusted for each version.
>
> Since there are too many changes (today we're in version 170!), making
> a big versioned union would be too complicated, so child_device_config
> is now a union of 3 things: (i) a "raw" byte array that's safe to use
> anywhere; (ii) an "old" structure that's the one we've been using and
> should be safe to keep in the SDVO and TV code; and (iii) a "common"
> structure that should contain only fields that are common for all the
> known VBT versions.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_bios.c | 36 ++++++++++++++++++------------------
> drivers/gpu/drm/i915/intel_bios.h | 33 +++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
> drivers/gpu/drm/i915/intel_tv.c | 8 ++++----
> 6 files changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 84b95b1..08fe1b6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1067,7 +1067,7 @@ struct intel_vbt_data {
> int crt_ddc_pin;
>
> int child_dev_num;
> - struct child_device_config *child_dev;
> + union child_device_config *child_dev;
> };
>
> enum intel_ddb_partitioning {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 53f2bed..8a27925 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -389,7 +389,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
> {
> struct sdvo_device_mapping *p_mapping;
> struct bdb_general_definitions *p_defs;
> - struct child_device_config *p_child;
> + union child_device_config *p_child;
> int i, child_device_num, count;
> u16 block_size;
>
> @@ -416,36 +416,36 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
> count = 0;
> for (i = 0; i < child_device_num; i++) {
> p_child = &(p_defs->devices[i]);
> - if (!p_child->device_type) {
> + if (!p_child->old.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> }
> - if (p_child->slave_addr != SLAVE_ADDR1 &&
> - p_child->slave_addr != SLAVE_ADDR2) {
> + if (p_child->old.slave_addr != SLAVE_ADDR1 &&
> + p_child->old.slave_addr != SLAVE_ADDR2) {
> /*
> * If the slave address is neither 0x70 nor 0x72,
> * it is not a SDVO device. Skip it.
> */
> continue;
> }
> - if (p_child->dvo_port != DEVICE_PORT_DVOB &&
> - p_child->dvo_port != DEVICE_PORT_DVOC) {
> + if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
> + p_child->old.dvo_port != DEVICE_PORT_DVOC) {
> /* skip the incorrect SDVO port */
> DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
> continue;
> }
> DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
> " %s port\n",
> - p_child->slave_addr,
> - (p_child->dvo_port == DEVICE_PORT_DVOB) ?
> + p_child->old.slave_addr,
> + (p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
> "SDVOB" : "SDVOC");
> - p_mapping = &(dev_priv->sdvo_mappings[p_child->dvo_port - 1]);
> + p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 1]);
> if (!p_mapping->initialized) {
> - p_mapping->dvo_port = p_child->dvo_port;
> - p_mapping->slave_addr = p_child->slave_addr;
> - p_mapping->dvo_wiring = p_child->dvo_wiring;
> - p_mapping->ddc_pin = p_child->ddc_pin;
> - p_mapping->i2c_pin = p_child->i2c_pin;
> + p_mapping->dvo_port = p_child->old.dvo_port;
> + p_mapping->slave_addr = p_child->old.slave_addr;
> + p_mapping->dvo_wiring = p_child->old.dvo_wiring;
> + p_mapping->ddc_pin = p_child->old.ddc_pin;
> + p_mapping->i2c_pin = p_child->old.i2c_pin;
> p_mapping->initialized = 1;
> DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
> p_mapping->dvo_port,
> @@ -457,7 +457,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
> DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
> "two SDVO device.\n");
> }
> - if (p_child->slave2_addr) {
> + if (p_child->old.slave2_addr) {
> /* Maybe this is a SDVO device with multiple inputs */
> /* And the mapping info is not added */
> DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
> @@ -573,7 +573,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> struct bdb_header *bdb)
> {
> struct bdb_general_definitions *p_defs;
> - struct child_device_config *p_child, *child_dev_ptr;
> + union child_device_config *p_child, *child_dev_ptr;
> int i, child_device_num, count;
> u16 block_size;
>
> @@ -601,7 +601,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> /* get the number of child device that is present */
> for (i = 0; i < child_device_num; i++) {
> p_child = &(p_defs->devices[i]);
> - if (!p_child->device_type) {
> + if (!p_child->common.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> }
> @@ -621,7 +621,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> count = 0;
> for (i = 0; i < child_device_num; i++) {
> p_child = &(p_defs->devices[i]);
> - if (!p_child->device_type) {
> + if (!p_child->common.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> }
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index e088d6f..ae1b423 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -201,7 +201,10 @@ struct bdb_general_features {
> #define DEVICE_PORT_DVOB 0x01
> #define DEVICE_PORT_DVOC 0x02
>
> -struct child_device_config {
> +/* We used to keep this struct but without any version control. We should avoid
> + * using it in the future, but it should be safe to keep using it in the old
> + * code. */
> +struct old_child_dev_config {
> u16 handle;
> u16 device_type;
> u8 device_id[10]; /* ascii string */
> @@ -223,6 +226,32 @@ struct child_device_config {
> u8 dvo_function;
> } __attribute__((packed));
>
> +/* This one contains field offsets that are known to be common for all BDB
> + * versions. Notice that the meaning of the contents contents may still change,
> + * but at least the offsets are consistent. */
> +struct common_child_dev_config {
> + u16 handle;
> + u16 device_type;
> + u8 not_common1[12];
> + u8 dvo_port;
> + u8 not_common2[2];
> + u8 ddc_pin;
> + u16 edid_ptr;
> +} __attribute__((packed));
> +
> +/* This field changes depending on the BDB version, so the most reliable way to
> + * read it is by checking the BDB version and reading the raw pointer. */
> +union child_device_config {
> + /* This one is safe to be used anywhere, but the code should still check
> + * the BDB version. */
> + u8 raw[33];
> + /* This one should only be kept for legacy code. */
> + struct old_child_dev_config old;
> + /* This one should also be safe to use anywhere, even without version
> + * checks. */
> + struct common_child_dev_config common;
> +};
> +
> struct bdb_general_definitions {
> /* DDC GPIO */
> u8 crt_ddc_gmbus_pin;
> @@ -248,7 +277,7 @@ struct bdb_general_definitions {
> * number = (block_size - sizeof(bdb_general_definitions))/
> * sizeof(child_device_config);
> */
> - struct child_device_config devices[0];
> + union child_device_config devices[0];
> } __attribute__((packed));
>
> struct bdb_lvds_options {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2151d13..6224ea2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3090,7 +3090,7 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
> bool intel_dpd_is_edp(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct child_device_config *p_child;
> + union child_device_config *p_child;
> int i;
>
> if (!dev_priv->vbt.child_dev_num)
> @@ -3099,8 +3099,8 @@ bool intel_dpd_is_edp(struct drm_device *dev)
> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> p_child = dev_priv->vbt.child_dev + i;
>
> - if (p_child->dvo_port == PORT_IDPD &&
> - p_child->device_type == DEVICE_TYPE_eDP)
> + if (p_child->common.dvo_port == PORT_IDPD &&
> + p_child->common.device_type == DEVICE_TYPE_eDP)
> return true;
> }
> return false;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 4d33278..dc99e38 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -786,7 +786,8 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
> return true;
>
> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> - struct child_device_config *child = dev_priv->vbt.child_dev + i;
> + union child_device_config *uchild = dev_priv->vbt.child_dev + i;
> + struct old_child_dev_config *child = &uchild->old;
>
> /* If the device type is not LFP, continue.
> * We have to check both the new identifiers as well as the
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index f2c6d79..78631a2 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1510,7 +1510,7 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
> static int tv_is_present_in_vbt(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct child_device_config *p_child;
> + union child_device_config *p_child;
> int i, ret;
>
> if (!dev_priv->vbt.child_dev_num)
> @@ -1522,13 +1522,13 @@ static int tv_is_present_in_vbt(struct drm_device *dev)
> /*
> * If the device type is not TV, continue.
> */
> - if (p_child->device_type != DEVICE_TYPE_INT_TV &&
> - p_child->device_type != DEVICE_TYPE_TV)
> + if (p_child->old.device_type != DEVICE_TYPE_INT_TV &&
> + p_child->old.device_type != DEVICE_TYPE_TV)
> continue;
> /* Only when the addin_offset is non-zero, it is regarded
> * as present.
> */
> - if (p_child->addin_offset) {
> + if (p_child->old.addin_offset) {
> ret = 1;
> break;
> }
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
2013-08-28 15:39 ` Paulo Zanoni
@ 2013-09-03 14:37 ` Rodrigo Vivi
2013-09-03 16:01 ` Daniel Vetter
2013-09-11 20:34 ` Paulo Zanoni
0 siblings, 2 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2013-09-03 14:37 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@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.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++++
> drivers/gpu/drm/i915/intel_bios.c | 69 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_ddi.c | 24 ++++++++++++--
> 3 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 08fe1b6..645dd74 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1039,6 +1039,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 */
> @@ -1068,6 +1072,8 @@ struct intel_vbt_data {
>
> int child_dev_num;
> union child_device_config *child_dev;
> +
> + struct ddi_vbt_port_info ddi_port_info[5];
> };
>
> enum intel_ddb_partitioning {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 8a27925..79bb651 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
> }
> }
>
> +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;
> + int dvo_ports[][2] = {
> + {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
> + };
> + int n_dvo_ports[] = {2, 2, 2, 2, 1};
I hadn't get this until you explain on irc, but I have no better idea
how to make it more clear, so just ignore me ;)
> +
> + /* 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 < n_dvo_ports[port]; j++) {
> + 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) {
0x8 is available since 157... is it useful to include it?
> + /* 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;
Some time in the past I had thought about a similar check, but the
version itself was added after 155...
So not sure how effective is this...
> +
> + 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)
> @@ -655,6 +712,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)) {
> + 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)
> @@ -745,6 +812,8 @@ intel_parse_bios(struct drm_device *dev)
> parse_device_mapping(dev_priv, bdb);
> parse_driver_features(dev_priv, bdb);
> parse_edp(dev_priv, bdb);
> + if (HAS_DDI(dev))
> + parse_ddi_ports(dev_priv, bdb);
>
> if (bios)
> pci_unmap_rom(pdev, bios);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 63aca49..ece226d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -42,7 +42,6 @@ static const u32 hsw_ddi_translations_dp[] = {
> 0x80C30FFF, 0x000B0000,
> 0x00FFFFFF, 0x00040006,
> 0x80D75FFF, 0x000B0000,
> - 0x00FFFFFF, 0x00040006 /* HDMI parameters */
> };
>
> static const u32 hsw_ddi_translations_fdi[] = {
> @@ -55,7 +54,22 @@ static const u32 hsw_ddi_translations_fdi[] = {
> 0x00C30FFF, 0x001E0000,
> 0x00FFFFFF, 0x00060006,
> 0x00D75FFF, 0x001E0000,
> - 0x00FFFFFF, 0x00040006 /* HDMI parameters */
> +};
> +
> +static const u32 hsw_ddi_translations_hdmi[] = {
> + /* Idx NT mV diff T mV diff db */
> + 0x00FFFFFF, 0x0006000E, /* 0: 400 400 0 */
> + 0x00E79FFF, 0x000E000C, /* 1: 400 500 2 */
> + 0x00D75FFF, 0x0005000A, /* 2: 400 600 3.5 */
> + 0x00FFFFFF, 0x0005000A, /* 3: 600 600 0 */
> + 0x00E79FFF, 0x001D0007, /* 4: 600 750 2 */
> + 0x00D75FFF, 0x000C0004, /* 5: 600 900 3.5 */
> + 0x00FFFFFF, 0x00040006, /* 6: 800 800 0 */
> + 0x80E79FFF, 0x00030002, /* 7: 800 1000 2 */
> + 0x00FFFFFF, 0x00140005, /* 8: 850 850 0 */
> + 0x00FFFFFF, 0x000C0004, /* 9: 900 900 0 */
> + 0x00FFFFFF, 0x001C0003, /* 10: 950 950 0 */
> + 0x80FFFFFF, 0x00030002, /* 11: 1000 1000 0 */
> };
>
> static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
> @@ -92,12 +106,18 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
> const u32 *ddi_translations = (port == PORT_E) ?
> hsw_ddi_translations_fdi :
> hsw_ddi_translations_dp;
> + int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>
> for (i = 0, reg = DDI_BUF_TRANS(port);
> i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
> I915_WRITE(reg, ddi_translations[i]);
> reg += 4;
> }
> + /* Entry 9 is for HDMI: */
> + for (i = 0; i < 2; i++) {
> + I915_WRITE(reg, hsw_ddi_translations_hdmi[hdmi_level * 2 + i]);
> + reg += 4;
> + }
> }
>
> /* Program DDI buffers translations for DP. By default, program ports A-D in DP
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
2013-09-03 14:37 ` Rodrigo Vivi
@ 2013-09-03 16:01 ` Daniel Vetter
2013-09-11 20:34 ` Paulo Zanoni
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2013-09-03 16:01 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Tue, Sep 03, 2013 at 11:37:27AM -0300, Rodrigo Vivi wrote:
> On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@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.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 6 ++++
> > drivers/gpu/drm/i915/intel_bios.c | 69 +++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_ddi.c | 24 ++++++++++++--
> > 3 files changed, 97 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 08fe1b6..645dd74 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1039,6 +1039,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 */
> > @@ -1068,6 +1072,8 @@ struct intel_vbt_data {
> >
> > int child_dev_num;
> > union child_device_config *child_dev;
> > +
> > + struct ddi_vbt_port_info ddi_port_info[5];
> > };
> >
> > enum intel_ddb_partitioning {
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 8a27925..79bb651 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
> > }
> > }
> >
> > +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;
> > + int dvo_ports[][2] = {
> > + {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
> > + };
> > + int n_dvo_ports[] = {2, 2, 2, 2, 1};
>
> I hadn't get this until you explain on irc, but I have no better idea
> how to make it more clear, so just ignore me ;)
If you use -1 for the unused slot (and check for that value in the loop)
you could ditch the n_dvo_ports array. Also I think the magic values in
dvo_ports need a comment or so ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT
2013-09-03 14:37 ` Rodrigo Vivi
2013-09-03 16:01 ` Daniel Vetter
@ 2013-09-11 20:34 ` Paulo Zanoni
1 sibling, 0 replies; 7+ messages in thread
From: Paulo Zanoni @ 2013-09-11 20:34 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
2013/9/3 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@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.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 6 ++++
>> drivers/gpu/drm/i915/intel_bios.c | 69 +++++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_ddi.c | 24 ++++++++++++--
>> 3 files changed, 97 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 08fe1b6..645dd74 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1039,6 +1039,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 */
>> @@ -1068,6 +1072,8 @@ struct intel_vbt_data {
>>
>> int child_dev_num;
>> union child_device_config *child_dev;
>> +
>> + struct ddi_vbt_port_info ddi_port_info[5];
>> };
>>
>> enum intel_ddb_partitioning {
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 8a27925..79bb651 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>> }
>> }
>>
>> +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;
>> + int dvo_ports[][2] = {
>> + {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
>> + };
>> + int n_dvo_ports[] = {2, 2, 2, 2, 1};
>
> I hadn't get this until you explain on irc, but I have no better idea
> how to make it more clear, so just ignore me ;)
I'll follow Daniel's suggestion and also add a comment. I agree this
is not beautiful code :(
>
>> +
>> + /* 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 < n_dvo_ports[port]; j++) {
>> + 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) {
>
> 0x8 is available since 157... is it useful to include it?
If the BDB version is 157 we'll use the BSpec-recommended value, which
is exactly 0x8.
>
>> + /* 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;
>
> Some time in the past I had thought about a similar check, but the
> version itself was added after 155...
> So not sure how effective is this...
IMHO if we ever get a list of which fields existed before version 155
we may want to revisit this function. For now I prefer to not parse
the older versions since we don't really know what to expect from
them.
Thanks for the reviews!
>
>> +
>> + 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)
>> @@ -655,6 +712,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)) {
>> + 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)
>> @@ -745,6 +812,8 @@ intel_parse_bios(struct drm_device *dev)
>> parse_device_mapping(dev_priv, bdb);
>> parse_driver_features(dev_priv, bdb);
>> parse_edp(dev_priv, bdb);
>> + if (HAS_DDI(dev))
>> + parse_ddi_ports(dev_priv, bdb);
>>
>> if (bios)
>> pci_unmap_rom(pdev, bios);
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 63aca49..ece226d 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -42,7 +42,6 @@ static const u32 hsw_ddi_translations_dp[] = {
>> 0x80C30FFF, 0x000B0000,
>> 0x00FFFFFF, 0x00040006,
>> 0x80D75FFF, 0x000B0000,
>> - 0x00FFFFFF, 0x00040006 /* HDMI parameters */
>> };
>>
>> static const u32 hsw_ddi_translations_fdi[] = {
>> @@ -55,7 +54,22 @@ static const u32 hsw_ddi_translations_fdi[] = {
>> 0x00C30FFF, 0x001E0000,
>> 0x00FFFFFF, 0x00060006,
>> 0x00D75FFF, 0x001E0000,
>> - 0x00FFFFFF, 0x00040006 /* HDMI parameters */
>> +};
>> +
>> +static const u32 hsw_ddi_translations_hdmi[] = {
>> + /* Idx NT mV diff T mV diff db */
>> + 0x00FFFFFF, 0x0006000E, /* 0: 400 400 0 */
>> + 0x00E79FFF, 0x000E000C, /* 1: 400 500 2 */
>> + 0x00D75FFF, 0x0005000A, /* 2: 400 600 3.5 */
>> + 0x00FFFFFF, 0x0005000A, /* 3: 600 600 0 */
>> + 0x00E79FFF, 0x001D0007, /* 4: 600 750 2 */
>> + 0x00D75FFF, 0x000C0004, /* 5: 600 900 3.5 */
>> + 0x00FFFFFF, 0x00040006, /* 6: 800 800 0 */
>> + 0x80E79FFF, 0x00030002, /* 7: 800 1000 2 */
>> + 0x00FFFFFF, 0x00140005, /* 8: 850 850 0 */
>> + 0x00FFFFFF, 0x000C0004, /* 9: 900 900 0 */
>> + 0x00FFFFFF, 0x001C0003, /* 10: 950 950 0 */
>> + 0x80FFFFFF, 0x00030002, /* 11: 1000 1000 0 */
>> };
>>
>> static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
>> @@ -92,12 +106,18 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
>> const u32 *ddi_translations = (port == PORT_E) ?
>> hsw_ddi_translations_fdi :
>> hsw_ddi_translations_dp;
>> + int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>>
>> for (i = 0, reg = DDI_BUF_TRANS(port);
>> i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
>> I915_WRITE(reg, ddi_translations[i]);
>> reg += 4;
>> }
>> + /* Entry 9 is for HDMI: */
>> + for (i = 0; i < 2; i++) {
>> + I915_WRITE(reg, hsw_ddi_translations_hdmi[hdmi_level * 2 + i]);
>> + reg += 4;
>> + }
>> }
>>
>> /* Program DDI buffers translations for DP. By default, program ports A-D in DP
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-11 20:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-27 20:22 [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Paulo Zanoni
2013-08-27 20:22 ` [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT Paulo Zanoni
2013-08-28 15:39 ` Paulo Zanoni
2013-09-03 14:37 ` Rodrigo Vivi
2013-09-03 16:01 ` Daniel Vetter
2013-09-11 20:34 ` Paulo Zanoni
2013-09-03 14:33 ` [PATCH 1/2] drm/i915: VBT's child_device_config changes over time Rodrigo Vivi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.