Re: [RFC v2 3/5] drm: Add HDMI infoframe helpers

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3207 bytes --]

On Thu, Dec 06, 2012 at 03:09:00PM +0100, Daniel Vetter wrote:
> On Wed, Dec 05, 2012 at 05:45:42PM +0100, Thierry Reding wrote:
[...]
> > diff --git a/drivers/gpu/drm/drm_hdmi.c b/drivers/gpu/drm/drm_hdmi.c
> > new file mode 100644
> > index 0000000..821ca56
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_hdmi.c
> > @@ -0,0 +1,71 @@
> > +/*
> > + * Copyright (C) 2012 Avionic Design GmbH
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/hdmi.h>
> > +
> > +#include <drm/drm_crtc.h>
> > +#include <drm/drm_hdmi.h>
> > +
> > +#include "drm_edid_modes.h"
> 
> This creates a 2nd copy of that massive table. Imo we could just shovel
> the infoframe helpers into the drm_edid.c file - allowing different
> helpers to be disabled isn't that useful, since most drivers will want
> them pretty much all anyway. Or at least most of them.

I can move this helper to drm_edid.c. But maybe it might be worth moving
the table from the header into a source file and just export it from
there.

> > +
> > +static inline unsigned int
> > +drm_mode_cea_vic(const struct drm_display_mode *mode)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < drm_num_cea_modes; i++)
> > +		if (drm_mode_equal(mode, &edid_cea_modes[i]))
> > +			return i + 1;
> > +
> > +	return 0;
> > +}
> 
> Same function in drm_edid will land through drm-intel tree in drm-next
> rsn. I'll send that pull request somewhen next week probably.
> 
> http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=374a868a726eb8a1cb28ba88805e51ce34222f8d

Yes, I'm aware of that patch (I reviewed it =) and I was planning on
dropping the implementation in this patch once the above patch makes
it into linux-next.

> > +/**
> > + * 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)
> > +{
> > +	int err;
> > +
> > +	if (!frame || !mode)
> > +		return -EINVAL;
> > +
> > +	err = hdmi_avi_infoframe_init(frame);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	frame->video_code = drm_mode_cea_vic(mode);
> > +	if (!frame->video_code)
> > +		return 0;
> > +
> > +	frame->picture_aspect = drm_display_mode_get_aspect(mode);
> > +	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
> 
> Note that the intel avi infoframe in intel_hdmi_set_avi_infoframe also
> sets the pixel repeat for double clocked modes with:
> 
> 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> 		avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;

I left that in place in the Intel driver, but I suppose we can just as
well move it to the infoframe helpers since it should be a generic
option.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel