[Intel-gfx] [PATCH] drm/i915: Check DVO reads for errors
Chris Wilson
chris at chris-wilson.co.uk
Thu Feb 26 09:10:17 PST 2015
Not all of the DVO functions were checking the return value from their
i2c routines when reading registers. This could lead to us feeding
garbage values back into the hardware, possible causing further
failures. In some cases the uninitialised stack values were being
written into the kernel log.
Quentin Casasnovas suggested the simple solution of just initialising
the output parameter to zero in all cases, but we may as well spend the
extra few moments to fix it correctly.
Reported-by: Quentin Casasnovas <quentin.casasnovas at oracle.com>
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Quentin Casasnovas <quentin.casasnovas at oracle.com>
---
drivers/gpu/drm/i915/dvo_ch7017.c | 23 ++++++++-----
drivers/gpu/drm/i915/dvo_ch7xxx.c | 36 ++++++++++----------
drivers/gpu/drm/i915/dvo_ivch.c | 64 +++++++++++++++---------------------
drivers/gpu/drm/i915/dvo_sil164.c | 49 +++++++++++++--------------
drivers/gpu/drm/i915/dvo_tfp410.c | 69 ++++++++++++++++++++++-----------------
5 files changed, 120 insertions(+), 121 deletions(-)
diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c b/drivers/gpu/drm/i915/dvo_ch7017.c
index 86b27d1d90c2..452f0144e6a2 100644
--- a/drivers/gpu/drm/i915/dvo_ch7017.c
+++ b/drivers/gpu/drm/i915/dvo_ch7017.c
@@ -335,7 +335,8 @@ static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable)
{
uint8_t val;
- ch7017_read(dvo, CH7017_LVDS_POWER_DOWN, &val);
+ if (!ch7017_read(dvo, CH7017_LVDS_POWER_DOWN, &val))
+ return;
/* Turn off TV/VGA, and never turn it on since we don't support it. */
ch7017_write(dvo, CH7017_POWER_MANAGEMENT,
@@ -363,7 +364,8 @@ static bool ch7017_get_hw_state(struct intel_dvo_device *dvo)
{
uint8_t val;
- ch7017_read(dvo, CH7017_LVDS_POWER_DOWN, &val);
+ if (!ch7017_read(dvo, CH7017_LVDS_POWER_DOWN, &val))
+ return false;
if (val & CH7017_LVDS_POWER_DOWN_EN)
return false;
@@ -371,16 +373,20 @@ static bool ch7017_get_hw_state(struct intel_dvo_device *dvo)
return true;
}
-static void ch7017_dump_regs(struct intel_dvo_device *dvo)
+static void ch7017_dump_reg(struct intel_dvo_device *dvo,
+ int reg, const char *name)
{
uint8_t val;
-#define DUMP(reg) \
-do { \
- ch7017_read(dvo, reg, &val); \
- DRM_DEBUG_KMS(#reg ": %02x\n", val); \
-} while (0)
+ if (ch7017_read(dvo, reg, &val))
+ DRM_DEBUG_KMS("%s: %02x\n", name, val);
+ else
+ DRM_DEBUG_KMS("%s: failed\n", name);
+}
+static void ch7017_dump_regs(struct intel_dvo_device *dvo)
+{
+#define DUMP(reg) ch7017_dump_reg(dvo, reg, #reg)
DUMP(CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT);
DUMP(CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT);
DUMP(CH7017_VERTICAL_ACTIVE_LINE_OUTPUT);
@@ -390,6 +396,7 @@ do { \
DUMP(CH7017_LVDS_CONTROL_2);
DUMP(CH7017_OUTPUTS_ENABLE);
DUMP(CH7017_LVDS_POWER_DOWN);
+#undef DUMP
}
static void ch7017_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c
index 80449f475960..fb23b16eaf07 100644
--- a/drivers/gpu/drm/i915/dvo_ch7xxx.c
+++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c
@@ -248,20 +248,20 @@ static enum drm_connector_status ch7xxx_detect(struct intel_dvo_device *dvo)
{
uint8_t cdet, orig_pm, pm;
- ch7xxx_readb(dvo, CH7xxx_PM, &orig_pm);
+ if (!ch7xxx_readb(dvo, CH7xxx_PM, &orig_pm))
+ return connector_status_disconnected;
pm = orig_pm;
pm &= ~CH7xxx_PM_FPD;
pm |= CH7xxx_PM_DVIL | CH7xxx_PM_DVIP;
+ cdet = 0;
ch7xxx_writeb(dvo, CH7xxx_PM, pm);
-
ch7xxx_readb(dvo, CH7xxx_CONNECTION_DETECT, &cdet);
-
ch7xxx_writeb(dvo, CH7xxx_PM, orig_pm);
-
if (cdet & CH7xxx_CDET_DVI)
return connector_status_connected;
+
return connector_status_disconnected;
}
@@ -300,16 +300,16 @@ static void ch7xxx_mode_set(struct intel_dvo_device *dvo,
ch7xxx_writeb(dvo, CH7xxx_TLPF, tlpf);
ch7xxx_writeb(dvo, CH7xxx_TCT, 0x00);
- ch7xxx_readb(dvo, CH7xxx_IDF, &idf);
+ if (ch7xxx_readb(dvo, CH7xxx_IDF, &idf)) {
+ idf &= ~(CH7xxx_IDF_HSP | CH7xxx_IDF_VSP);
+ if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+ idf |= CH7xxx_IDF_HSP;
- idf &= ~(CH7xxx_IDF_HSP | CH7xxx_IDF_VSP);
- if (mode->flags & DRM_MODE_FLAG_PHSYNC)
- idf |= CH7xxx_IDF_HSP;
+ if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+ idf |= CH7xxx_IDF_VSP;
- if (mode->flags & DRM_MODE_FLAG_PVSYNC)
- idf |= CH7xxx_IDF_VSP;
-
- ch7xxx_writeb(dvo, CH7xxx_IDF, idf);
+ ch7xxx_writeb(dvo, CH7xxx_IDF, idf);
+ }
}
/* set the CH7xxx power state */
@@ -323,14 +323,9 @@ static void ch7xxx_dpms(struct intel_dvo_device *dvo, bool enable)
static bool ch7xxx_get_hw_state(struct intel_dvo_device *dvo)
{
- u8 val;
-
+ u8 val = 0;
ch7xxx_readb(dvo, CH7xxx_PM, &val);
-
- if (val & (CH7xxx_PM_DVIL | CH7xxx_PM_DVIP))
- return true;
- else
- return false;
+ return val & (CH7xxx_PM_DVIL | CH7xxx_PM_DVIP);
}
static void ch7xxx_dump_regs(struct intel_dvo_device *dvo)
@@ -339,8 +334,11 @@ static void ch7xxx_dump_regs(struct intel_dvo_device *dvo)
for (i = 0; i < CH7xxx_NUM_REGS; i++) {
uint8_t val;
+
if ((i % 8) == 0)
DRM_DEBUG_KMS("\n %02X: ", i);
+
+ val = 0;
ch7xxx_readb(dvo, i, &val);
DRM_DEBUG_KMS("%02X ", val);
}
diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
index 0f2587ff347c..4908b3a00cac 100644
--- a/drivers/gpu/drm/i915/dvo_ivch.c
+++ b/drivers/gpu/drm/i915/dvo_ivch.c
@@ -151,8 +151,6 @@
struct ivch_priv {
bool quiet;
-
- uint16_t width, height;
};
@@ -263,9 +261,6 @@ static bool ivch_init(struct intel_dvo_device *dvo,
goto out;
}
- ivch_read(dvo, VR20, &priv->width);
- ivch_read(dvo, VR21, &priv->height);
-
return true;
out:
@@ -372,46 +367,41 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
ivch_dump_regs(dvo);
}
-static void ivch_dump_regs(struct intel_dvo_device *dvo)
+static void ivch_dump_reg(struct intel_dvo_device *dvo,
+ int reg, const char *name)
{
uint16_t val;
- ivch_read(dvo, VR00, &val);
- DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
- ivch_read(dvo, VR01, &val);
- DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
- ivch_read(dvo, VR30, &val);
- DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
- ivch_read(dvo, VR40, &val);
- DRM_DEBUG_KMS("VR40: 0x%04x\n", val);
+ if (ivch_read(dvo, reg, &val))
+ DRM_DEBUG_KMS("%s: 0x%04x\n", name, val);
+ else
+ DRM_DEBUG_KMS("%s: failed\n", name);
+}
+
+static void ivch_dump_regs(struct intel_dvo_device *dvo)
+{
+#define DUMP(R) ivch_dump_reg(dvo, R, #R)
+ DUMP(VR00);
+ DUMP(VR01);
+ DUMP(VR30);
+ DUMP(VR40);
/* GPIO registers */
- ivch_read(dvo, VR80, &val);
- DRM_DEBUG_KMS("VR80: 0x%04x\n", val);
- ivch_read(dvo, VR81, &val);
- DRM_DEBUG_KMS("VR81: 0x%04x\n", val);
- ivch_read(dvo, VR82, &val);
- DRM_DEBUG_KMS("VR82: 0x%04x\n", val);
- ivch_read(dvo, VR83, &val);
- DRM_DEBUG_KMS("VR83: 0x%04x\n", val);
- ivch_read(dvo, VR84, &val);
- DRM_DEBUG_KMS("VR84: 0x%04x\n", val);
- ivch_read(dvo, VR85, &val);
- DRM_DEBUG_KMS("VR85: 0x%04x\n", val);
- ivch_read(dvo, VR86, &val);
- DRM_DEBUG_KMS("VR86: 0x%04x\n", val);
- ivch_read(dvo, VR87, &val);
- DRM_DEBUG_KMS("VR87: 0x%04x\n", val);
- ivch_read(dvo, VR88, &val);
- DRM_DEBUG_KMS("VR88: 0x%04x\n", val);
+ DUMP(VR80);
+ DUMP(VR81);
+ DUMP(VR82);
+ DUMP(VR83);
+ DUMP(VR84);
+ DUMP(VR85);
+ DUMP(VR86);
+ DUMP(VR87);
+ DUMP(VR88);
/* Scratch register 0 - AIM Panel type */
- ivch_read(dvo, VR8E, &val);
- DRM_DEBUG_KMS("VR8E: 0x%04x\n", val);
-
+ DUMP(VR8E);
/* Scratch register 1 - Status register */
- ivch_read(dvo, VR8F, &val);
- DRM_DEBUG_KMS("VR8F: 0x%04x\n", val);
+ DUMP(VR8F);
+#undef DUMP
}
static void ivch_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_sil164.c b/drivers/gpu/drm/i915/dvo_sil164.c
index fa0114967076..23aac3dd5883 100644
--- a/drivers/gpu/drm/i915/dvo_sil164.c
+++ b/drivers/gpu/drm/i915/dvo_sil164.c
@@ -134,7 +134,7 @@ static bool sil164_init(struct intel_dvo_device *dvo,
{
/* this will detect the SIL164 chip on the specified i2c bus */
struct sil164_priv *sil;
- unsigned char ch;
+ uint8_t ch;
sil = kzalloc(sizeof(struct sil164_priv), GFP_KERNEL);
if (sil == NULL)
@@ -175,8 +175,8 @@ static enum drm_connector_status sil164_detect(struct intel_dvo_device *dvo)
{
uint8_t reg9;
+ reg9 = 0;
sil164_readb(dvo, SIL164_REG9, ®9);
-
if (reg9 & SIL164_9_HTPLG)
return connector_status_connected;
else
@@ -210,11 +210,9 @@ static void sil164_mode_set(struct intel_dvo_device *dvo,
/* set the SIL164 power state */
static void sil164_dpms(struct intel_dvo_device *dvo, bool enable)
{
- int ret;
- unsigned char ch;
+ uint8_t ch;
- ret = sil164_readb(dvo, SIL164_REG8, &ch);
- if (ret == false)
+ if (!sil164_readb(dvo, SIL164_REG8, &ch))
return;
if (enable)
@@ -223,38 +221,35 @@ static void sil164_dpms(struct intel_dvo_device *dvo, bool enable)
ch &= ~SIL164_8_PD;
sil164_writeb(dvo, SIL164_REG8, ch);
- return;
}
static bool sil164_get_hw_state(struct intel_dvo_device *dvo)
{
- int ret;
- unsigned char ch;
+ uint8_t ch = 0;
+ sil164_readb(dvo, SIL164_REG8, &ch);
+ return ch & SIL164_8_PD;
+}
- ret = sil164_readb(dvo, SIL164_REG8, &ch);
- if (ret == false)
- return false;
+static void sil164_dump_reg(struct intel_dvo_device *dvo,
+ int reg, const char *name)
+{
+ uint8_t val;
- if (ch & SIL164_8_PD)
- return true;
+ if (sil164_readb(dvo, reg, &val))
+ DRM_DEBUG_KMS("%s: 0x%02x\n", name, val);
else
- return false;
+ DRM_DEBUG_KMS("%s: failed\n", name);
}
static void sil164_dump_regs(struct intel_dvo_device *dvo)
{
- uint8_t val;
-
- sil164_readb(dvo, SIL164_FREQ_LO, &val);
- DRM_DEBUG_KMS("SIL164_FREQ_LO: 0x%02x\n", val);
- sil164_readb(dvo, SIL164_FREQ_HI, &val);
- DRM_DEBUG_KMS("SIL164_FREQ_HI: 0x%02x\n", val);
- sil164_readb(dvo, SIL164_REG8, &val);
- DRM_DEBUG_KMS("SIL164_REG8: 0x%02x\n", val);
- sil164_readb(dvo, SIL164_REG9, &val);
- DRM_DEBUG_KMS("SIL164_REG9: 0x%02x\n", val);
- sil164_readb(dvo, SIL164_REGC, &val);
- DRM_DEBUG_KMS("SIL164_REGC: 0x%02x\n", val);
+#define DUMP(R) sil164_dump_reg(dvo, R, #R)
+ DUMP(SIL164_FREQ_LO);
+ DUMP(SIL164_FREQ_HI);
+ DUMP(SIL164_REG8);
+ DUMP(SIL164_REG9);
+ DUMP(SIL164_REGC);
+#undef DUMP
}
static void sil164_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c b/drivers/gpu/drm/i915/dvo_tfp410.c
index 7853719a0e81..c24e5a5b320f 100644
--- a/drivers/gpu/drm/i915/dvo_tfp410.c
+++ b/drivers/gpu/drm/i915/dvo_tfp410.c
@@ -262,38 +262,47 @@ static bool tfp410_get_hw_state(struct intel_dvo_device *dvo)
return false;
}
+static void tfp410_dump_reg8(struct intel_dvo_device *dvo,
+ int reg, const char *name)
+{
+ uint8_t val;
+
+ if (tfp410_readb(dvo, reg, &val))
+ DRM_DEBUG_KMS("%s: 0x%02X\n", name, val);
+ else
+ DRM_DEBUG_KMS("%s: failed\n", name);
+}
+
+static void tfp410_dump_reg16(struct intel_dvo_device *dvo,
+ int reg1, int reg2, const char *name)
+{
+ uint8_t val[2];
+
+ if (tfp410_readb(dvo, reg1, &val[0]) &&
+ tfp410_readb(dvo, reg2, &val[1]))
+ DRM_DEBUG_KMS("%s: 0x%02X%02X\n", name, val[1], val[0]);
+ else
+ DRM_DEBUG_KMS("%s: failed\n", name);
+}
+
static void tfp410_dump_regs(struct intel_dvo_device *dvo)
{
- uint8_t val, val2;
-
- tfp410_readb(dvo, TFP410_REV, &val);
- DRM_DEBUG_KMS("TFP410_REV: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_CTL_1, &val);
- DRM_DEBUG_KMS("TFP410_CTL1: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_CTL_2, &val);
- DRM_DEBUG_KMS("TFP410_CTL2: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_CTL_3, &val);
- DRM_DEBUG_KMS("TFP410_CTL3: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_USERCFG, &val);
- DRM_DEBUG_KMS("TFP410_USERCFG: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_DE_DLY, &val);
- DRM_DEBUG_KMS("TFP410_DE_DLY: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_DE_CTL, &val);
- DRM_DEBUG_KMS("TFP410_DE_CTL: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_DE_TOP, &val);
- DRM_DEBUG_KMS("TFP410_DE_TOP: 0x%02X\n", val);
- tfp410_readb(dvo, TFP410_DE_CNT_LO, &val);
- tfp410_readb(dvo, TFP410_DE_CNT_HI, &val2);
- DRM_DEBUG_KMS("TFP410_DE_CNT: 0x%02X%02X\n", val2, val);
- tfp410_readb(dvo, TFP410_DE_LIN_LO, &val);
- tfp410_readb(dvo, TFP410_DE_LIN_HI, &val2);
- DRM_DEBUG_KMS("TFP410_DE_LIN: 0x%02X%02X\n", val2, val);
- tfp410_readb(dvo, TFP410_H_RES_LO, &val);
- tfp410_readb(dvo, TFP410_H_RES_HI, &val2);
- DRM_DEBUG_KMS("TFP410_H_RES: 0x%02X%02X\n", val2, val);
- tfp410_readb(dvo, TFP410_V_RES_LO, &val);
- tfp410_readb(dvo, TFP410_V_RES_HI, &val2);
- DRM_DEBUG_KMS("TFP410_V_RES: 0x%02X%02X\n", val2, val);
+#define DUMP8(R) tfp410_dump_reg8(dvo, R, #R)
+#define DUMP16(R1, R2) tfp410_dump_reg16(dvo, R1, R2, #R1)
+ DUMP8(TFP410_REV);
+ DUMP8(TFP410_CTL_1);
+ DUMP8(TFP410_CTL_2);
+ DUMP8(TFP410_CTL_3);
+ DUMP8(TFP410_USERCFG);
+ DUMP8(TFP410_DE_DLY);
+ DUMP8(TFP410_DE_CTL);
+ DUMP8(TFP410_DE_TOP);
+ DUMP16(TFP410_DE_CNT_LO, TFP410_DE_CNT_HI);
+ DUMP16(TFP410_DE_LIN_LO, TFP410_DE_LIN_HI);
+ DUMP16(TFP410_H_RES_LO, TFP410_H_RES_HI);
+ DUMP16(TFP410_V_RES_LO, TFP410_V_RES_HI);
+#undef DUMP16
+#undef DUMP8
}
static void tfp410_destroy(struct intel_dvo_device *dvo)
--
2.1.4
More information about the Intel-gfx
mailing list