[PATCH 2/3] [VPG HSW-A] drm/i915: Add PAR support in AVI infoframe

[Ville Syrjälä]

On Thu, Aug 15, 2013 at 10:29:02AM +0530, vandana.kannan at intel.com wrote:
> From: vkannan <vandana.kannan at intel.com>
> 
> Populate picture aspect ratio field of AVI infoframe.
> If there is a custom value to be set for aspect ratio, it takes highest
> priority, followed by a check in the CEA mode list. If the mode is not
> found in the standard mode list, aspect ratio is calculated based on aspect
> ratio.
> 
> Priority of PAR value: 1. custom value, 2. based on CEA mode list,
> 3. calculate from resolution

There's been a lot of change to the infoframe code lately. We're
using the generic infoframe helpers these days, so this thing will need
to be rebased.

> 
> Signed-off-by: Vandana Kannan <vandana.kannan at intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        |    7 +++++++
>  drivers/gpu/drm/i915/intel_hdmi.c |   25 +++++++++++++++++++++++++
>  include/drm/drm_crtc.h            |    1 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 83e2fda..110a56f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2427,6 +2427,13 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>  }
>  EXPORT_SYMBOL(drm_match_cea_mode);
>  
> +enum hdmi_picture_aspect drm_get_cea_aspect_ratio(u8 vic)
> +{
> +	/*return Aspect Ratio for VIC-1 to access the right array element*/
> +	return edid_cea_modes[vic-1].picture_aspect_ratio;
> +}
> +EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
> +
>  static int
>  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 1e6b5cf..7cf6fc5 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -29,6 +29,7 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/hdmi.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> @@ -334,10 +335,12 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  {
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> +	enum hdmi_picture_aspect PAR;
>  	struct dip_infoframe avi_if = {
>  		.type = DIP_TYPE_AVI,
>  		.ver = DIP_VERSION_AVI,
>  		.len = DIP_LEN_AVI,
> +		.body.avi.C_M_R = 8,
>  	};
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> @@ -352,6 +355,28 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  
>  	avi_if.body.avi.VIC = drm_match_cea_mode(adjusted_mode);
>  
> +	/*If picture aspect ratio (PAR) is set to custom value, then use that,
> +	else if VIC > 1, then get PAR from CEA mode list, else, calculate
> +	PAR based on resolution */
> +	if (adjusted_mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> +	adjusted_mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) {
> +		avi_if.body.avi.C_M_R |=
> +			adjusted_mode->picture_aspect_ratio << 4;
> +		/*PAR is bit 5:4 of data byte 2 of AVI infoframe */
> +	} else if (avi_if.body.avi.VIC) {
> +		PAR = drm_get_cea_aspect_ratio(avi_if.body.avi.VIC);
> +		avi_if.body.avi.C_M_R |= PAR << 4;
> +	} else {
> +		if (!(adjusted_mode->vdisplay % 3) &&
> +			((adjusted_mode->vdisplay * 4 / 3) ==
> +			adjusted_mode->hdisplay))
> +			avi_if.body.avi.C_M_R |= HDMI_PICTURE_ASPECT_4_3 << 4;
> +		else if (!(adjusted_mode->vdisplay % 9) &&
> +			((adjusted_mode->vdisplay * 16 / 9) ==
> +			adjusted_mode->hdisplay))
> +			avi_if.body.avi.C_M_R |= HDMI_PICTURE_ASPECT_16_9 << 4;
> +	}

I think what we need is a new property to control the aspect ratio.

I'm thinking something like this:
enum {
	"Automatic",
	"Preferred", /* M=0 /*
	"4:3",  /* M=1 */
	"16:9", /* M=2 */
}

If the user selects anything but "Automatic" we just directly set the M
bits to the appropriate value. Also we should probably extend
drm_match_cea_mode() so that it will also compare the user provided
aspect ratio w/ the VIC aspect ratio. That way we would send a VIC which
matches (in the 4:3 and 16:9 cases) the user selected aspect ratio. 
Although the spec does say that the M bits will take precendence over
the VIC, but given how badly this stuff is generally implemented in
displays, I have a feeling that some displays might only look at the VIC.

For the "Automatic" case we should just pick the first VIC that
otherwise matches the display mode. If there's no VIC for the mode, we
could just send M=0, or we could try to determine the aspect ratio from
the resolution like in your patch.


> +
>  	intel_set_infoframe(encoder, &avi_if);
>  }
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 07c0d58..e215bcc 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1048,6 +1048,7 @@ extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  				    void *data, struct drm_file *file_priv);
>  extern u8 *drm_find_cea_extension(struct edid *edid);
>  extern u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
> +extern enum hdmi_picture_aspect drm_get_cea_aspect_ratio(u8 vic);
>  extern bool drm_detect_hdmi_monitor(struct edid *edid);
>  extern bool drm_detect_monitor_audio(struct edid *edid);
>  extern bool drm_rgb_quant_range_selectable(struct edid *edid);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC