drivers/video/hdmi.c: Source Product Description (SPD) Information frame logging
damien.lespiau at intel.com
Tue Aug 26 06:31:02 PDT 2014
On Tue, Aug 26, 2014 at 12:46:32PM +0200, Martin Bugge (marbugge) wrote:
> Hello Damien
> I'm writing to you as you seems to be one of latest maintainers of:
> I wanted to add "Source Product Description information frame"
> logging for the drivers in
> But was advised to contact you and use some of the defines in
> As you can see in:
> Would you consider the addition of a hdmi_spd_infoframe_log function
> in hdmi.c a good idea ?
> And the same goes for other HDMI Information frames.
I don't see why not. Currently the code there is somewhat geared for
writing HDMI info frames, not really reading back/debug.
The various structures are not a 1:1 mapping with how the fields are
actually represented in an info frame packet, so you get a _pack()
function to serialize the info frame structures. Something to
consider, then, for your logging API would be to either:
- Give a buffer/size to your log() function and decode a raw buffer
- make an _unpack() function that takes a raw buffer and translate it
into one of the various infoframe structure and then have a _log
function that operates on the "unpacked" structure.
Another thing to keep in mind is that those "unpacked" versions of the
infoframes don't actually have all the fields. eg.
* Data byte 1, bit 4 has to be set if we provide the active format
* aspect ratio
if (frame->active_aspect & 0xf)
ptr |= BIT(4);
ptr = ((frame->colorimetry & 0x3) << 6) |
((frame->picture_aspect & 0x3) << 4) |
(frame->active_aspect & 0xf);
You can see that active_aspect needs two fields set, a "active aspect valid"
bit (data byte 1, bit 4) and the actual active_aspect value. To prevent the
user of the API to generate invalid infoframes (ie have a active_aspect value
but forgetting to set the valid bit), I decided to "hide" the valid bit and set
it automatically if active_aspect is set. This isn't just theoritical, we've
had the bug that forgot the valid bit before.
That's the details I can think about atm, I don't seen any reason to not
improve that code to be able to dump info frames. I have a slight preference to
have an _unpack() version and have the _log() functions operate on the
"expoded" structures, maybe Thierry/Paulo (in Cc.) have some input as well.
More information about the dri-devel