Hello Thierry, 2015-11-17 13:55 GMT+01:00 Thierry Reding <treding@xxxxxxxxxx>: > On Mon, Nov 16, 2015 at 05:28:24PM +0100, Enric Balletbo Serra wrote: >> Hello Thierry, >> >> Many thanks for your comments. >> >> 2015-11-16 12:50 GMT+01:00 Thierry Reding <treding@xxxxxxxxxx>: >> > 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? >> > >> >> To be honest I'm not sure if I have the full picture. In the use case >> I'm trying there is a hardware mechanism to store the data and send >> the infoframe through a "Packet Send Control Register". > > Okay, sounds like the hardware will automatically send out packets at > the right time. That still leaves open the issue of how to ensure this > is synchronized with userspace. Perhaps this could be done by attaching > a property to a framebuffer, so that we'd know what exact frame the meta > data is attached to and when to update the FIFOs for the infoframe. > > Usually it's a good idea to send this type of patch as part of a larger > series precisely so that people can see how it is used. That should make > it easier to see if this is good enough or needs some more thought on > how to synchronize. Do you have any code that you could post that makes > use of this new infoframe? > I was thinking use this and other helpers in the anx7814 bridge driver[1], I thought that this patch should go through another tree, this is the reason why I send it separately, but If you want or you prefer I can send as part of these patch series. [1] https://lkml.org/lkml/2015/11/13/284 >> >> @@ -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. >> > >> >> I don't have any inconvenient to change, are the following names more >> canonical ? >> >> HDMI_MPEG_UNKNOWN_FRAME = 0x00, >> HDMI_MPEG_I_FRAME = 0x01, >> HDMI_MPEG_B_FRAME = 0x02, >> HDMI_MPEG_P_FRAME = 0x03, > > I wasn't very clear. What I meant was the names for the constants. At > least personally I know immediately what is meant when I see "I-Frame", > "P-Frame" or "B-Frame", whereas "Bi-predictive picture" needs more > thinking. > Got it, I'll change only the names in next version, then. > Thierry Thanks, Enric _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
