On 12/05/2012 05:45 PM, Thierry Reding wrote: > Add a generic helper to fill in an HDMI AVI infoframe with data > extracted from a DRM display mode. That's a very nice patch series, comes in pretty handy. Thanks :) I've just one comment. > [...] > +static inline enum hdmi_picture_aspect > +drm_display_mode_get_aspect(const struct drm_display_mode *mode) > +{ > + enum hdmi_picture_aspect aspect = HDMI_PICTURE_ASPECT_NONE; > + > + if ((mode->hdisplay * 9) / 16 == mode->vdisplay) > + aspect = HDMI_PICTURE_ASPECT_16_9; > + else if ((mode->hdisplay * 3) / 4 == mode->vdisplay) > + aspect = HDMI_PICTURE_ASPECT_4_3; > + > + return aspect; > +} > + > +/** > + * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with > + * data from a DRM display mode > + * @frame: HDMI AVI infoframe > + * @mode: DRM display mode > + * > + * Returns 0 on success or a negative error code on failure. > + */ > +int > +drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > + const struct drm_display_mode *mode) > +{ [...] > + > + frame->picture_aspect = drm_display_mode_get_aspect(mode); As far as I know the picture aspect ratio is supposed to be the aspect ratio of the displayed content not the pixel aspect ratio. The receiver already knows the pixel aspect ratio since it knows both the pixel width and the height. The picture aspect ratio is something that could be set by a userspace application running in fullscreen, e.g. a video player. > + frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > + > + return 0; > +} _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel