diff mbox

[RFC] drm/i915: Use generic HDMI infoframe helpers

Message ID 1354893089-3596-1-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Dec. 7, 2012, 3:11 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Use the generic HDMI infoframe helpers to get rid of the duplicate
implementation in the i915 driver.

This patch is based on the initial patch by Thierry Reding, but with a
different approach.

TODO:
  - The SDVO part is totally untested. I am not sure if the buffer
    size on the SDVO code must be a multiple of 4.
  - The HDMI part was tested only on SNB/CPT.
  - The patch is forcing pixel_repeat to 1 so I can properly test the
    patch. Remove this before the final version.
  - The correctnes of this patch depends on a fix on patch "video: add
    Generic HDMI infoframe helpers"

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/Kconfig           |    2 +
 drivers/gpu/drm/i915/intel_drv.h  |   62 +-----------
 drivers/gpu/drm/i915/intel_hdmi.c |  189 ++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_sdvo.c |   21 ++---
 4 files changed, 117 insertions(+), 157 deletions(-)

Comments

Daniel Vetter Dec. 7, 2012, 3:32 p.m. UTC | #1 
Hi Paulo,

On Fri, Dec 7, 2012 at 4:11 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Use the generic HDMI infoframe helpers to get rid of the duplicate
> implementation in the i915 driver.
>
> This patch is based on the initial patch by Thierry Reding, but with a
> different approach.
>
> TODO:
>   - The SDVO part is totally untested. I am not sure if the buffer
>     size on the SDVO code must be a multiple of 4.
>   - The HDMI part was tested only on SNB/CPT.
>   - The patch is forcing pixel_repeat to 1 so I can properly test the
>     patch. Remove this before the final version.
>   - The correctnes of this patch depends on a fix on patch "video: add
>     Generic HDMI infoframe helpers"
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I like what this looks like, so I think we have a rough plan for
integrating the proposed hdmi infoframe helpers into i915. Some minor
bikeshedding left, but we can look at this after your big vacation in
2013. For reference my bikeshed:

- Maybe extract the type argument sprinkling into a prep patch, that
way it's even more clear what's changing (and that very little
changes).

+       uint8_t buffer[32]; /* will be read as uint32_t, so 32, not 30 */

- ROUND_UP(30, sizeof(uint32_t)) and maybe use #defines for 30.

Cheers, Daniel
Thierry Reding Dec. 11, 2012, 8:35 a.m. UTC | #2 
On Fri, Dec 07, 2012 at 01:11:29PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Use the generic HDMI infoframe helpers to get rid of the duplicate
> implementation in the i915 driver.
> 
> This patch is based on the initial patch by Thierry Reding, but with a
> different approach.
> 
> TODO:
>   - The SDVO part is totally untested. I am not sure if the buffer
>     size on the SDVO code must be a multiple of 4.
>   - The HDMI part was tested only on SNB/CPT.
>   - The patch is forcing pixel_repeat to 1 so I can properly test the
>     patch. Remove this before the final version.
>   - The correctnes of this patch depends on a fix on patch "video: add
>     Generic HDMI infoframe helpers"
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi Paulo,

This looks good to me in general. I still think that writing the
infoframe to the registers could be refactored some more, but I suppose
it could just as well be done in a separate patch. Or not. It's your
code after all.

Thierry
Daniel Vetter Dec. 11, 2012, 10:39 a.m. UTC | #3 
On Fri, Dec 7, 2012 at 4:32 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> TODO:
>>   - The SDVO part is totally untested. I am not sure if the buffer
>>     size on the SDVO code must be a multiple of 4.

Forgotten to mention: SDVO actually needs alignment to blocks of 8,
but intel_sdvo_write_infoframe already takes care of all that so no
rounding of the buffer size is needed.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 94a4623..5225012 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -144,6 +144,8 @@  config DRM_I915
 	select INPUT if ACPI
 	select ACPI_VIDEO if ACPI
 	select ACPI_BUTTON if ACPI
+	select DRM_HDMI
+	select HDMI
 	help
 	  Choose this option if you have a system that has "Intel Graphics
 	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 522061c..2712396 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -26,6 +26,7 @@ 
 #define __INTEL_DRV_H__
 
 #include <linux/i2c.h>
+#include <linux/hdmi.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 #include <drm/drm_crtc.h>
@@ -276,63 +277,6 @@  struct cxsr_latency {
 #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
 #define to_intel_plane(x) container_of(x, struct intel_plane, base)
 
-#define DIP_HEADER_SIZE	5
-
-#define DIP_TYPE_AVI    0x82
-#define DIP_VERSION_AVI 0x2
-#define DIP_LEN_AVI     13
-#define DIP_AVI_PR_1    0
-#define DIP_AVI_PR_2    1
-
-#define DIP_TYPE_SPD	0x83
-#define DIP_VERSION_SPD	0x1
-#define DIP_LEN_SPD	25
-#define DIP_SPD_UNKNOWN	0
-#define DIP_SPD_DSTB	0x1
-#define DIP_SPD_DVDP	0x2
-#define DIP_SPD_DVHS	0x3
-#define DIP_SPD_HDDVR	0x4
-#define DIP_SPD_DVC	0x5
-#define DIP_SPD_DSC	0x6
-#define DIP_SPD_VCD	0x7
-#define DIP_SPD_GAME	0x8
-#define DIP_SPD_PC	0x9
-#define DIP_SPD_BD	0xa
-#define DIP_SPD_SCD	0xb
-
-struct dip_infoframe {
-	uint8_t type;		/* HB0 */
-	uint8_t ver;		/* HB1 */
-	uint8_t len;		/* HB2 - body len, not including checksum */
-	uint8_t ecc;		/* Header ECC */
-	uint8_t checksum;	/* PB0 */
-	union {
-		struct {
-			/* PB1 - Y 6:5, A 4:4, B 3:2, S 1:0 */
-			uint8_t Y_A_B_S;
-			/* PB2 - C 7:6, M 5:4, R 3:0 */
-			uint8_t C_M_R;
-			/* PB3 - ITC 7:7, EC 6:4, Q 3:2, SC 1:0 */
-			uint8_t ITC_EC_Q_SC;
-			/* PB4 - VIC 6:0 */
-			uint8_t VIC;
-			/* PB5 - YQ 7:6, CN 5:4, PR 3:0 */
-			uint8_t YQ_CN_PR;
-			/* PB6 to PB13 */
-			uint16_t top_bar_end;
-			uint16_t bottom_bar_start;
-			uint16_t left_bar_end;
-			uint16_t right_bar_start;
-		} __attribute__ ((packed)) avi;
-		struct {
-			uint8_t vn[8];
-			uint8_t pd[16];
-			uint8_t sdi;
-		} __attribute__ ((packed)) spd;
-		uint8_t payload[27];
-	} __attribute__ ((packed)) body;
-} __attribute__((packed));
-
 struct intel_hdmi {
 	u32 sdvox_reg;
 	int ddc_bus;
@@ -341,7 +285,8 @@  struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
-				struct dip_infoframe *frame);
+				uint8_t *frame, ssize_t len,
+				enum hdmi_infoframe_type type);
 	void (*set_infoframes)(struct drm_encoder *encoder,
 			       struct drm_display_mode *adjusted_mode);
 };
@@ -430,7 +375,6 @@  extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 extern bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode);
-extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
 extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
 			    bool is_sdvob);
 extern void intel_dvo_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5c279b4..c4cf8f8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -29,9 +29,11 @@ 
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/hdmi.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_hdmi.h>
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
@@ -66,88 +68,75 @@  static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
 	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
 }
 
-void intel_dip_infoframe_csum(struct dip_infoframe *frame)
+static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
 {
-	uint8_t *data = (uint8_t *)frame;
-	uint8_t sum = 0;
-	unsigned i;
-
-	frame->checksum = 0;
-	frame->ecc = 0;
-
-	for (i = 0; i < frame->len + DIP_HEADER_SIZE; i++)
-		sum += data[i];
-
-	frame->checksum = 0x100 - sum;
-}
-
-static u32 g4x_infoframe_index(struct dip_infoframe *frame)
-{
-	switch (frame->type) {
-	case DIP_TYPE_AVI:
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
 		return VIDEO_DIP_SELECT_AVI;
-	case DIP_TYPE_SPD:
+	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_SELECT_SPD;
 	default:
-		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
+		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
 	}
 }
 
-static u32 g4x_infoframe_enable(struct dip_infoframe *frame)
+static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
 {
-	switch (frame->type) {
-	case DIP_TYPE_AVI:
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
 		return VIDEO_DIP_ENABLE_AVI;
-	case DIP_TYPE_SPD:
+	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_ENABLE_SPD;
 	default:
-		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
+		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
 	}
 }
 
-static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
+static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
 {
-	switch (frame->type) {
-	case DIP_TYPE_AVI:
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
 		return VIDEO_DIP_ENABLE_AVI_HSW;
-	case DIP_TYPE_SPD:
+	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_ENABLE_SPD_HSW;
 	default:
-		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
+		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
 	}
 }
 
-static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe)
+static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type, enum pipe pipe)
 {
-	switch (frame->type) {
-	case DIP_TYPE_AVI:
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
 		return HSW_TVIDEO_DIP_AVI_DATA(pipe);
-	case DIP_TYPE_SPD:
+	case HDMI_INFOFRAME_TYPE_SPD:
 		return HSW_TVIDEO_DIP_SPD_DATA(pipe);
 	default:
-		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
+		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
 	}
 }
 
 static void g4x_write_infoframe(struct drm_encoder *encoder,
-				struct dip_infoframe *frame)
+				uint8_t *frame,
+				ssize_t len,
+				enum hdmi_infoframe_type type)
 {
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 val = I915_READ(VIDEO_DIP_CTL);
-	unsigned i, len = DIP_HEADER_SIZE + frame->len;
+	unsigned int i;
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(frame);
+	val |= g4x_infoframe_index(type);
 
-	val &= ~g4x_infoframe_enable(frame);
+	val &= ~g4x_infoframe_enable(type);
 
 	I915_WRITE(VIDEO_DIP_CTL, val);
 
@@ -161,7 +150,7 @@  static void g4x_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(VIDEO_DIP_DATA, 0);
 	mmiowb();
 
-	val |= g4x_infoframe_enable(frame);
+	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
@@ -170,22 +159,24 @@  static void g4x_write_infoframe(struct drm_encoder *encoder,
 }
 
 static void ibx_write_infoframe(struct drm_encoder *encoder,
-				struct dip_infoframe *frame)
+				uint8_t *frame,
+				ssize_t len,
+				enum hdmi_infoframe_type type)
 {
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	unsigned i, len = DIP_HEADER_SIZE + frame->len;
+	unsigned int i;
 	u32 val = I915_READ(reg);
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(frame);
+	val |= g4x_infoframe_index(type);
 
-	val &= ~g4x_infoframe_enable(frame);
+	val &= ~g4x_infoframe_enable(type);
 
 	I915_WRITE(reg, val);
 
@@ -199,7 +190,7 @@  static void ibx_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();
 
-	val |= g4x_infoframe_enable(frame);
+	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
@@ -208,25 +199,27 @@  static void ibx_write_infoframe(struct drm_encoder *encoder,
 }
 
 static void cpt_write_infoframe(struct drm_encoder *encoder,
-				struct dip_infoframe *frame)
+				uint8_t *frame,
+				ssize_t len,
+				enum hdmi_infoframe_type type)
 {
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
-	unsigned i, len = DIP_HEADER_SIZE + frame->len;
+	unsigned int i;
 	u32 val = I915_READ(reg);
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(frame);
+	val |= g4x_infoframe_index(type);
 
 	/* The DIP control register spec says that we need to update the AVI
 	 * infoframe without clearing its enable bit */
-	if (frame->type != DIP_TYPE_AVI)
-		val &= ~g4x_infoframe_enable(frame);
+	if (type != HDMI_INFOFRAME_TYPE_AVI)
+		val &= ~g4x_infoframe_enable(type);
 
 	I915_WRITE(reg, val);
 
@@ -240,31 +233,34 @@  static void cpt_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();
 
-	val |= g4x_infoframe_enable(frame);
+	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
 	I915_WRITE(reg, val);
 	POSTING_READ(reg);
+
 }
 
 static void vlv_write_infoframe(struct drm_encoder *encoder,
-				     struct dip_infoframe *frame)
+				uint8_t *frame,
+				ssize_t len,
+				enum hdmi_infoframe_type type)
 {
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
-	unsigned i, len = DIP_HEADER_SIZE + frame->len;
+	unsigned int i;
 	u32 val = I915_READ(reg);
 
 	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
-	val |= g4x_infoframe_index(frame);
+	val |= g4x_infoframe_index(type);
 
-	val &= ~g4x_infoframe_enable(frame);
+	val &= ~g4x_infoframe_enable(type);
 
 	I915_WRITE(reg, val);
 
@@ -278,7 +274,7 @@  static void vlv_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();
 
-	val |= g4x_infoframe_enable(frame);
+	val |= g4x_infoframe_enable(type);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
@@ -287,21 +283,23 @@  static void vlv_write_infoframe(struct drm_encoder *encoder,
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
-				struct dip_infoframe *frame)
+				uint8_t *frame,
+				ssize_t len,
+				enum hdmi_infoframe_type type)
 {
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
-	unsigned int i, len = DIP_HEADER_SIZE + frame->len;
+	u32 data_reg = hsw_infoframe_data_reg(type, intel_crtc->pipe);
+	unsigned int i;
 	u32 val = I915_READ(ctl_reg);
 
 	if (data_reg == 0)
 		return;
 
-	val &= ~hsw_infoframe_enable(frame);
+	val &= ~hsw_infoframe_enable(type);
 	I915_WRITE(ctl_reg, val);
 
 	mmiowb();
@@ -314,48 +312,67 @@  static void hsw_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(data_reg + i, 0);
 	mmiowb();
 
-	val |= hsw_infoframe_enable(frame);
+	val |= hsw_infoframe_enable(type);
 	I915_WRITE(ctl_reg, val);
 	POSTING_READ(ctl_reg);
 }
 
-static void intel_set_infoframe(struct drm_encoder *encoder,
-				struct dip_infoframe *frame)
+static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
+					 struct drm_display_mode *adjusted_mode)
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	struct hdmi_avi_infoframe frame;
+	uint8_t buffer[20]; /* will be read as uin32_t, so 20 not 18 */
+	ssize_t len;
 
-	intel_dip_infoframe_csum(frame);
-	intel_hdmi->write_infoframe(encoder, frame);
-}
+	memset(buffer, 0, sizeof(buffer));
 
-static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
-					 struct drm_display_mode *adjusted_mode)
-{
-	struct dip_infoframe avi_if = {
-		.type = DIP_TYPE_AVI,
-		.ver = DIP_VERSION_AVI,
-		.len = DIP_LEN_AVI,
-	};
+	len = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
+	WARN(len < 0, "drm_hdmi_avi_infoframe_from_display_mode failed\n");
+
+	/* if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) helps me test */
+		frame.pixel_repeat = 1;
+
+	len = hdmi_avi_infoframe_pack(&frame, buffer + 1, sizeof(buffer) - 1);
+	WARN(len < 0, "hdmi_avi_infoframe_pack failed\n");
 
-	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
-		avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
+	/* Insert the ECC */
+	buffer[0] = buffer[1];
+	buffer[1] = buffer[2];
+	buffer[2] = buffer[3];
+	buffer[3] = 0;
+	len++;
 
-	intel_set_infoframe(encoder, &avi_if);
+	intel_hdmi->write_infoframe(encoder, buffer, len,
+				    HDMI_INFOFRAME_TYPE_AVI);
 }
 
 static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
 {
-	struct dip_infoframe spd_if;
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	struct hdmi_spd_infoframe frame;
+	uint8_t buffer[32]; /* will be read as uint32_t, so 32, not 30 */
+	ssize_t len;
+
+	memset(buffer, 0, sizeof(buffer));
+
+	len = hdmi_spd_infoframe_init(&frame, "Intel", "Integrated gfx");
+	WARN(len < 0, "hdmi_spd_infoframe_init failed\n");
+
+	frame.sdi = HDMI_SPD_SDI_PC;
+
+	len = hdmi_spd_infoframe_pack(&frame, buffer + 1, sizeof(buffer) - 1);
+	WARN(len < 0, "hdmi_spd_infoframe_pack failed\n");
 
-	memset(&spd_if, 0, sizeof(spd_if));
-	spd_if.type = DIP_TYPE_SPD;
-	spd_if.ver = DIP_VERSION_SPD;
-	spd_if.len = DIP_LEN_SPD;
-	strcpy(spd_if.body.spd.vn, "Intel");
-	strcpy(spd_if.body.spd.pd, "Integrated gfx");
-	spd_if.body.spd.sdi = DIP_SPD_PC;
+	/* Insert the ECC */
+	buffer[0] = buffer[1];
+	buffer[1] = buffer[2];
+	buffer[2] = buffer[3];
+	buffer[3] = 0;
+	len++;
 
-	intel_set_infoframe(encoder, &spd_if);
+	intel_hdmi->write_infoframe(encoder, buffer, len,
+				    HDMI_INFOFRAME_TYPE_SPD);
 }
 
 static void g4x_set_infoframes(struct drm_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a4bee83..867eb65 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -29,6 +29,7 @@ 
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/export.h>
+#include <linux/hdmi.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
@@ -935,20 +936,16 @@  static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 
 static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
 {
-	struct dip_infoframe avi_if = {
-		.type = DIP_TYPE_AVI,
-		.ver = DIP_VERSION_AVI,
-		.len = DIP_LEN_AVI,
-	};
-	uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
+	uint8_t sdvo_data[HDMI_AVI_INFOFRAME_SIZE +
+			  HDMI_INFOFRAME_HEADER_SIZE];
+	struct hdmi_avi_infoframe frame;
+	ssize_t len;
 
-	intel_dip_infoframe_csum(&avi_if);
+	len = hdmi_avi_infoframe_init(&frame);
+	WARN(len < 0, "hdmi_avi_infoframe_init failed!\n");
 
-	/* sdvo spec says that the ecc is handled by the hw, and it looks like
-	 * we must not send the ecc field, either. */
-	memcpy(sdvo_data, &avi_if, 3);
-	sdvo_data[3] = avi_if.checksum;
-	memcpy(&sdvo_data[4], &avi_if.body, sizeof(avi_if.body.avi));
+	len = hdmi_avi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data));
+	WARN(len < 0, "hdmi_avi_infoframe_pack failed!\n");
 
 	return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
 					  SDVO_HBUF_TX_VSYNC,