* [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2]
@ 2010-09-09 21:00 David Härdeman
2010-09-09 21:58 ` Chris Wilson
2010-09-10 22:04 ` David Härdeman
0 siblings, 2 replies; 6+ messages in thread
From: David Härdeman @ 2010-09-09 21:00 UTC (permalink / raw)
To: intel-gfx; +Cc: jesse.barnes
This patch enables the sending of AVI infoframes in
drivers/gpu/drm/i915/intel_hdmi.c.
My receiver currently loses sync when the HDMI output on my computer
(DG45FC motherboard) is switched from 800x600 (the BIOS resolution) to
1920x1080 as part of the boot. Fixable by switching inputs on the receiver
a couple of times.
With this patch, my receiver has not lost sync yet (> 40 tries).
I'm not sure that the general approach is sound, which is why I'm sending
this out for some feedback and review (in particular, is the SDVOB and
SDVOC handling in intel_hdmi_set_avi_infoframe() correct?).
This is the second version which merges the infoframe code from
intel_hdmi.c and intel_sdvo.c, I hope this is something along the lines
Chris Wilson had in mind. Note that I'm assuming that the sdvo hardware
also stores a header ECC byte in the MSB of the first dword (haven't found
any documentation for the sdvo).
---
drivers/gpu/drm/i915/i915_reg.h | 16 +++++
drivers/gpu/drm/i915/intel_drv.h | 27 +++++++++
drivers/gpu/drm/i915/intel_hdmi.c | 76 ++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_sdvo.c | 117 +++++--------------------------------
4 files changed, 136 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 67e3ec1..6c2d9a6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1330,6 +1330,22 @@
#define LVDS_B0B3_POWER_DOWN (0 << 2)
#define LVDS_B0B3_POWER_UP (3 << 2)
+/* Video Data Island Packet control */
+#define VIDEO_DIP_DATA 0x61178
+#define VIDEO_DIP_CTL 0x61170
+#define VIDEO_DIP_ENABLE (1 << 31)
+#define VIDEO_DIP_PORT_B (1 << 29)
+#define VIDEO_DIP_PORT_C (2 << 29)
+#define VIDEO_DIP_ENABLE_AVI (1 << 21)
+#define VIDEO_DIP_ENABLE_VENDOR (2 << 21)
+#define VIDEO_DIP_ENABLE_SPD (8 << 21)
+#define VIDEO_DIP_SELECT_AVI (0 << 19)
+#define VIDEO_DIP_SELECT_VENDOR (1 << 19)
+#define VIDEO_DIP_SELECT_SPD (3 << 19)
+#define VIDEO_DIP_FREQ_ONCE (0 << 16)
+#define VIDEO_DIP_FREQ_VSYNC (1 << 16)
+#define VIDEO_DIP_FREQ_2VSYNC (2 << 16)
+
/* Panel power sequencing */
#define PP_STATUS 0x61200
#define PP_ON (1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e92aa0..f5808e7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -176,6 +176,30 @@ struct intel_crtc {
#define enc_to_intel_encoder(x) container_of(x, struct intel_encoder, enc)
#define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
+#define DIP_TYPE_AVI 0x82
+#define DIP_VERSION_AVI 0x2
+#define DIP_LEN_AVI 13
+
+struct dip_infoframe {
+ uint8_t type; /* HB0 */
+ uint8_t ver; /* HB1 */
+ uint8_t len; /* HB2 - body len, not including checksum */
+ uint8_t ecc; /* HW specific, not part of the infoframe */
+ uint8_t checksum; /* PB0 */
+ uint8_t Y_A_B_S; /* PB1 - Y 6:5, A 4:4, B 3:2, S 1:0 */
+ uint8_t C_M_R; /* PB2 - C 7:6, M 5:4, R 3:0 */
+ uint8_t ITC_EC_Q_SC; /* PB3 - ITC 7:7, EC 6:4, Q 3:2, SC 1:0 */
+ uint8_t VIC; /* PB4 - VIC 6:0 */
+ uint8_t PR; /* PB5 - PR 3:0 */
+ /* PB6 - PB13 */
+ uint16_t top_bar_end;
+ uint16_t bottom_bar_start;
+ uint16_t left_bar_end;
+ uint16_t right_bar_start;
+ /* Pad to 32-bit */
+ uint16_t padding;
+} __attribute__((packed));
+
struct i2c_adapter *intel_i2c_create(struct drm_device *dev, const u32 reg,
const char *name);
void intel_i2c_destroy(struct i2c_adapter *adapter);
@@ -186,6 +210,9 @@ void intel_i2c_reset_gmbus(struct drm_device *dev);
extern void intel_crt_init(struct drm_device *dev);
extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
+extern bool intel_hdmi_proc_infoframe(struct drm_encoder *encoder,
+ struct dip_infoframe *avi_if,
+ bool (*write_func)(struct drm_encoder *, uint8_t *));
extern bool intel_sdvo_init(struct drm_device *dev, int output_device);
extern void intel_dvo_init(struct drm_device *dev);
extern void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ccd4c97..4fc323c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -48,6 +48,79 @@ static struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder)
return container_of(enc_to_intel_encoder(encoder), struct intel_hdmi, base);
}
+static void intel_hdmi_calc_infoframe_csum(struct dip_infoframe *avi_if)
+{
+ uint8_t *data = (uint8_t *)avi_if;
+ uint8_t sum = 0;
+ unsigned i;
+
+ avi_if->checksum = 0;
+ avi_if->ecc = 0;
+ avi_if->padding = 0;
+
+ for (i = 0; i < sizeof(*avi_if); i++)
+ sum += data[i];
+
+ avi_if->checksum = 0x100 - sum;
+}
+
+static bool intel_hdmi_write_avi_infoframe(struct drm_encoder *encoder,
+ uint8_t *data)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ I915_WRITE(VIDEO_DIP_DATA, *((u32 *)data));
+ return true;
+}
+
+bool intel_hdmi_proc_infoframe(struct drm_encoder *encoder,
+ struct dip_infoframe *avi_if,
+ bool (*write_func)(struct drm_encoder *, uint8_t *))
+{
+ unsigned size;
+ uint8_t *data = (uint8_t *)avi_if;
+
+ intel_hdmi_calc_infoframe_csum(avi_if);
+ for (size = sizeof(*avi_if); size > 0; size -= 4) {
+ if (!write_func(encoder, data))
+ return false;
+ data += 4;
+ }
+ return true;
+}
+
+static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
+{
+ struct dip_infoframe avi_if = {
+ .type = DIP_TYPE_AVI,
+ .ver = DIP_VERSION_AVI,
+ .len = DIP_LEN_AVI,
+ .Y_A_B_S = 0x02,
+ .C_M_R = 0x28,
+ };
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+ u32 port;
+
+ if (!intel_hdmi->has_hdmi_sink)
+ return;
+
+ if (intel_hdmi->sdvox_reg == SDVOB)
+ port = VIDEO_DIP_PORT_B;
+ else if (intel_hdmi->sdvox_reg == SDVOC)
+ port = VIDEO_DIP_PORT_C;
+ else
+ return;
+
+ I915_WRITE(VIDEO_DIP_CTL, VIDEO_DIP_ENABLE | port | VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC);
+ POSTING_READ(VIDEO_DIP_ENABLE);
+ intel_hdmi_proc_infoframe(encoder, &avi_if, intel_hdmi_write_avi_infoframe);
+ I915_WRITE(VIDEO_DIP_CTL, VIDEO_DIP_ENABLE | port | VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC);
+ POSTING_READ(VIDEO_DIP_ENABLE);
+}
+
static void intel_hdmi_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
@@ -67,6 +140,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
if (intel_hdmi->has_hdmi_sink) {
sdvox |= SDVO_AUDIO_ENABLE;
+ sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
if (HAS_PCH_CPT(dev))
sdvox |= HDMI_MODE_SELECT;
}
@@ -80,6 +154,8 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
POSTING_READ(intel_hdmi->sdvox_reg);
+
+ intel_hdmi_set_avi_infoframe(encoder);
}
static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 093e914..b99ec72 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -874,115 +874,32 @@ static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
}
#endif
-static bool intel_sdvo_set_hdmi_buf(struct intel_sdvo *intel_sdvo,
- int index,
- uint8_t *data, int8_t size, uint8_t tx_rate)
+static bool intel_sdvo_write_avi_infoframe(struct drm_encoder *encoder,
+ uint8_t *data)
{
- uint8_t set_buf_index[2];
-
- set_buf_index[0] = index;
- set_buf_index[1] = 0;
-
- if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_INDEX,
- set_buf_index, 2))
- return false;
-
- for (; size > 0; size -= 8) {
- if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_DATA, data, 8))
- return false;
-
- data += 8;
- }
-
- return intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_TXRATE, &tx_rate, 1);
-}
-
-static uint8_t intel_sdvo_calc_hbuf_csum(uint8_t *data, uint8_t size)
-{
- uint8_t csum = 0;
- int i;
-
- for (i = 0; i < size; i++)
- csum += data[i];
+ struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
- return 0x100 - csum;
+ return intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_DATA, data, 4);
}
-#define DIP_TYPE_AVI 0x82
-#define DIP_VERSION_AVI 0x2
-#define DIP_LEN_AVI 13
-
-struct dip_infoframe {
- uint8_t type;
- uint8_t version;
- uint8_t len;
- uint8_t checksum;
- union {
- struct {
- /* Packet Byte #1 */
- uint8_t S:2;
- uint8_t B:2;
- uint8_t A:1;
- uint8_t Y:2;
- uint8_t rsvd1:1;
- /* Packet Byte #2 */
- uint8_t R:4;
- uint8_t M:2;
- uint8_t C:2;
- /* Packet Byte #3 */
- uint8_t SC:2;
- uint8_t Q:2;
- uint8_t EC:3;
- uint8_t ITC:1;
- /* Packet Byte #4 */
- uint8_t VIC:7;
- uint8_t rsvd2:1;
- /* Packet Byte #5 */
- uint8_t PR:4;
- uint8_t rsvd3:4;
- /* Packet Byte #6~13 */
- uint16_t top_bar_end;
- uint16_t bottom_bar_start;
- uint16_t left_bar_end;
- uint16_t right_bar_start;
- } avi;
- struct {
- /* Packet Byte #1 */
- uint8_t channel_count:3;
- uint8_t rsvd1:1;
- uint8_t coding_type:4;
- /* Packet Byte #2 */
- uint8_t sample_size:2; /* SS0, SS1 */
- uint8_t sample_frequency:3;
- uint8_t rsvd2:3;
- /* Packet Byte #3 */
- uint8_t coding_type_private:5;
- uint8_t rsvd3:3;
- /* Packet Byte #4 */
- uint8_t channel_allocation;
- /* Packet Byte #5 */
- uint8_t rsvd4:3;
- uint8_t level_shift:4;
- uint8_t downmix_inhibit:1;
- } audio;
- uint8_t payload[28];
- } __attribute__ ((packed)) u;
-} __attribute__((packed));
-
-static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
- struct drm_display_mode * mode)
+static bool intel_sdvo_set_avi_infoframe(struct drm_encoder *encoder)
{
+ struct intel_sdvo *intel_sdvo = enc_to_intel_sdvo(encoder);
struct dip_infoframe avi_if = {
.type = DIP_TYPE_AVI,
- .version = DIP_VERSION_AVI,
+ .ver = DIP_VERSION_AVI,
.len = DIP_LEN_AVI,
};
+ uint8_t buf[2] = { 1, 0 };
+
+ if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_INDEX, buf, 2))
+ return false;
+
+ if (!intel_hdmi_proc_infoframe(encoder, &avi_if, intel_sdvo_write_avi_infoframe))
+ return false;
- avi_if.checksum = intel_sdvo_calc_hbuf_csum((uint8_t *)&avi_if,
- 4 + avi_if.len);
- return intel_sdvo_set_hdmi_buf(intel_sdvo, 1, (uint8_t *)&avi_if,
- 4 + avi_if.len,
- SDVO_HBUF_TX_VSYNC);
+ buf[0] = SDVO_HBUF_TX_VSYNC;
+ return intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_HBUF_TXRATE, buf, 1);
}
static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
@@ -1114,7 +1031,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
return;
if (intel_sdvo->is_hdmi) {
- if (!intel_sdvo_set_avi_infoframe(intel_sdvo, mode))
+ if (!intel_sdvo_set_avi_infoframe(encoder))
return;
sdvox |= SDVO_AUDIO_ENABLE;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2]
2010-09-09 21:00 [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2] David Härdeman
@ 2010-09-09 21:58 ` Chris Wilson
2010-09-10 8:13 ` David Härdeman
2010-09-20 22:01 ` David Härdeman
2010-09-10 22:04 ` David Härdeman
1 sibling, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2010-09-09 21:58 UTC (permalink / raw)
To: intel-gfx; +Cc: jesse.barnes
[-- Attachment #1: Type: text/plain, Size: 3819 bytes --]
On Thu, 09 Sep 2010 23:00:01 +0200, David Härdeman <david@hardeman.nu> wrote:
> This is the second version which merges the infoframe code from
> intel_hdmi.c and intel_sdvo.c, I hope this is something along the lines
> Chris Wilson had in mind. Note that I'm assuming that the sdvo hardware
> also stores a header ECC byte in the MSB of the first dword (haven't found
> any documentation for the sdvo).
Pretty close. Using a callback function for writing the data is overkill,
just call the common function to compute the avi csum, then pass the avi
to the sdvo or hdmi handlers. It just makes following the logic that
little bit easier.
drm-intel-next has changed slightly since you wrote the patch, so can you
respin it - just a couple of the macro names have changed to be
consistent.
> ---
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ccd4c97..4fc323c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -48,6 +48,79 @@ static struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder)
> return container_of(enc_to_intel_encoder(encoder), struct intel_hdmi, base);
> }
>
> +static void intel_hdmi_calc_infoframe_csum(struct dip_infoframe *avi_if)
void intel_dip_infoframe_csum(struct dip_infoframe *avi_if)
> +{
> + uint8_t *data = (uint8_t *)avi_if;
> + uint8_t sum = 0;
> + unsigned i;
> +
> + avi_if->checksum = 0;
> + avi_if->ecc = 0;
> + avi_if->padding = 0;
> +
> + for (i = 0; i < sizeof(*avi_if); i++)
> + sum += data[i];
> +
> + avi_if->checksum = 0x100 - sum;
> +}
> +
> +static bool intel_hdmi_write_avi_infoframe(struct drm_encoder *encoder,
> + uint8_t *data)
> +{
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + I915_WRITE(VIDEO_DIP_DATA, *((u32 *)data));
> + return true;
> +}
> +
> +bool intel_hdmi_proc_infoframe(struct drm_encoder *encoder,
> + struct dip_infoframe *avi_if,
> + bool (*write_func)(struct drm_encoder *, uint8_t *))
> +{
> + unsigned size;
> + uint8_t *data = (uint8_t *)avi_if;
> +
> + intel_hdmi_calc_infoframe_csum(avi_if);
> + for (size = sizeof(*avi_if); size > 0; size -= 4) {
> + if (!write_func(encoder, data))
> + return false;
> + data += 4;
> + }
> + return true;
> +}
This I think is unnecessary, especially as the current SDVO code handles
it in 8 byte chunks.
> +
> +static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
> +{
> + struct dip_infoframe avi_if = {
> + .type = DIP_TYPE_AVI,
> + .ver = DIP_VERSION_AVI,
> + .len = DIP_LEN_AVI,
> + .Y_A_B_S = 0x02,
> + .C_M_R = 0x28,
> + };
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> + u32 port;
> +
> + if (!intel_hdmi->has_hdmi_sink)
> + return;
> +
> + if (intel_hdmi->sdvox_reg == SDVOB)
> + port = VIDEO_DIP_PORT_B;
> + else if (intel_hdmi->sdvox_reg == SDVOC)
> + port = VIDEO_DIP_PORT_C;
> + else
> + return;
> +
> + I915_WRITE(VIDEO_DIP_CTL, VIDEO_DIP_ENABLE | port | VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC);
Split this up over multiple lines to fit within 80-columns.
> + POSTING_READ(VIDEO_DIP_ENABLE);
Shouldn't need to flush the write here.
> + intel_hdmi_proc_infoframe(encoder, &avi_if, intel_hdmi_write_avi_infoframe);
I think this works better as:
intel_dip_infofame_csum(&avi_if);
intel_hdmi_write_dip_infoframe(encoder, &avi_if);
> + I915_WRITE(VIDEO_DIP_CTL, VIDEO_DIP_ENABLE | port | VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC);
> + POSTING_READ(VIDEO_DIP_ENABLE);
Same as above.
Thanks, adding a second copy of that data structure would have been too
ugly.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2]
2010-09-09 21:58 ` Chris Wilson
@ 2010-09-10 8:13 ` David Härdeman
2010-09-20 22:01 ` David Härdeman
1 sibling, 0 replies; 6+ messages in thread
From: David Härdeman @ 2010-09-10 8:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, jesse.barnes
On Thu, September 9, 2010 23:58, Chris Wilson wrote:
> On Thu, 09 Sep 2010 23:00:01 +0200, David Härdeman <david@hardeman.nu>
> wrote:
>> This is the second version which merges the infoframe code from
>> intel_hdmi.c and intel_sdvo.c, I hope this is something along the lines
>> Chris Wilson had in mind. Note that I'm assuming that the sdvo hardware
>> also stores a header ECC byte in the MSB of the first dword (haven't
>> found
>> any documentation for the sdvo).
>
> Pretty close. Using a callback function for writing the data is overkill,
> just call the common function to compute the avi csum, then pass the avi
> to the sdvo or hdmi handlers. It just makes following the logic that
> little bit easier.
>
> drm-intel-next has changed slightly since you wrote the patch, so can you
> respin it - just a couple of the macro names have changed to be
> consistent.
I'll respin the patch, but I still need to know if the intel_sdvo.c
hardware has an ECC byte as the MSB of the first dword or not.
--
David Härdeman
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2]
2010-09-09 21:58 ` Chris Wilson
2010-09-10 8:13 ` David Härdeman
@ 2010-09-20 22:01 ` David Härdeman
2010-09-21 8:02 ` Chris Wilson
1 sibling, 1 reply; 6+ messages in thread
From: David Härdeman @ 2010-09-20 22:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, jesse.barnes
On Thu, Sep 09, 2010 at 10:58:00PM +0100, Chris Wilson wrote:
> On Thu, 09 Sep 2010 23:00:01 +0200, David Härdeman <david@hardeman.nu> wrote:
> > This is the second version which merges the infoframe code from
> > intel_hdmi.c and intel_sdvo.c, I hope this is something along the lines
> > Chris Wilson had in mind. Note that I'm assuming that the sdvo hardware
> > also stores a header ECC byte in the MSB of the first dword (haven't found
> > any documentation for the sdvo).
>
> drm-intel-next has changed slightly since you wrote the patch, so can
> you
> respin it - just a couple of the macro names have changed to be
> consistent.
I based my patch on this:
http://git.kernel.org/?p=linux/kernel/git/anholt/drm-intel.git;a=shortlog;h=drm-intel-next
And I'm not seeing any such changes...am I using the wrong tree/branch?
--
David Härdeman
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2]
2010-09-09 21:00 [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2] David Härdeman
2010-09-09 21:58 ` Chris Wilson
@ 2010-09-10 22:04 ` David Härdeman
1 sibling, 0 replies; 6+ messages in thread
From: David Härdeman @ 2010-09-10 22:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jesse.barnes
On Thu, Sep 09, 2010 at 11:00:01PM +0200, David Härdeman wrote:
> This is the second version which merges the infoframe code from
> intel_hdmi.c and intel_sdvo.c, I hope this is something along the lines
> Chris Wilson had in mind. Note that I'm assuming that the sdvo hardware
> also stores a header ECC byte in the MSB of the first dword (haven't found
> any documentation for the sdvo).
Just to clarify that last sentence a bit...
intel_sdvo.c currently defines this struct:
struct dip_infoframe {
uint8_t type;
uint8_t version;
uint8_t len;
uint8_t checksum;
union {
...
uint8_t payload[28];
] __attribute__ ((packed)) u;
} __attribute__((packed));
If the "checksum" member refers to the body checksum of the infoframe,
then the payload size is incorrect. The checksum is the first byte of
the body which is 28 bytes in total (HDMI spec 1.3a, table 5-15) so the
payload array should be 27 bytes. If the payload size is corrected to 27
bytes, then the size of the entire struct is 31 bytes and the writing
(which is done in 8 byte chunks in intel_sdvo.c) needs to be fixed.
If, on the other hand, the checksum field refers to the header ecc
checksum, then the payload member is correctly sized but the body
checksum is missing.
My guess is that the sdvo hardware has the same setup as described in
http://intellinuxgraphics.org/VOL_3_display_registers_updated.pdf,
section 2.8.9, meaning that the correct struct would be:
struct dip_infoframe {
uint8_t type;
uint8_t version;
uint8_t len;
uint8_t ecc;
uint8_t checksum;
union {
...
uint8_t payload[27];
] __attribute__ ((packed)) u;
} __attribute__((packed));
Which would give a 32 byte struct which can be written in 8-byte chunks
and which has the correct body size.
However, I can't confirm that suspicion...see:
http://software.intel.com/en-us/forums/showannouncement.php?a=17
Could someone at Intel please clarify?
On a related note, the struct currently defined in intel_sdvo.c relies
on bitfields and expect the bits to be encoded in a certain order (which
is the reason a casual reader might get the impression that the order of
the members of the struct is reversed when compared to the relevant
specs). However, the order of the bits within a unit is
implementation-defined (C99, 6.7.2.1, point 10), so it shouldn't be
relied upon...which is why I didn't use bitfields in my patch.
--
David Härdeman
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-21 8:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-09 21:00 [PATCH] i915: enable AVI infoframe for intel_hdmi.c [v2] David Härdeman
2010-09-09 21:58 ` Chris Wilson
2010-09-10 8:13 ` David Härdeman
2010-09-20 22:01 ` David Härdeman
2010-09-21 8:02 ` Chris Wilson
2010-09-10 22:04 ` David Härdeman
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.