Re: [PATCH] drm/radeon: fix AVI infoframe generation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 8, 2013 at 7:46 AM, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
> 2013/6/7  <alexdeucher@xxxxxxxxx>:
>> From: Alex Deucher <alexander.deucher@xxxxxxx>
>>
>> - remove adding 2 to checksum, this breaks certain monitors
>> - properly emit the AVI infoframe version, not emitting
>> the version breaks some monitors.
>>
>> This should fix blank screen when HDMI audio is enabled on
>> certain monitors.
>
> Err, nack. I believe this it actually going to *break* some monitors
> compatibility.
>
> For some unknown reason AMD hardware uses 0x81 and 0x01 for type and
> version of infoframe. See this comment (written/published by you):
> /* The following packets and infoframes are required for HDMI:
>  * Packets:
>  * 0x00 - Null packet
>  * 0x01 - Audio clock regen
>  * 0x02 - Audio sample
>  * 0x03 - General Control
>  * Infoframes:
>  * 0x81 0x01 - AVI
>  * 0x83 0x03 - audio
>  */
>
> As you can see, AMD hardware uses 0x81 and 0x01. I've no idea why,
> according to the HDMI standard it should be 0x82 and 0x02. All other
> vendors seems to also use 0x82 and 0x02. In function
> hdmi_avi_infoframe_init type it set to 0x82 and 0x02.
>
> This type and version it's written anywhere, but they are used to
> calculate checksum. Checksum it what we store is frame[0x0]. We
> calculate checksum as 0x100 - sum (see hdmi_infoframe_checksum).
>
> Now... with common drm code we use 0x82 and 0x02 instead of 0x81 and
> 0x01. It means our "sum" is too big by 0x02. It means we have to use
> 0x100 - sum + 0x02 as checksum for AMD hardware.
>
> This whole hack was introduced in 92db7f6c860b8190571a9dc1fcbc16d003422fe8:
> drm/radeon/kms: workaround invalid AVI infoframe checksum issue
> and I'm pretty sure it was verified to fix somebody's HDMI mode. It
> also seems to be compatible with AMD specs (at least the part of it we
> can see in the quoted comment).
>
> For proof of fglrx doing the same, please see my e-mail where I
> provided dumps from registers when using fglrx:
> http://lists.freedesktop.org/archives/dri-devel/2011-December/017717.html
> (it was tested on 5 different cards!).

Actually, I think the patch is correct.  I think you were adding the
version twice:  From your examples:
[Rafał Miłecki][RV620] fglrx:
0x7454: 00 A8 5E 79	R600_HDMI_VIDEOINFOFRAME_0
0x7458: 00 28 00 10	R600_HDMI_VIDEOINFOFRAME_1
0x745C: 00 48 00 28	R600_HDMI_VIDEOINFOFRAME_2
0x7460: 02 00 00 48	R600_HDMI_VIDEOINFOFRAME_3
===================
(0x82 + 0x2 + 0xD) + 0x1F8 = 0x289
-0x289 = 0x77

The payload sum is not 0x1f8, it's 0x1f6.
00 + A8 + 5E + 00 +
00 + 28 + 00 + 10 +
00 + 48 + 00 + 28 +
00 + 48 =
0x1f6

Bits 25:24 of HDMI_VIDEOINFOFRAME_3 are the packet version, not part
of the payload.  So the total would be:
(0x82 + 0x2 + 0xD) + 0x1f6 = 0x287
-0x287 = 0x79

I think the issue is that we are not setting the version in bits 25:24
of HDMI_VIDEOINFOFRAME_3 so we were sending a version 0 AVI packet.

Alex

>
> When working on that patch I was talking & debugging with few people
> on IRC. That mean I don't have logs anymore, but I'm sure I was
> testing exactly that single value and it was immediately fixing
> selected displays. I mean testing by writing something like:
>
> avivotool regset 0x10884 0x00a85e4f
> vs.
> avivotool regset 0x10884 0x00a85e4d
>
> --
> Rafał
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux