Re: [RFC v2 5/5] drm/i915: Use generic HDMI infoframe helpers

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Dec 05, 2012 at 05:45:44PM +0100, Thierry Reding wrote:
> Use the generic HDMI infoframe helpers to get rid of the duplicate
> implementation in the i915 driver.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 4b07401..192d791 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -29,6 +29,7 @@
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>  #include <linux/export.h>
> +#include <linux/hdmi.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> @@ -935,20 +936,17 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>  
>  static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
>  {
> -	struct dip_infoframe avi_if = {
> -		.type = DIP_TYPE_AVI,
> -		.ver = DIP_VERSION_AVI,
> -		.len = DIP_LEN_AVI,
> -	};
> -	uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
> +	uint8_t sdvo_data[HDMI_AVI_INFOFRAME_SIZE];
> +	struct hdmi_avi_infoframe frame;
> +	ssize_t len;
>  
> -	intel_dip_infoframe_csum(&avi_if);
> +	len = hdmi_avi_infoframe_init(&frame);
> +	if (len < 0)
> +		return false;
>  
> -	/* sdvo spec says that the ecc is handled by the hw, and it looks like
> -	 * we must not send the ecc field, either. */
> -	memcpy(sdvo_data, &avi_if, 3);
> -	sdvo_data[3] = avi_if.checksum;

This difference between how we need to send the infoframe to the hw
between sdvo (without ecc field present) and native hdmi (ecc field
present, but still computed by the hw) seems to have been lost. On a quick
read from your infoframe packer it looks like the ecc field isn't part of
the packed infoframe, so this patch would break native hdmi ports (in the
intel_hdmi.c file).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch