On Wed, Dec 05, 2012 at 06:51:20PM +0100, Lars-Peter Clausen wrote:
> 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.
Isn't that what the active aspect ratio is supposed to do? Unfortunately
neither the HDMI specification nor CEA-861-D seem to be very specific
about this. What I did in the above was basically just refactoring from
what the Tegra driver does. None of the other drivers are filling in any
useful values here. Most equipment seems to work just fine if you pass a
zeroed out AVI infoframe. =)
Given that we don't have a way to pass any kind of information to the
kernel about this, the only options we have are to either use this or go
with HDMI_PICTURE_ASPECT_NONE. I suppose that the latter will work fine
too.
Does anybody else have more information about what the correct value for
this is?
Thierry
Attachment:
pgplbkKvsENea.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
