[RFC 0/2] Add HDMI helpers

[Thierry Reding]

On Fri, Nov 23, 2012 at 10:24:22AM +0100, Christian König wrote:
> Hi Thierry,
> 
> On 21.11.2012 16:01, Thierry Reding wrote:
> >This small series is very much work in progress, but I still wanted to
> >get feedback in this early stage to gather requirements from the folks
> >working on the display drivers that these helpers target.
> >
> >Patch 1 in the series adds a generic helper to pack a structure that
> >describes an HDMI AVI infoframe into the binary format as specified in
> >the HDMI specification. The resulting binary buffer should be easily
> >programmable into the HDMI controller.
> >
> >Patch 2 provides a helper to fill an HDMI AVI infoframe with data from
> >a struct drm_display_mode.
> >
> >This is all pretty rough right now, but I think some feedback would be
> >good at this point, to see if the design is at all sensible. I should
> >also mention that I haven't actually tested this on real hardware yet.
> >Furthermore I have plans to add something similar for the other types
> >of infoframes specified by HDMI once the direction becomes clearer.
> In general I like the idea of storing the informations in a C struct
> and only packing it into the binary form when needed.
> 
> I would rather like to see a complete implementation of all the
> interesting HDMI packets, including the necessary
> calculations/tables for audio timing recovery etc before it gets
> committed upstream.

I'm afraid I'm not very familiar with those packets. Also I haven't seen
any actual users of audio timing recovery packets so far, so I'm not
sure that adding support for them would be very useful.

What I have added so far is support for audio and vendor-specific
infoframes, since those have actual users. I also noticed that i915
implements SPD infoframes as well, so I can extract that code into the
generic helpers as well.

> Not sure about the separate configuration option. I'm not so much
> into the config/build system of linux (I know that it is rather
> complicated), but in general I would like to see that activated
> automatically as soon as any driver starts using it (or at least the
> driver depending on that option to be active).

The configuration option isn't supposed to be user visible. Drivers that
make use of this are supposed to select HDMI or DRM_HDMI to cause the
generic code to be pulled in.

> Also two notes about the code in itself:
> 1. hdmi_avi_infoframe_pack: gets the size of the target buffer, but
> unfortunately doesn't checks it.

That should already be fixed in the latest code in my local working
branch.

> 2. Separate the CRC calculation, we probably need that more than once.

I've also pulled that out into a separate function. I haven't made it a
public function yet, but I don't think that'll be necessary either since
it is used internally by the various infoframe packing functions.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121123/b4a9834e/attachment.pgp>