diff mbox

drm/exynos/hdmi: refactor infoframe code

Message ID 1477485391-8911-1-git-send-email-a.hajda@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrzej Hajda Oct. 26, 2016, 12:36 p.m. UTC 
Use core helpers to generate infoframes and generate vendor frame if necessary.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 141 ++++++++++-------------------------
 drivers/gpu/drm/exynos/regs-hdmi.h   |   2 +
 2 files changed, 42 insertions(+), 101 deletions(-)

Comments

Inki Dae Nov. 7, 2016, 1:45 a.m. UTC | #1 
2016년 10월 26일 21:36에 Andrzej Hajda 이(가) 쓴 글:
> Use core helpers to generate infoframes and generate vendor frame if necessary.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 141 ++++++++++-------------------------
>  drivers/gpu/drm/exynos/regs-hdmi.h   |   2 +
>  2 files changed, 42 insertions(+), 101 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index e8fb6ef..1bb2df7 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -47,19 +47,6 @@
>  
>  #define HOTPLUG_DEBOUNCE_MS		1100
>  
> -/* AVI header and aspect ratio */
> -#define HDMI_AVI_VERSION		0x02
> -#define HDMI_AVI_LENGTH			0x0d
> -
> -/* AUI header info */
> -#define HDMI_AUI_VERSION		0x01
> -#define HDMI_AUI_LENGTH			0x0a
> -
> -/* AVI active format aspect ratio */
> -#define AVI_SAME_AS_PIC_ASPECT_RATIO	0x08
> -#define AVI_4_3_CENTER_RATIO		0x09
> -#define AVI_16_9_CENTER_RATIO		0x0a
> -
>  enum hdmi_type {
>  	HDMI_TYPE13,
>  	HDMI_TYPE14,
> @@ -131,7 +118,6 @@ struct hdmi_context {
>  	bool				dvi_mode;
>  	struct delayed_work		hotplug_work;
>  	struct drm_display_mode		current_mode;
> -	u8				cea_video_id;
>  	const struct hdmi_driver_data	*drv_data;
>  
>  	void __iomem			*regs;
> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context *hdata, u32 reg_id,
>  	}
>  }
>  
> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
> +				      u8 *buf, int size)
> +{
> +	for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
> +		writel(*buf++, hdata->regs + reg_id);
> +}
> +
>  static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>  				 u32 reg_id, u32 value, u32 mask)
>  {
> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
>  	return ret;
>  }
>  
> -static u8 hdmi_chksum(struct hdmi_context *hdata,
> -			u32 start, u8 len, u32 hdr_sum)
> -{
> -	int i;
> -
> -	/* hdr_sum : header0 + header1 + header2
> -	* start : start address of packet byte1
> -	* len : packet bytes - 1 */
> -	for (i = 0; i < len; ++i)
> -		hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
> -
> -	/* return 2's complement of 8 bit hdr_sum */
> -	return (u8)(~(hdr_sum & 0xff) + 1);
> -}
> -
> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
> -			union hdmi_infoframe *infoframe)
> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  {
> -	u32 hdr_sum;
> -	u8 chksum;
> -	u8 ar;
> +	union hdmi_infoframe frm;
> +	u8 buf[25];
> +	int ret;
>  
>  	if (hdata->dvi_mode) {
> -		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
> -				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON,
>  				HDMI_AVI_CON_DO_NOT_TRANSMIT);
> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
> +				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>  		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
>  		return;
>  	}
>  
> -	switch (infoframe->any.type) {
> -	case HDMI_INFOFRAME_TYPE_AVI:
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> +			&hdata->current_mode);
> +	if (ret >= 0)

Seems above condition is not clear becuase drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative value.

> +		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
> +	if (ret > 0) {
>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);

I think above code should be called when drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value from hdmi_avi_infoframe_pack function is more than 1.

> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
> -				infoframe->any.version);
> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
> -		hdr_sum = infoframe->any.type + infoframe->any.version +
> -			  infoframe->any.length;
> -
> -		/* Output format zero hardcoded ,RGB YBCR selection */
> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
> -			AVI_ACTIVE_FORMAT_VALID |
> -			AVI_UNDERSCANNED_DISPLAY_VALID);
> -
> -		/*
> -		 * Set the aspect ratio as per the mode, mentioned in
> -		 * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
> -		 */
> -		ar = hdata->current_mode.picture_aspect_ratio;
> -		switch (ar) {
> -		case HDMI_PICTURE_ASPECT_4_3:
> -			ar |= AVI_4_3_CENTER_RATIO;
> -			break;
> -		case HDMI_PICTURE_ASPECT_16_9:
> -			ar |= AVI_16_9_CENTER_RATIO;
> -			break;
> -		case HDMI_PICTURE_ASPECT_NONE:
> -		default:
> -			ar |= AVI_SAME_AS_PIC_ASPECT_RATIO;
> -			break;
> -		}
> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), ar);
> +		hdmi_reg_write_buf(hdata, HDMI_AVI_HEADER0, buf, ret);
> +	} else {
> +		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);

If drm_hdmi_avi_infoframe_from_display_mode function returns 0 on success, then above message will be printed out. Seems not reasonable.

> +	}
>  
> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(4), hdata->cea_video_id);
> +	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
> +			&hdata->current_mode);
> +	if (ret >= 0)

Ditto.

> +		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
> +				sizeof(buf));
> +	if (ret > 0) {
> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON, HDMI_VSI_CON_EVERY_VSYNC);
> +		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, ret);

Also above codes should be called when drm_hdmi_vendor_infoframe_from_display_mode function returned 0 and the value from hdmi_vendor_infoframe_pack function is more than 1.

> +	}
>  
> -		chksum = hdmi_chksum(hdata, HDMI_AVI_BYTE(1),
> -					infoframe->any.length, hdr_sum);
> -		DRM_DEBUG_KMS("AVI checksum = 0x%x\n", chksum);
> -		hdmi_reg_writeb(hdata, HDMI_AVI_CHECK_SUM, chksum);
> -		break;
> -	case HDMI_INFOFRAME_TYPE_AUDIO:
> -		hdmi_reg_writeb(hdata, HDMI_AUI_CON, 0x02);
> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER0, infoframe->any.type);
> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER1,
> -				infoframe->any.version);
> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER2, infoframe->any.length);
> -		hdr_sum = infoframe->any.type + infoframe->any.version +
> -			  infoframe->any.length;
> -		chksum = hdmi_chksum(hdata, HDMI_AUI_BYTE(1),
> -					infoframe->any.length, hdr_sum);
> -		DRM_DEBUG_KMS("AUI checksum = 0x%x\n", chksum);
> -		hdmi_reg_writeb(hdata, HDMI_AUI_CHECK_SUM, chksum);
> -		break;
> -	default:
> -		break;
> +	ret = hdmi_audio_infoframe_init(&frm.audio);
> +	if (ret >= 0) {

Ditto.

> +		frm.audio.channels = 2;
> +		ret = hdmi_audio_infoframe_pack(&frm.audio, buf, sizeof(buf));
> +	}
> +	if (ret > 0) {
> +		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
> +		hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, ret);

Also above codes should be called when hdmi_audio_infoframe_init function returned 0 and the value from hdmi_audio_infoframe_pack function is more than 1.

>  	}
>  }
>  
> @@ -1127,8 +1077,6 @@ static void hdmi_start(struct hdmi_context *hdata, bool start)
>  
>  static void hdmi_conf_init(struct hdmi_context *hdata)
>  {
> -	union hdmi_infoframe infoframe;
> -
>  	/* disable HPD interrupts from HDMI IP block, use GPIO instead */
>  	hdmi_reg_writemask(hdata, HDMI_INTC_CON, 0, HDMI_INTC_EN_GLOBAL |
>  		HDMI_INTC_EN_HPD_PLUG | HDMI_INTC_EN_HPD_UNPLUG);
> @@ -1164,15 +1112,7 @@ static void hdmi_conf_init(struct hdmi_context *hdata)
>  		hdmi_reg_writeb(hdata, HDMI_V13_AUI_CON, 0x02);
>  		hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 0x04);
>  	} else {
> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AVI;
> -		infoframe.any.version = HDMI_AVI_VERSION;
> -		infoframe.any.length = HDMI_AVI_LENGTH;
> -		hdmi_reg_infoframe(hdata, &infoframe);
> -
> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AUDIO;
> -		infoframe.any.version = HDMI_AUI_VERSION;
> -		infoframe.any.length = HDMI_AUI_LENGTH;
> -		hdmi_reg_infoframe(hdata, &infoframe);
> +		hdmi_reg_infoframes(hdata);
>  
>  		/* enable AVI packet every vsync, fixes purple line problem */
>  		hdmi_reg_writemask(hdata, HDMI_CON_1, 2, 3 << 5);
> @@ -1458,7 +1398,6 @@ static void hdmi_mode_set(struct drm_encoder *encoder,
>  		"INTERLACED" : "PROGRESSIVE");
>  
>  	drm_mode_copy(&hdata->current_mode, m);
> -	hdata->cea_video_id = drm_match_cea_mode(mode);
>  }
>  
>  static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
> index 169667a..a0507dc 100644
> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
> @@ -361,9 +361,11 @@
>  
>  /* AUI bit definition */
>  #define HDMI_AUI_CON_NO_TRAN		(0 << 0)
> +#define HDMI_AUI_CON_EVERY_VSYNC	(1 << 1)
>  
>  /* VSI bit definition */
>  #define HDMI_VSI_CON_DO_NOT_TRANSMIT	(0 << 0)
> +#define HDMI_VSI_CON_EVERY_VSYNC	(1 << 1)
>  
>  /* HDCP related registers */
>  #define HDMI_HDCP_SHA1(n)		HDMI_CORE_BASE(0x7000 + 4 * (n))
>
Andrzej Hajda Nov. 7, 2016, 8:05 a.m. UTC | #2 
On 07.11.2016 02:45, Inki Dae wrote:
>
> 2016년 10월 26일 21:36에 Andrzej Hajda 이(가) 쓴 글:
>> Use core helpers to generate infoframes and generate vendor frame if necessary.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 141 ++++++++++-------------------------
>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   2 +
>>  2 files changed, 42 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index e8fb6ef..1bb2df7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -47,19 +47,6 @@
>>  
>>  #define HOTPLUG_DEBOUNCE_MS		1100
>>  
>> -/* AVI header and aspect ratio */
>> -#define HDMI_AVI_VERSION		0x02
>> -#define HDMI_AVI_LENGTH			0x0d
>> -
>> -/* AUI header info */
>> -#define HDMI_AUI_VERSION		0x01
>> -#define HDMI_AUI_LENGTH			0x0a
>> -
>> -/* AVI active format aspect ratio */
>> -#define AVI_SAME_AS_PIC_ASPECT_RATIO	0x08
>> -#define AVI_4_3_CENTER_RATIO		0x09
>> -#define AVI_16_9_CENTER_RATIO		0x0a
>> -
>>  enum hdmi_type {
>>  	HDMI_TYPE13,
>>  	HDMI_TYPE14,
>> @@ -131,7 +118,6 @@ struct hdmi_context {
>>  	bool				dvi_mode;
>>  	struct delayed_work		hotplug_work;
>>  	struct drm_display_mode		current_mode;
>> -	u8				cea_video_id;
>>  	const struct hdmi_driver_data	*drv_data;
>>  
>>  	void __iomem			*regs;
>> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context *hdata, u32 reg_id,
>>  	}
>>  }
>>  
>> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
>> +				      u8 *buf, int size)
>> +{
>> +	for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
>> +		writel(*buf++, hdata->regs + reg_id);
>> +}
>> +
>>  static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>>  				 u32 reg_id, u32 value, u32 mask)
>>  {
>> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
>>  	return ret;
>>  }
>>  
>> -static u8 hdmi_chksum(struct hdmi_context *hdata,
>> -			u32 start, u8 len, u32 hdr_sum)
>> -{
>> -	int i;
>> -
>> -	/* hdr_sum : header0 + header1 + header2
>> -	* start : start address of packet byte1
>> -	* len : packet bytes - 1 */
>> -	for (i = 0; i < len; ++i)
>> -		hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
>> -
>> -	/* return 2's complement of 8 bit hdr_sum */
>> -	return (u8)(~(hdr_sum & 0xff) + 1);
>> -}
>> -
>> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
>> -			union hdmi_infoframe *infoframe)
>> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>>  {
>> -	u32 hdr_sum;
>> -	u8 chksum;
>> -	u8 ar;
>> +	union hdmi_infoframe frm;
>> +	u8 buf[25];
>> +	int ret;
>>  
>>  	if (hdata->dvi_mode) {
>> -		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>> -				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON,
>>  				HDMI_AVI_CON_DO_NOT_TRANSMIT);
>> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>> +				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>  		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
>>  		return;
>>  	}
>>  
>> -	switch (infoframe->any.type) {
>> -	case HDMI_INFOFRAME_TYPE_AVI:
>> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>> +			&hdata->current_mode);
>> +	if (ret >= 0)
> Seems above condition is not clear becuase drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative value.

So it means 'there is no error', I can change it to '!ret' or 'ret == 0'
if you prefer, I have just used '>= 0' to be more concise with next
error check.
>
>> +		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>> +	if (ret > 0) {
>>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
> I think above code should be called when drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value from hdmi_avi_infoframe_pack function is more than 1.

Why do you think 'more than 1' is better than 'more than 0' in this
case? hdmi_avi_infoframe_pack returns length of generated frame or
negative value in case of error,
so any positive value is OK, there is no special meaning for '1'.

>
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
>> -				infoframe->any.version);
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
>> -		hdr_sum = infoframe->any.type + infoframe->any.version +
>> -			  infoframe->any.length;
>> -
>> -		/* Output format zero hardcoded ,RGB YBCR selection */
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
>> -			AVI_ACTIVE_FORMAT_VALID |
>> -			AVI_UNDERSCANNED_DISPLAY_VALID);
>> -
>> -		/*
>> -		 * Set the aspect ratio as per the mode, mentioned in
>> -		 * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
>> -		 */
>> -		ar = hdata->current_mode.picture_aspect_ratio;
>> -		switch (ar) {
>> -		case HDMI_PICTURE_ASPECT_4_3:
>> -			ar |= AVI_4_3_CENTER_RATIO;
>> -			break;
>> -		case HDMI_PICTURE_ASPECT_16_9:
>> -			ar |= AVI_16_9_CENTER_RATIO;
>> -			break;
>> -		case HDMI_PICTURE_ASPECT_NONE:
>> -		default:
>> -			ar |= AVI_SAME_AS_PIC_ASPECT_RATIO;
>> -			break;
>> -		}
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), ar);
>> +		hdmi_reg_write_buf(hdata, HDMI_AVI_HEADER0, buf, ret);
>> +	} else {
>> +		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);
> If drm_hdmi_avi_infoframe_from_display_mode function returns 0 on success, then above message will be printed out. Seems not reasonable.

If drm_hdmi_avi_infoframe_from_display_mode returns 0, then
hdmi_avi_infoframe_pack is called and if the latter
fails (practically impossible) this message will be printed which is
correct behavior.

The same answer is for your next comments.

Regards
Andrzej

>
>> +	}
>>  
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(4), hdata->cea_video_id);
>> +	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
>> +			&hdata->current_mode);
>> +	if (ret >= 0)
> Ditto.
>
>> +		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
>> +				sizeof(buf));
>> +	if (ret > 0) {
>> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON, HDMI_VSI_CON_EVERY_VSYNC);
>> +		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, ret);
> Also above codes should be called when drm_hdmi_vendor_infoframe_from_display_mode function returned 0 and the value from hdmi_vendor_infoframe_pack function is more than 1.
>
>> +	}
>>  
>> -		chksum = hdmi_chksum(hdata, HDMI_AVI_BYTE(1),
>> -					infoframe->any.length, hdr_sum);
>> -		DRM_DEBUG_KMS("AVI checksum = 0x%x\n", chksum);
>> -		hdmi_reg_writeb(hdata, HDMI_AVI_CHECK_SUM, chksum);
>> -		break;
>> -	case HDMI_INFOFRAME_TYPE_AUDIO:
>> -		hdmi_reg_writeb(hdata, HDMI_AUI_CON, 0x02);
>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER0, infoframe->any.type);
>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER1,
>> -				infoframe->any.version);
>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER2, infoframe->any.length);
>> -		hdr_sum = infoframe->any.type + infoframe->any.version +
>> -			  infoframe->any.length;
>> -		chksum = hdmi_chksum(hdata, HDMI_AUI_BYTE(1),
>> -					infoframe->any.length, hdr_sum);
>> -		DRM_DEBUG_KMS("AUI checksum = 0x%x\n", chksum);
>> -		hdmi_reg_writeb(hdata, HDMI_AUI_CHECK_SUM, chksum);
>> -		break;
>> -	default:
>> -		break;
>> +	ret = hdmi_audio_infoframe_init(&frm.audio);
>> +	if (ret >= 0) {
> Ditto.
>
>> +		frm.audio.channels = 2;
>> +		ret = hdmi_audio_infoframe_pack(&frm.audio, buf, sizeof(buf));
>> +	}
>> +	if (ret > 0) {
>> +		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
>> +		hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, ret);
> Also above codes should be called when hdmi_audio_infoframe_init function returned 0 and the value from hdmi_audio_infoframe_pack function is more than 1.
>
>>  	}
>>  }
>>  
>> @@ -1127,8 +1077,6 @@ static void hdmi_start(struct hdmi_context *hdata, bool start)
>>  
>>  static void hdmi_conf_init(struct hdmi_context *hdata)
>>  {
>> -	union hdmi_infoframe infoframe;
>> -
>>  	/* disable HPD interrupts from HDMI IP block, use GPIO instead */
>>  	hdmi_reg_writemask(hdata, HDMI_INTC_CON, 0, HDMI_INTC_EN_GLOBAL |
>>  		HDMI_INTC_EN_HPD_PLUG | HDMI_INTC_EN_HPD_UNPLUG);
>> @@ -1164,15 +1112,7 @@ static void hdmi_conf_init(struct hdmi_context *hdata)
>>  		hdmi_reg_writeb(hdata, HDMI_V13_AUI_CON, 0x02);
>>  		hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 0x04);
>>  	} else {
>> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AVI;
>> -		infoframe.any.version = HDMI_AVI_VERSION;
>> -		infoframe.any.length = HDMI_AVI_LENGTH;
>> -		hdmi_reg_infoframe(hdata, &infoframe);
>> -
>> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AUDIO;
>> -		infoframe.any.version = HDMI_AUI_VERSION;
>> -		infoframe.any.length = HDMI_AUI_LENGTH;
>> -		hdmi_reg_infoframe(hdata, &infoframe);
>> +		hdmi_reg_infoframes(hdata);
>>  
>>  		/* enable AVI packet every vsync, fixes purple line problem */
>>  		hdmi_reg_writemask(hdata, HDMI_CON_1, 2, 3 << 5);
>> @@ -1458,7 +1398,6 @@ static void hdmi_mode_set(struct drm_encoder *encoder,
>>  		"INTERLACED" : "PROGRESSIVE");
>>  
>>  	drm_mode_copy(&hdata->current_mode, m);
>> -	hdata->cea_video_id = drm_match_cea_mode(mode);
>>  }
>>  
>>  static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
>> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
>> index 169667a..a0507dc 100644
>> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
>> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
>> @@ -361,9 +361,11 @@
>>  
>>  /* AUI bit definition */
>>  #define HDMI_AUI_CON_NO_TRAN		(0 << 0)
>> +#define HDMI_AUI_CON_EVERY_VSYNC	(1 << 1)
>>  
>>  /* VSI bit definition */
>>  #define HDMI_VSI_CON_DO_NOT_TRANSMIT	(0 << 0)
>> +#define HDMI_VSI_CON_EVERY_VSYNC	(1 << 1)
>>  
>>  /* HDCP related registers */
>>  #define HDMI_HDCP_SHA1(n)		HDMI_CORE_BASE(0x7000 + 4 * (n))
>>
>
Inki Dae Nov. 7, 2016, 8:14 a.m. UTC | #3 
2016년 11월 07일 17:05에 Andrzej Hajda 이(가) 쓴 글:
> On 07.11.2016 02:45, Inki Dae wrote:
>>
>> 2016년 10월 26일 21:36에 Andrzej Hajda 이(가) 쓴 글:
>>> Use core helpers to generate infoframes and generate vendor frame if necessary.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 141 ++++++++++-------------------------
>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   2 +
>>>  2 files changed, 42 insertions(+), 101 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index e8fb6ef..1bb2df7 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -47,19 +47,6 @@
>>>  
>>>  #define HOTPLUG_DEBOUNCE_MS		1100
>>>  
>>> -/* AVI header and aspect ratio */
>>> -#define HDMI_AVI_VERSION		0x02
>>> -#define HDMI_AVI_LENGTH			0x0d
>>> -
>>> -/* AUI header info */
>>> -#define HDMI_AUI_VERSION		0x01
>>> -#define HDMI_AUI_LENGTH			0x0a
>>> -
>>> -/* AVI active format aspect ratio */
>>> -#define AVI_SAME_AS_PIC_ASPECT_RATIO	0x08
>>> -#define AVI_4_3_CENTER_RATIO		0x09
>>> -#define AVI_16_9_CENTER_RATIO		0x0a
>>> -
>>>  enum hdmi_type {
>>>  	HDMI_TYPE13,
>>>  	HDMI_TYPE14,
>>> @@ -131,7 +118,6 @@ struct hdmi_context {
>>>  	bool				dvi_mode;
>>>  	struct delayed_work		hotplug_work;
>>>  	struct drm_display_mode		current_mode;
>>> -	u8				cea_video_id;
>>>  	const struct hdmi_driver_data	*drv_data;
>>>  
>>>  	void __iomem			*regs;
>>> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context *hdata, u32 reg_id,
>>>  	}
>>>  }
>>>  
>>> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
>>> +				      u8 *buf, int size)
>>> +{
>>> +	for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
>>> +		writel(*buf++, hdata->regs + reg_id);
>>> +}
>>> +
>>>  static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>>>  				 u32 reg_id, u32 value, u32 mask)
>>>  {
>>> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
>>>  	return ret;
>>>  }
>>>  
>>> -static u8 hdmi_chksum(struct hdmi_context *hdata,
>>> -			u32 start, u8 len, u32 hdr_sum)
>>> -{
>>> -	int i;
>>> -
>>> -	/* hdr_sum : header0 + header1 + header2
>>> -	* start : start address of packet byte1
>>> -	* len : packet bytes - 1 */
>>> -	for (i = 0; i < len; ++i)
>>> -		hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
>>> -
>>> -	/* return 2's complement of 8 bit hdr_sum */
>>> -	return (u8)(~(hdr_sum & 0xff) + 1);
>>> -}
>>> -
>>> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
>>> -			union hdmi_infoframe *infoframe)
>>> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>>>  {
>>> -	u32 hdr_sum;
>>> -	u8 chksum;
>>> -	u8 ar;
>>> +	union hdmi_infoframe frm;
>>> +	u8 buf[25];
>>> +	int ret;
>>>  
>>>  	if (hdata->dvi_mode) {
>>> -		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>>> -				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON,
>>>  				HDMI_AVI_CON_DO_NOT_TRANSMIT);
>>> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>>> +				HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>>  		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
>>>  		return;
>>>  	}
>>>  
>>> -	switch (infoframe->any.type) {
>>> -	case HDMI_INFOFRAME_TYPE_AVI:
>>> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>> +			&hdata->current_mode);
>>> +	if (ret >= 0)
>> Seems above condition is not clear becuase drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative value.
> 
> So it means 'there is no error', I can change it to '!ret' or 'ret == 0'
> if you prefer, I have just used '>= 0' to be more concise with next
> error check.

As I mentioned, never return bigger than 0. Let's change it to !ret or ret == 0.

>>
>>> +		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>>> +	if (ret > 0) {
>>>  		hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
>> I think above code should be called when drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value from hdmi_avi_infoframe_pack function is more than 1.
> 
> Why do you think 'more than 1' is better than 'more than 0' in this
> case? hdmi_avi_infoframe_pack returns length of generated frame or
> negative value in case of error,
> so any positive value is OK, there is no special meaning for '1'.

Because hdmi_avi_infoframe_pack returns 0 or bigger than 0. Anyway, seems you are saying exactly same thing.

> 
>>
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
>>> -				infoframe->any.version);
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
>>> -		hdr_sum = infoframe->any.type + infoframe->any.version +
>>> -			  infoframe->any.length;
>>> -
>>> -		/* Output format zero hardcoded ,RGB YBCR selection */
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
>>> -			AVI_ACTIVE_FORMAT_VALID |
>>> -			AVI_UNDERSCANNED_DISPLAY_VALID);
>>> -
>>> -		/*
>>> -		 * Set the aspect ratio as per the mode, mentioned in
>>> -		 * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
>>> -		 */
>>> -		ar = hdata->current_mode.picture_aspect_ratio;
>>> -		switch (ar) {
>>> -		case HDMI_PICTURE_ASPECT_4_3:
>>> -			ar |= AVI_4_3_CENTER_RATIO;
>>> -			break;
>>> -		case HDMI_PICTURE_ASPECT_16_9:
>>> -			ar |= AVI_16_9_CENTER_RATIO;
>>> -			break;
>>> -		case HDMI_PICTURE_ASPECT_NONE:
>>> -		default:
>>> -			ar |= AVI_SAME_AS_PIC_ASPECT_RATIO;
>>> -			break;
>>> -		}
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), ar);
>>> +		hdmi_reg_write_buf(hdata, HDMI_AVI_HEADER0, buf, ret);
>>> +	} else {
>>> +		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);
>> If drm_hdmi_avi_infoframe_from_display_mode function returns 0 on success, then above message will be printed out. Seems not reasonable.
> 
> If drm_hdmi_avi_infoframe_from_display_mode returns 0, then
> hdmi_avi_infoframe_pack is called and if the latter
> fails (practically impossible) this message will be printed which is
> correct behavior.

Right. The condition, "ret >= 0" made me confusing.

> 
> The same answer is for your next comments.
> 
> Regards
> Andrzej
> 
>>
>>> +	}
>>>  
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(4), hdata->cea_video_id);
>>> +	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
>>> +			&hdata->current_mode);
>>> +	if (ret >= 0)
>> Ditto.
>>
>>> +		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
>>> +				sizeof(buf));
>>> +	if (ret > 0) {
>>> +		hdmi_reg_writeb(hdata, HDMI_VSI_CON, HDMI_VSI_CON_EVERY_VSYNC);
>>> +		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, ret);
>> Also above codes should be called when drm_hdmi_vendor_infoframe_from_display_mode function returned 0 and the value from hdmi_vendor_infoframe_pack function is more than 1.
>>
>>> +	}
>>>  
>>> -		chksum = hdmi_chksum(hdata, HDMI_AVI_BYTE(1),
>>> -					infoframe->any.length, hdr_sum);
>>> -		DRM_DEBUG_KMS("AVI checksum = 0x%x\n", chksum);
>>> -		hdmi_reg_writeb(hdata, HDMI_AVI_CHECK_SUM, chksum);
>>> -		break;
>>> -	case HDMI_INFOFRAME_TYPE_AUDIO:
>>> -		hdmi_reg_writeb(hdata, HDMI_AUI_CON, 0x02);
>>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER0, infoframe->any.type);
>>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER1,
>>> -				infoframe->any.version);
>>> -		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER2, infoframe->any.length);
>>> -		hdr_sum = infoframe->any.type + infoframe->any.version +
>>> -			  infoframe->any.length;
>>> -		chksum = hdmi_chksum(hdata, HDMI_AUI_BYTE(1),
>>> -					infoframe->any.length, hdr_sum);
>>> -		DRM_DEBUG_KMS("AUI checksum = 0x%x\n", chksum);
>>> -		hdmi_reg_writeb(hdata, HDMI_AUI_CHECK_SUM, chksum);
>>> -		break;
>>> -	default:
>>> -		break;
>>> +	ret = hdmi_audio_infoframe_init(&frm.audio);
>>> +	if (ret >= 0) {
>> Ditto.
>>
>>> +		frm.audio.channels = 2;
>>> +		ret = hdmi_audio_infoframe_pack(&frm.audio, buf, sizeof(buf));
>>> +	}
>>> +	if (ret > 0) {
>>> +		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
>>> +		hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, ret);
>> Also above codes should be called when hdmi_audio_infoframe_init function returned 0 and the value from hdmi_audio_infoframe_pack function is more than 1.
>>
>>>  	}
>>>  }
>>>  
>>> @@ -1127,8 +1077,6 @@ static void hdmi_start(struct hdmi_context *hdata, bool start)
>>>  
>>>  static void hdmi_conf_init(struct hdmi_context *hdata)
>>>  {
>>> -	union hdmi_infoframe infoframe;
>>> -
>>>  	/* disable HPD interrupts from HDMI IP block, use GPIO instead */
>>>  	hdmi_reg_writemask(hdata, HDMI_INTC_CON, 0, HDMI_INTC_EN_GLOBAL |
>>>  		HDMI_INTC_EN_HPD_PLUG | HDMI_INTC_EN_HPD_UNPLUG);
>>> @@ -1164,15 +1112,7 @@ static void hdmi_conf_init(struct hdmi_context *hdata)
>>>  		hdmi_reg_writeb(hdata, HDMI_V13_AUI_CON, 0x02);
>>>  		hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 0x04);
>>>  	} else {
>>> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AVI;
>>> -		infoframe.any.version = HDMI_AVI_VERSION;
>>> -		infoframe.any.length = HDMI_AVI_LENGTH;
>>> -		hdmi_reg_infoframe(hdata, &infoframe);
>>> -
>>> -		infoframe.any.type = HDMI_INFOFRAME_TYPE_AUDIO;
>>> -		infoframe.any.version = HDMI_AUI_VERSION;
>>> -		infoframe.any.length = HDMI_AUI_LENGTH;
>>> -		hdmi_reg_infoframe(hdata, &infoframe);
>>> +		hdmi_reg_infoframes(hdata);
>>>  
>>>  		/* enable AVI packet every vsync, fixes purple line problem */
>>>  		hdmi_reg_writemask(hdata, HDMI_CON_1, 2, 3 << 5);
>>> @@ -1458,7 +1398,6 @@ static void hdmi_mode_set(struct drm_encoder *encoder,
>>>  		"INTERLACED" : "PROGRESSIVE");
>>>  
>>>  	drm_mode_copy(&hdata->current_mode, m);
>>> -	hdata->cea_video_id = drm_match_cea_mode(mode);
>>>  }
>>>  
>>>  static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
>>> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
>>> index 169667a..a0507dc 100644
>>> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
>>> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
>>> @@ -361,9 +361,11 @@
>>>  
>>>  /* AUI bit definition */
>>>  #define HDMI_AUI_CON_NO_TRAN		(0 << 0)
>>> +#define HDMI_AUI_CON_EVERY_VSYNC	(1 << 1)
>>>  
>>>  /* VSI bit definition */
>>>  #define HDMI_VSI_CON_DO_NOT_TRANSMIT	(0 << 0)
>>> +#define HDMI_VSI_CON_EVERY_VSYNC	(1 << 1)
>>>  
>>>  /* HDCP related registers */
>>>  #define HDMI_HDCP_SHA1(n)		HDMI_CORE_BASE(0x7000 + 4 * (n))
>>>
>>
> 
> 
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index e8fb6ef..1bb2df7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -47,19 +47,6 @@ 
 
 #define HOTPLUG_DEBOUNCE_MS		1100
 
-/* AVI header and aspect ratio */
-#define HDMI_AVI_VERSION		0x02
-#define HDMI_AVI_LENGTH			0x0d
-
-/* AUI header info */
-#define HDMI_AUI_VERSION		0x01
-#define HDMI_AUI_LENGTH			0x0a
-
-/* AVI active format aspect ratio */
-#define AVI_SAME_AS_PIC_ASPECT_RATIO	0x08
-#define AVI_4_3_CENTER_RATIO		0x09
-#define AVI_16_9_CENTER_RATIO		0x0a
-
 enum hdmi_type {
 	HDMI_TYPE13,
 	HDMI_TYPE14,
@@ -131,7 +118,6 @@  struct hdmi_context {
 	bool				dvi_mode;
 	struct delayed_work		hotplug_work;
 	struct drm_display_mode		current_mode;
-	u8				cea_video_id;
 	const struct hdmi_driver_data	*drv_data;
 
 	void __iomem			*regs;
@@ -681,6 +667,13 @@  static inline void hdmi_reg_writev(struct hdmi_context *hdata, u32 reg_id,
 	}
 }
 
+static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
+				      u8 *buf, int size)
+{
+	for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
+		writel(*buf++, hdata->regs + reg_id);
+}
+
 static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
 				 u32 reg_id, u32 value, u32 mask)
 {
@@ -762,93 +755,50 @@  static int hdmi_clk_set_parents(struct hdmi_context *hdata, bool to_phy)
 	return ret;
 }
 
-static u8 hdmi_chksum(struct hdmi_context *hdata,
-			u32 start, u8 len, u32 hdr_sum)
-{
-	int i;
-
-	/* hdr_sum : header0 + header1 + header2
-	* start : start address of packet byte1
-	* len : packet bytes - 1 */
-	for (i = 0; i < len; ++i)
-		hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
-
-	/* return 2's complement of 8 bit hdr_sum */
-	return (u8)(~(hdr_sum & 0xff) + 1);
-}
-
-static void hdmi_reg_infoframe(struct hdmi_context *hdata,
-			union hdmi_infoframe *infoframe)
+static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 {
-	u32 hdr_sum;
-	u8 chksum;
-	u8 ar;
+	union hdmi_infoframe frm;
+	u8 buf[25];
+	int ret;
 
 	if (hdata->dvi_mode) {
-		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
-				HDMI_VSI_CON_DO_NOT_TRANSMIT);
 		hdmi_reg_writeb(hdata, HDMI_AVI_CON,
 				HDMI_AVI_CON_DO_NOT_TRANSMIT);
+		hdmi_reg_writeb(hdata, HDMI_VSI_CON,
+				HDMI_VSI_CON_DO_NOT_TRANSMIT);
 		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
 		return;
 	}
 
-	switch (infoframe->any.type) {
-	case HDMI_INFOFRAME_TYPE_AVI:
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
+			&hdata->current_mode);
+	if (ret >= 0)
+		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
+	if (ret > 0) {
 		hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
-		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
-		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
-				infoframe->any.version);
-		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
-		hdr_sum = infoframe->any.type + infoframe->any.version +
-			  infoframe->any.length;
-
-		/* Output format zero hardcoded ,RGB YBCR selection */
-		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
-			AVI_ACTIVE_FORMAT_VALID |
-			AVI_UNDERSCANNED_DISPLAY_VALID);
-
-		/*
-		 * Set the aspect ratio as per the mode, mentioned in
-		 * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
-		 */
-		ar = hdata->current_mode.picture_aspect_ratio;
-		switch (ar) {
-		case HDMI_PICTURE_ASPECT_4_3:
-			ar |= AVI_4_3_CENTER_RATIO;
-			break;
-		case HDMI_PICTURE_ASPECT_16_9:
-			ar |= AVI_16_9_CENTER_RATIO;
-			break;
-		case HDMI_PICTURE_ASPECT_NONE:
-		default:
-			ar |= AVI_SAME_AS_PIC_ASPECT_RATIO;
-			break;
-		}
-		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), ar);
+		hdmi_reg_write_buf(hdata, HDMI_AVI_HEADER0, buf, ret);
+	} else {
+		DRM_INFO("%s: invalid AVI infoframe (%d)\n", __func__, ret);
+	}
 
-		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(4), hdata->cea_video_id);
+	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,
+			&hdata->current_mode);
+	if (ret >= 0)
+		ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
+				sizeof(buf));
+	if (ret > 0) {
+		hdmi_reg_writeb(hdata, HDMI_VSI_CON, HDMI_VSI_CON_EVERY_VSYNC);
+		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, ret);
+	}
 
-		chksum = hdmi_chksum(hdata, HDMI_AVI_BYTE(1),
-					infoframe->any.length, hdr_sum);
-		DRM_DEBUG_KMS("AVI checksum = 0x%x\n", chksum);
-		hdmi_reg_writeb(hdata, HDMI_AVI_CHECK_SUM, chksum);
-		break;
-	case HDMI_INFOFRAME_TYPE_AUDIO:
-		hdmi_reg_writeb(hdata, HDMI_AUI_CON, 0x02);
-		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER0, infoframe->any.type);
-		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER1,
-				infoframe->any.version);
-		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER2, infoframe->any.length);
-		hdr_sum = infoframe->any.type + infoframe->any.version +
-			  infoframe->any.length;
-		chksum = hdmi_chksum(hdata, HDMI_AUI_BYTE(1),
-					infoframe->any.length, hdr_sum);
-		DRM_DEBUG_KMS("AUI checksum = 0x%x\n", chksum);
-		hdmi_reg_writeb(hdata, HDMI_AUI_CHECK_SUM, chksum);
-		break;
-	default:
-		break;
+	ret = hdmi_audio_infoframe_init(&frm.audio);
+	if (ret >= 0) {
+		frm.audio.channels = 2;
+		ret = hdmi_audio_infoframe_pack(&frm.audio, buf, sizeof(buf));
+	}
+	if (ret > 0) {
+		hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_EVERY_VSYNC);
+		hdmi_reg_write_buf(hdata, HDMI_AUI_HEADER0, buf, ret);
 	}
 }
 
@@ -1127,8 +1077,6 @@  static void hdmi_start(struct hdmi_context *hdata, bool start)
 
 static void hdmi_conf_init(struct hdmi_context *hdata)
 {
-	union hdmi_infoframe infoframe;
-
 	/* disable HPD interrupts from HDMI IP block, use GPIO instead */
 	hdmi_reg_writemask(hdata, HDMI_INTC_CON, 0, HDMI_INTC_EN_GLOBAL |
 		HDMI_INTC_EN_HPD_PLUG | HDMI_INTC_EN_HPD_UNPLUG);
@@ -1164,15 +1112,7 @@  static void hdmi_conf_init(struct hdmi_context *hdata)
 		hdmi_reg_writeb(hdata, HDMI_V13_AUI_CON, 0x02);
 		hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 0x04);
 	} else {
-		infoframe.any.type = HDMI_INFOFRAME_TYPE_AVI;
-		infoframe.any.version = HDMI_AVI_VERSION;
-		infoframe.any.length = HDMI_AVI_LENGTH;
-		hdmi_reg_infoframe(hdata, &infoframe);
-
-		infoframe.any.type = HDMI_INFOFRAME_TYPE_AUDIO;
-		infoframe.any.version = HDMI_AUI_VERSION;
-		infoframe.any.length = HDMI_AUI_LENGTH;
-		hdmi_reg_infoframe(hdata, &infoframe);
+		hdmi_reg_infoframes(hdata);
 
 		/* enable AVI packet every vsync, fixes purple line problem */
 		hdmi_reg_writemask(hdata, HDMI_CON_1, 2, 3 << 5);
@@ -1458,7 +1398,6 @@  static void hdmi_mode_set(struct drm_encoder *encoder,
 		"INTERLACED" : "PROGRESSIVE");
 
 	drm_mode_copy(&hdata->current_mode, m);
-	hdata->cea_video_id = drm_match_cea_mode(mode);
 }
 
 static void hdmi_set_refclk(struct hdmi_context *hdata, bool on)
diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
index 169667a..a0507dc 100644
--- a/drivers/gpu/drm/exynos/regs-hdmi.h
+++ b/drivers/gpu/drm/exynos/regs-hdmi.h
@@ -361,9 +361,11 @@ 
 
 /* AUI bit definition */
 #define HDMI_AUI_CON_NO_TRAN		(0 << 0)
+#define HDMI_AUI_CON_EVERY_VSYNC	(1 << 1)
 
 /* VSI bit definition */
 #define HDMI_VSI_CON_DO_NOT_TRANSMIT	(0 << 0)
+#define HDMI_VSI_CON_EVERY_VSYNC	(1 << 1)
 
 /* HDCP related registers */
 #define HDMI_HDCP_SHA1(n)		HDMI_CORE_BASE(0x7000 + 4 * (n))