From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: fixup infoframe support for sdvo Date: Sat, 19 May 2012 23:29:01 +0200 Message-ID: <20120519212901.GF4738@phenom.ffwll.local> References: <1336846921-2294-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id E4D2D9F385 for ; Sat, 19 May 2012 14:27:53 -0700 (PDT) Received: by werc12 with SMTP id c12so3133978wer.36 for ; Sat, 19 May 2012 14:27:53 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1336846921-2294-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Intel Graphics Development Cc: Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On Sat, May 12, 2012 at 08:22:00PM +0200, Daniel Vetter wrote: > 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=E4rdeman > 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=3D25732 > Signed-Off-by: Daniel Vetter Forwarded from the bug entry: Tested-by: Peter Ross (G35 SDVO-HDMI) > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > drivers/gpu/drm/i915/intel_sdvo.c | 11 +++++++++-- > 2 files changed, 11 insertions(+), 4 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index d51f27f..3e09188 100644 > --- 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/int= el_sdvo.c > index 7d3f238..125228e 100644 > --- 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 int= el_sdvo *intel_sdvo) > }; > uint8_t tx_rate =3D SDVO_HBUF_TX_VSYNC; > uint8_t set_buf_index[2] =3D { 1, 0 }; > - uint64_t *data =3D (uint64_t *)&avi_if; > + uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)]; > + uint64_t *data =3D (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] =3D 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 =3D 0; i < sizeof(avi_if); i +=3D 8) { > + for (i =3D 0; i < sizeof(sdvo_data); i +=3D 8) { > if (!intel_sdvo_set_value(intel_sdvo, > SDVO_CMD_SET_HBUF_DATA, > data, 8)) > -- = > 1.7.8.3 > = -- = Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48