On Sat, Nov 14, 2015 at 07:38:19PM +0100, Enric Balletbo i Serra wrote: > The MPEG Source (MS) InfoFrame is in EIA/CEA-861B. It describes aspects of > the compressed video stream that were used to produce the uncompressed > video. > > The patch adds functions to work with MPEG InfoFrames. > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx> > --- > drivers/video/hdmi.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/hdmi.h | 24 ++++++++ > 2 files changed, 180 insertions(+) According to the CEA specification a source is expected to send this type of infoframe once per video frame. I'm curious how you envision this to be ensured. Would hardware provide a mechanism to store this data and send the infoframe automatically? How would you ensure that updates sent to the hardware match the upcoming frame? > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c [...] > @@ -435,6 +510,8 @@ hdmi_infoframe_pack(union hdmi_infoframe *frame, void *buffer, size_t size) > length = hdmi_vendor_any_infoframe_pack(&frame->vendor, > buffer, size); > break; > + case HDMI_INFOFRAME_TYPE_MPEG: > + length = hdmi_mpeg_infoframe_pack(&frame->mpeg, buffer, size); Missing "break;"? > @@ -899,6 +978,41 @@ static void hdmi_audio_infoframe_log(const char *level, > frame->downmix_inhibit ? "Yes" : "No"); > } > > +static const char *hdmi_mpeg_picture_get_name(enum hdmi_mpeg_picture_type type) > +{ > + switch (type) { > + case HDMI_MPEG_PICTURE_TYPE_UNKNOWN: > + return "Unknown"; > + case HDMI_MPEG_PICTURE_TYPE_I: > + return "Intra-coded picture"; > + case HDMI_MPEG_PICTURE_TYPE_B: > + return "Bi-predictive picture"; > + case HDMI_MPEG_PICTURE_TYPE_P: > + return "Predicted picture"; > + } I'd have chosen the slightly more canonical "I-Frame", "P-Frame", "B-Frame" here, but that's not a strong objection. > + return "Reserved"; There really can't be another value here, so I think this should either return NULL or even go as far as let it crash. There should be no way for the invalid value to make it this far. > +static int hdmi_mpeg_infoframe_unpack(struct hdmi_mpeg_infoframe *frame, > + void *buffer) > +{ > + u8 *ptr = buffer; > + > + if (ptr[0] != HDMI_INFOFRAME_TYPE_MPEG || > + ptr[1] != 1 || > + ptr[2] != HDMI_MPEG_INFOFRAME_SIZE) { > + return -EINVAL; > + } There's no need for the braces here. > @@ -314,6 +336,7 @@ union hdmi_vendor_any_infoframe { > * @spd: spd infoframe > * @vendor: union of all vendor infoframes > * @audio: audio infoframe > + * @mpeg: mpeg infoframe s/mpeg/MPEG/ Thierry
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel