On Fri, Jan 18, 2013 at 09:56:36AM +0100, Thierry Reding wrote:
> On Wed, Jan 16, 2013 at 03:27:14PM +0200, Ville Syrjälä wrote:
> > On Mon, Jan 14, 2013 at 03:30:22PM +0100, Thierry Reding wrote:
> > > Add generic helpers to pack HDMI infoframes into binary buffers.
> [...]
> > > +/**
> > > + * hdmi_avi_infoframe_init() - initialize an HDMI AVI infoframe
> > > + * @frame: HDMI AVI infoframe
> > > + *
> > > + * Returns 0 on success or a negative error code on failure.
> > > + */
> > > +int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame)
> > > +{
> > > + if (!frame)
> > > + return -EINVAL;
> >
> > There's quite a bit of error checking all around which seems a bit
> > pointless since this is all internal stuff. Any errors are bugs in
> > some driver code, so just letting the thing oops would be fine in my
> > opinion.
>
> Okay, I can remove some of the more obvious checks. Checking for space
> requirements might still be useful, so I'll leave that in.
>
> > > +ssize_t hdmi_spd_infoframe_pack(struct hdmi_spd_infoframe *frame, void
> > > *buffer,
> > > + size_t size)
> [...]
> > > + for (i = 0; i < sizeof(frame->vendor); i++)
> > > + ptr[i] = frame->vendor[i];
> > > +
> > > + for (i = 0; i < sizeof(frame->product); i++)
> > > + ptr[8 + i] = frame->product[i];
> >
> > memcpy()
>
> Done.
>
> > > +enum hdmi_spd_sdi {
> > > + HDMI_SPD_SDI_UNKNOWN,
> > > + HDMI_SPD_SDI_DSTB,
> > > + HDMI_SPD_SDI_DVDP,
> > > + HDMI_SPD_SDI_DVHS,
> > > + HDMI_SPD_SDI_HDDVR,
> > > + HDMI_SPD_SDI_DVC,
> > > + HDMI_SPD_SDI_DSC,
> > > + HDMI_SPD_SDI_VCD,
> > > + HDMI_SPD_SDI_GAME,
> > > + HDMI_SPD_SDI_PC,
> > > + HDMI_SPD_SDI_BD,
> > > + HDMI_SPD_SDI_SCD,
> >
> > I believe SACD is a more correct name.
>
> Done.
>
> > HD DVD and PMP are missing from this list.
>
> I can't find any reference to those in CEA-861-D. HDMI 1.4 doesn't have
> them either. Can you provide any pointers?
CEA-861-E has them.
>
> > > +enum hdmi_audio_coding_type {
> > > + HDMI_AUDIO_CODING_TYPE_STREAM,
> > > + HDMI_AUDIO_CODING_TYPE_IEC_60958,
> > > + HDMI_AUDIO_CODING_TYPE_AC3,
> > > + HDMI_AUDIO_CODING_TYPE_MPEG1,
> > > + HDMI_AUDIO_CODING_TYPE_MP3,
> > > + HDMI_AUDIO_CODING_TYPE_MPEG2,
> > > + HDMI_AUDIO_CODING_TYPE_AAC,
> > > + HDMI_AUDIO_CODING_TYPE_DTS,
> > > + HDMI_AUDIO_CODING_TYPE_ATRAC,
> > > + HDMI_AUDIO_CODING_TYPE_ONE_BIT_AUDIO,
> > > + HDMI_AUDIO_CODING_TYPE_DOLBY_DIGITAL_PLUS,
> > > + HDMI_AUDIO_CODING_TYPE_DTS_HD,
> > > + HDMI_AUDIO_CODING_TYPE_MAT_MLP,
> > > + HDMI_AUDIO_CODING_TYPE_DST,
> > > + HDMI_AUDIO_CODING_TYPE_WMPRO,
> >
> > These don't always quite match the names in CEA-861-E. Wouldn't it
> > be better to use matching names?
>
> Are HD-DVD and PMP above listed in CEA-861-E? I'll need to find a copy
> of it and will go over this again and match the names to those in the
> document.
Great.
> > Also the audio coding extension type is missing.
>
> I assume this is also part of CEA-861-E as I can't find a reference in
> -D?
Yeah, I only checked against CEA-861-E.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
