drm/i915: fixup infoframe support for sdvo

At least the worst offenders:
- SDVO specifies that the encoder should compute the ecc. Testing also
  shows that we must not send the ecc field, so copy the dip_infoframe
  struct to a temporay place and avoid the ecc field. This way the avi
  infoframe is exactly 17 bytes long, which agrees with what the spec
  mandates as a minimal storage capacity (with the ecc field it would
  be 18 bytes).
- Only 17 when sending the avi infoframe. The SDVO spec explicitly
  says that sending more data than what the device announces results
  in undefined behaviour.
- Add __attribute__((packed)) to the avi and spd infoframes, for
  otherwise they're wrongly aligned. Noticed because the avi infoframe
  ended up being 18 bytes large instead of 17. We haven't noticed this
  yet because we don't use the uint16_t fields yet (which are the only
  ones that would be wrongly aligned).

This regression has been introduce by

3c17fe4b8f40a112a85758a9ab2aebf772bdd647 is the first bad commit
commit 3c17fe4b8f40a112a85758a9ab2aebf772bdd647
Author: David Härdeman <david@hardeman.nu>
Date:   Fri Sep 24 21:44:32 2010 +0200

    i915: enable AVI infoframe for intel_hdmi.c [v4]

Patch tested on my g33 with a sdvo hdmi adaptor.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=25732
Tested-by: Peter Ross <pross@xvid.org> (G35 SDVO-HDMI)
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d51f27faa202c4ab3404540c7c441396181e5b31..3e0918834e7e017cf865c48fcfa8677903c031f7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -280,12 +280,12 @@ struct dip_infoframe {
                        uint16_t bottom_bar_start;
                        uint16_t left_bar_end;
                        uint16_t right_bar_start;
-               } avi;
+               } __attribute__ ((packed)) avi;
                struct {
                        uint8_t vn[8];
                        uint8_t pd[16];
                        uint8_t sdi;
-               } spd;
+               } __attribute__ ((packed)) spd;
                uint8_t payload[27];
        } __attribute__ ((packed)) body;
 } __attribute__((packed));

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7d3f238e826562050c6fcb1771d765f7f1ad0e55..125228e77c505b65a857758b4cbd0e38ce78a2f7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -887,17 +887,24 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
        };
        uint8_t tx_rate = SDVO_HBUF_TX_VSYNC;
        uint8_t set_buf_index[2] = { 1, 0 };
-       uint64_t *data = (uint64_t *)&avi_if;
+       uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
+       uint64_t *data = (uint64_t *)sdvo_data;
        unsigned i;
 
        intel_dip_infoframe_csum(&avi_if);
 
+       /* 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));
+
        if (!intel_sdvo_set_value(intel_sdvo,
                                  SDVO_CMD_SET_HBUF_INDEX,
                                  set_buf_index, 2))
                return false;
 
-       for (i = 0; i < sizeof(avi_if); i += 8) {
+       for (i = 0; i < sizeof(sdvo_data); i += 8) {
                if (!intel_sdvo_set_value(intel_sdvo,
                                          SDVO_CMD_SET_HBUF_DATA,
                                          data, 8))