[PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Rahul Sharma]

This patch set adds provision for composing and sending AVI and AUI
infoframes by exynos drm hdmi driver.

It also adds provision to get CEA Video ID Code through the display mode
which is required for making AVI infoframe.

Based on exynos-drm-fixes branch of
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

Rahul Sharma (1):
  drm: exynos: compose and send avi and aui info frames

Stephane Marchesin (1):
  drm: get cea video id code for a given display mode

 drivers/gpu/drm/drm_edid.c           |   20 +++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
 drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
 include/drm/drm_crtc.h               |    1 +
 5 files changed, 154 insertions(+), 4 deletions(-)

[PATCH 1/2] drm: get cea video id code for a given display mode

[Rahul Sharma]

From: Stephane Marchesin <marcheu@chromium.org>

This patch adds support for getting CEA Video ID Code for a given
display mode after matching with edid_cea_modes list. Its index in
the list added with one, gives the desired code.

This exported function will be used by hdmi drivers for composing
AVI info frame data.

Signed-off-by: Stephane Marchesin <marcheu@chromium.org>
Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 drivers/gpu/drm/drm_edid.c |   20 ++++++++++++++++++++
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fadcd44..856dcd9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1516,6 +1516,26 @@ u8 *drm_find_cea_extension(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_find_cea_extension);
 
+/*
+ * Looks for a CEA mode matching given drm_display_mode.
+ * Returns its CEA Video ID code, or 0 if not found.
+ */
+u8 drm_match_cea_mode(struct drm_display_mode *to_match)
+{
+	struct drm_display_mode *cea_mode;
+	u8 mode;
+
+	for (mode = 0; mode < drm_num_cea_modes; mode++) {
+		cea_mode = (struct drm_display_mode *)&edid_cea_modes[mode];
+
+		if (drm_mode_equal(to_match, cea_mode))
+			return mode + 1;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(drm_match_cea_mode);
+
+
 static int
 do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
 {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 92889be..467a327 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1038,6 +1038,7 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 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(struct drm_display_mode *to_match);
 extern bool drm_detect_hdmi_monitor(struct edid *edid);
 extern bool drm_detect_monitor_audio(struct edid *edid);
 extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
-- 
1.7.0.4

[PATCH 2/2] drm: exynos: compose and send avi and aui info frames

[Rahul Sharma]

This patch adds code for composing AVI and AUI info frames
and send them every VSYNC.

This patch is important for hdmi certification.

Signed-off-by: Fahad Kunnathadi <fahad.k@samsung.com>
Signed-off-by: Shirish S <s.shirish@samsung.com>
Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
 drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
 3 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c115f8..bb8a045 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -82,6 +82,7 @@ struct hdmi_context {
 
 	/* current hdmiphy conf index */
 	int cur_conf;
+	int cur_video_id;
 
 	struct hdmi_resources		res;
 	void				*parent_ctx;
@@ -944,6 +945,11 @@ static const struct hdmi_conf hdmi_confs[] = {
 	{ 1920, 1080, 60, false, hdmiphy_conf148_5, &hdmi_conf_1080p60 },
 };
 
+struct hdmi_infoframe {
+	enum HDMI_PACKET_TYPE type;
+	u8 ver;
+	u8 len;
+};
 
 static inline u32 hdmi_reg_read(struct hdmi_context *hdata, u32 reg_id)
 {
@@ -1267,6 +1273,81 @@ static int hdmi_conf_index(struct hdmi_context *hdata,
 	return hdmi_v14_conf_index(mode);
 }
 
+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 (u8)(0x100 - (hdr_sum & 0xff));
+}
+
+void hdmi_reg_infoframe(struct hdmi_context *hdata,
+			struct hdmi_infoframe *infoframe)
+{
+	u32 hdr_sum;
+	u8 chksum;
+	u32 aspect_ratio;
+	u32 mod;
+
+	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+
+	mod = hdmi_reg_read(hdata, HDMI_MODE_SEL);
+	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_AUI_CON, HDMI_AUI_CON_NO_TRAN);
+		return;
+	}
+
+	switch (infoframe->type) {
+
+	case HDMI_PACKET_TYPE_AVI:
+		hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
+		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->type);
+		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1, infoframe->ver);
+		hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->len);
+		hdr_sum = infoframe->type + infoframe->ver + infoframe->len;
+		/* 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);
+
+		aspect_ratio = AVI_PIC_ASPECT_RATIO_16_9;
+
+		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio |
+				AVI_SAME_AS_PIC_ASPECT_RATIO);
+		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(4), hdata->cur_video_id);
+
+		chksum = hdmi_chksum(hdata, HDMI_AVI_BYTE(1),
+					infoframe->len, hdr_sum);
+		DRM_DEBUG_KMS("AVI checksum = 0x%x\n", chksum);
+		hdmi_reg_writeb(hdata, HDMI_AVI_CHECK_SUM, chksum);
+		break;
+
+	case HDMI_PACKET_TYPE_AUI:
+		hdmi_reg_writeb(hdata, HDMI_AUI_CON, 0x02);
+		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER0, infoframe->type);
+		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER1, infoframe->ver);
+		hdmi_reg_writeb(hdata, HDMI_AUI_HEADER2, infoframe->len);
+		hdr_sum = infoframe->type + infoframe->ver + infoframe->len;
+		chksum = hdmi_chksum(hdata, HDMI_AUI_BYTE(1),
+					infoframe->len, hdr_sum);
+		DRM_DEBUG_KMS("AUI checksum = 0x%x\n", chksum);
+		hdmi_reg_writeb(hdata, HDMI_AUI_CHECK_SUM, chksum);
+		break;
+
+	default:
+		break;
+	}
+}
+
 static bool hdmi_is_connected(void *ctx)
 {
 	struct hdmi_context *hdata = ctx;
@@ -1541,6 +1622,8 @@ static void hdmi_conf_reset(struct hdmi_context *hdata)
 
 static void hdmi_conf_init(struct hdmi_context *hdata)
 {
+	struct hdmi_infoframe infoframe;
+
 	/* disable HPD interrupts */
 	hdmi_reg_writemask(hdata, HDMI_INTC_CON, 0, HDMI_INTC_EN_GLOBAL |
 		HDMI_INTC_EN_HPD_PLUG | HDMI_INTC_EN_HPD_UNPLUG);
@@ -1575,9 +1658,17 @@ 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.type = HDMI_PACKET_TYPE_AVI;
+		infoframe.ver = HDMI_AVI_VERSION;
+		infoframe.len = HDMI_AVI_LENGTH;
+		hdmi_reg_infoframe(hdata, &infoframe);
+
+		infoframe.type = HDMI_PACKET_TYPE_AUI;
+		infoframe.ver = HDMI_AUI_VERSION;
+		infoframe.len = HDMI_AUI_LENGTH;
+		hdmi_reg_infoframe(hdata, &infoframe);
+
 		/* enable AVI packet every vsync, fixes purple line problem */
-		hdmi_reg_writeb(hdata, HDMI_AVI_CON, 0x02);
-		hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 2 << 5);
 		hdmi_reg_writemask(hdata, HDMI_CON_1, 2, 3 << 5);
 	}
 }
@@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
 
 	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
+	hdata->cur_video_id = drm_match_cea_mode(mode);
+
 	conf_idx = hdmi_conf_index(hdata, mode);
 	if (conf_idx >= 0)
 		hdata->cur_conf = conf_idx;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.h b/drivers/gpu/drm/exynos/exynos_hdmi.h
index 1c3b6d8..fc4de49 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.h
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.h
@@ -28,6 +28,29 @@
 #ifndef _EXYNOS_HDMI_H_
 #define _EXYNOS_HDMI_H_
 
+/* AVI header and aspect ratio */
+#define HDMI_AVI_VERSION		0x02
+#define HDMI_AVI_LENGTH		0x0d
+#define AVI_PIC_ASPECT_RATIO_16_9	(2 << 4)
+#define AVI_SAME_AS_PIC_ASPECT_RATIO	8
+
+/* AUI header info */
+#define HDMI_AUI_VERSION		0x01
+#define HDMI_AUI_LENGTH		0x0a
+
+/* HDMI infoframe to configure HDMI out packet header, AUI and AVI */
+enum HDMI_PACKET_TYPE {
+	/** refer to Table 5-8 Packet Type in HDMI specification v1.4a */
+	/** InfoFrame packet type */
+	HDMI_PACKET_TYPE_INFOFRAME = 0X80,
+	/** Vendor-Specific InfoFrame */
+	HDMI_PACKET_TYPE_VSI = HDMI_PACKET_TYPE_INFOFRAME + 1,
+	/** Auxiliary Video information InfoFrame */
+	HDMI_PACKET_TYPE_AVI = HDMI_PACKET_TYPE_INFOFRAME + 2,
+	/** Audio information InfoFrame */
+	HDMI_PACKET_TYPE_AUI = HDMI_PACKET_TYPE_INFOFRAME + 4
+};
+
 void hdmi_attach_ddc_client(struct i2c_client *ddc);
 void hdmi_attach_hdmiphy_client(struct i2c_client *hdmiphy);
 
diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h
index 9cc7c5e..970cdb5 100644
--- a/drivers/gpu/drm/exynos/regs-hdmi.h
+++ b/drivers/gpu/drm/exynos/regs-hdmi.h
@@ -298,14 +298,14 @@
 #define HDMI_AVI_HEADER1		HDMI_CORE_BASE(0x0714)
 #define HDMI_AVI_HEADER2		HDMI_CORE_BASE(0x0718)
 #define HDMI_AVI_CHECK_SUM		HDMI_CORE_BASE(0x071C)
-#define HDMI_AVI_BYTE(n)		HDMI_CORE_BASE(0x0720 + 4 * (n))
+#define HDMI_AVI_BYTE(n)		HDMI_CORE_BASE(0x0720 + 4 * (n-1))
 
 #define HDMI_AUI_CON			HDMI_CORE_BASE(0x0800)
 #define HDMI_AUI_HEADER0		HDMI_CORE_BASE(0x0810)
 #define HDMI_AUI_HEADER1		HDMI_CORE_BASE(0x0814)
 #define HDMI_AUI_HEADER2		HDMI_CORE_BASE(0x0818)
 #define HDMI_AUI_CHECK_SUM		HDMI_CORE_BASE(0x081C)
-#define HDMI_AUI_BYTE(n)		HDMI_CORE_BASE(0x0820 + 4 * (n))
+#define HDMI_AUI_BYTE(n)		HDMI_CORE_BASE(0x0820 + 4 * (n-1))
 
 #define HDMI_MPG_CON			HDMI_CORE_BASE(0x0900)
 #define HDMI_MPG_CHECK_SUM		HDMI_CORE_BASE(0x091C)
@@ -338,6 +338,19 @@
 #define HDMI_AN_SEED_2			HDMI_CORE_BASE(0x0E60)
 #define HDMI_AN_SEED_3			HDMI_CORE_BASE(0x0E64)
 
+/* AVI bit definition */
+#define HDMI_AVI_CON_DO_NOT_TRANSMIT	(0 << 1)
+#define HDMI_AVI_CON_EVERY_VSYNC	(1 << 1)
+
+#define AVI_ACTIVE_FORMAT_VALID	(1 << 4)
+#define AVI_UNDERSCANNED_DISPLAY_VALID	(1 << 1)
+
+/* AUI bit definition */
+#define HDMI_AUI_CON_NO_TRAN		(0 << 0)
+
+/* VSI bit definition */
+#define HDMI_VSI_CON_DO_NOT_TRANSMIT	(0 << 0)
+
 /* HDCP related registers */
 #define HDMI_HDCP_SHA1(n)		HDMI_CORE_BASE(0x7000 + 4 * (n))
 #define HDMI_HDCP_KSV_LIST(n)		HDMI_CORE_BASE(0x7050 + 4 * (n))
-- 
1.7.0.4

Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Rahul Sharma]

Hi All,

Kindly review the following patch-set.

regards,
Rahul Sharma

On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
> This patch set adds provision for composing and sending AVI and AUI
> infoframes by exynos drm hdmi driver.
>
> It also adds provision to get CEA Video ID Code through the display mode
> which is required for making AVI infoframe.
>
> Based on exynos-drm-fixes branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>
> Rahul Sharma (1):
>   drm: exynos: compose and send avi and aui info frames
>
> Stephane Marchesin (1):
>   drm: get cea video id code for a given display mode
>
>  drivers/gpu/drm/drm_edid.c           |   20 +++++++
>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>  include/drm/drm_crtc.h               |    1 +
>  5 files changed, 154 insertions(+), 4 deletions(-)
>

Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Kyungmin Park]

Hi,

To get the proper review, please add proper maintainers.

Thank you,
Kyungmin Park

On Tue, Nov 20, 2012 at 8:55 PM, Rahul Sharma <r.sh.open@gmail.com> wrote:
> Hi All,
>
> Kindly review the following patch-set.
>
> regards,
> Rahul Sharma
>
> On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>> This patch set adds provision for composing and sending AVI and AUI
>> infoframes by exynos drm hdmi driver.
>>
>> It also adds provision to get CEA Video ID Code through the display mode
>> which is required for making AVI infoframe.
>>
>> Based on exynos-drm-fixes branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>>
>> Rahul Sharma (1):
>>   drm: exynos: compose and send avi and aui info frames
>>
>> Stephane Marchesin (1):
>>   drm: get cea video id code for a given display mode
>>
>>  drivers/gpu/drm/drm_edid.c           |   20 +++++++
>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>>  include/drm/drm_crtc.h               |    1 +
>>  5 files changed, 154 insertions(+), 4 deletions(-)
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: exynos: compose and send avi and aui info frames

[김승우]

Hi Rahul,

Control part seems good, and my comment is below.

On 2012년 11월 10일 01:21, Rahul Sharma wrote:
> This patch adds code for composing AVI and AUI info frames
> and send them every VSYNC.
> 
> This patch is important for hdmi certification.
> 
> Signed-off-by: Fahad Kunnathadi <fahad.k@samsung.com>
> Signed-off-by: Shirish S <s.shirish@samsung.com>
> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>  3 files changed, 133 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2c115f8..bb8a045 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c

<snip>

> @@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
>  
>  	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>  
> +	hdata->cur_video_id = drm_match_cea_mode(mode);
> +

How do you think about using predefined cea video id in struct
hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
compares only mode information. Considering this, IMHO, cea video id can
be embedded in struct hdmi_conf.

>  	conf_idx = hdmi_conf_index(hdata, mode);
>  	if (conf_idx >= 0)
>  		hdata->cur_conf = conf_idx;

<snip>

Thanks and Regards,
- Seung-Woo Kim

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

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

Re: [PATCH 2/2] drm: exynos: compose and send avi and aui info frames

[Rahul Sharma]

Hi Seung Woo,

Thanks for your inputs. Please find my response below.

On Wed, Nov 21, 2012 at 2:12 PM, 김승우 <sw0312.kim@samsung.com> wrote:
> Hi Rahul,
>
> Control part seems good, and my comment is below.
>
> On 2012년 11월 10일 01:21, Rahul Sharma wrote:
>> This patch adds code for composing AVI and AUI info frames
>> and send them every VSYNC.
>>
>> This patch is important for hdmi certification.
>>
>> Signed-off-by: Fahad Kunnathadi <fahad.k@samsung.com>
>> Signed-off-by: Shirish S <s.shirish@samsung.com>
>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>>  3 files changed, 133 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 2c115f8..bb8a045 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>
> <snip>
>
>> @@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
>>
>>       DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>
>> +     hdata->cur_video_id = drm_match_cea_mode(mode);
>> +
>
> How do you think about using predefined cea video id in struct
> hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
> compares only mode information. Considering this, IMHO, cea video id can
> be embedded in struct hdmi_conf.
>

I feel, It will leads to duplication of video id information. In
edid_cea_modes, modes are
strictly arranged in the order of respective cea video ID codes.
"drm_add_edid_modes"
also passes the cea codes (recieved after edid data parsing) as the index to
edid_cea_modes to get mode details.

Secondly, mode to cea code translation is required by all platforms
for AVI packet
composition. By adding it to hdmi_conf, we are limiting its usage for exynos.

regards,
Rahul Sharma.

>>       conf_idx = hdmi_conf_index(hdata, mode);
>>       if (conf_idx >= 0)
>>               hdata->cur_conf = conf_idx;
>
> <snip>
>
> Thanks and Regards,
> - Seung-Woo Kim
>
> --
> Seung-Woo Kim
> Samsung Software R&D Center
> --
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: exynos: compose and send avi and aui info frames

[김승우]

On 2012년 11월 21일 20:36, Rahul Sharma wrote:
> Hi Seung Woo,
> 
> Thanks for your inputs. Please find my response below.
> 
> On Wed, Nov 21, 2012 at 2:12 PM, 김승우 <sw0312.kim@samsung.com> wrote:
>> Hi Rahul,
>>
>> Control part seems good, and my comment is below.
>>
>> On 2012년 11월 10일 01:21, Rahul Sharma wrote:
>>> This patch adds code for composing AVI and AUI info frames
>>> and send them every VSYNC.
>>>
>>> This patch is important for hdmi certification.
>>>
>>> Signed-off-by: Fahad Kunnathadi <fahad.k@samsung.com>
>>> Signed-off-by: Shirish S <s.shirish@samsung.com>
>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>>>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>>>  3 files changed, 133 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index 2c115f8..bb8a045 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>
>> <snip>
>>
>>> @@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
>>>
>>>       DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>>
>>> +     hdata->cur_video_id = drm_match_cea_mode(mode);
>>> +
>>
>> How do you think about using predefined cea video id in struct
>> hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
>> compares only mode information. Considering this, IMHO, cea video id can
>> be embedded in struct hdmi_conf.
>>
> 
> I feel, It will leads to duplication of video id information. In
> edid_cea_modes, modes are
> strictly arranged in the order of respective cea video ID codes.
> "drm_add_edid_modes"
> also passes the cea codes (recieved after edid data parsing) as the index to
> edid_cea_modes to get mode details.

It might be a concern related with your first patch, anyway
edid_cea_modes has few pair of exact same modes because struct drm_mode
does not have picture ratio. For example, video id 2 and 3 have exact
same values for struct drm_mode. So cea video id can be used to get a
mode, but a drm_mode is not sufficient to get exact video id.
Considering that exynos hdmi does not support video ids with same mode,
I suggested video id in struct hdmi_conf.
At the point of exynos drm, I can ack this patch.

> 
> Secondly, mode to cea code translation is required by all platforms
> for AVI packet
> composition. By adding it to hdmi_conf, we are limiting its usage for exynos.

I agree with you at this point. I quickly checked i915 and radeon and I
found that they use fixed value for avi packet at sw level, but I don't
have information hw can properly build avi packet. If they also need
video id for building avi packet, video id translation can be used.

Best Regards,
- Seung-Woo Kim

> 
> regards,
> Rahul Sharma.
> 
>>>       conf_idx = hdmi_conf_index(hdata, mode);
>>>       if (conf_idx >= 0)
>>>               hdata->cur_conf = conf_idx;
>>
>> <snip>
>>
>> Thanks and Regards,
>> - Seung-Woo Kim
>>
>> --
>> Seung-Woo Kim
>> Samsung Software R&D Center
>> --
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: exynos: compose and send avi and aui info frames

[Rahul Sharma]

On Thu, Nov 22, 2012 at 12:06 PM, 김승우 <sw0312.kim@samsung.com> wrote:
> On 2012년 11월 21일 20:36, Rahul Sharma wrote:
>> Hi Seung Woo,
>>
>> Thanks for your inputs. Please find my response below.
>>
>> On Wed, Nov 21, 2012 at 2:12 PM, 김승우 <sw0312.kim@samsung.com> wrote:
>>> Hi Rahul,
>>>
>>> Control part seems good, and my comment is below.
>>>
>>> On 2012년 11월 10일 01:21, Rahul Sharma wrote:
>>>> This patch adds code for composing AVI and AUI info frames
>>>> and send them every VSYNC.
>>>>
>>>> This patch is important for hdmi certification.
>>>>
>>>> Signed-off-by: Fahad Kunnathadi <fahad.k@samsung.com>
>>>> Signed-off-by: Shirish S <s.shirish@samsung.com>
>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97 +++++++++++++++++++++++++++++++++-
>>>>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>>>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>>>>  3 files changed, 133 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>> index 2c115f8..bb8a045 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>
>>> <snip>
>>>
>>>> @@ -1993,6 +2084,8 @@ static void hdmi_mode_set(void *ctx, void *mode)
>>>>
>>>>       DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>>>
>>>> +     hdata->cur_video_id = drm_match_cea_mode(mode);
>>>> +
>>>
>>> How do you think about using predefined cea video id in struct
>>> hdmi_conf? drm_mode does not have cea video id, so drm_match_cea_mode()
>>> compares only mode information. Considering this, IMHO, cea video id can
>>> be embedded in struct hdmi_conf.
>>>
>>
>> I feel, It will leads to duplication of video id information. In
>> edid_cea_modes, modes are
>> strictly arranged in the order of respective cea video ID codes.
>> "drm_add_edid_modes"
>> also passes the cea codes (recieved after edid data parsing) as the index to
>> edid_cea_modes to get mode details.
>
> It might be a concern related with your first patch, anyway
> edid_cea_modes has few pair of exact same modes because struct drm_mode
> does not have picture ratio. For example, video id 2 and 3 have exact
> same values for struct drm_mode. So cea video id can be used to get a
> mode, but a drm_mode is not sufficient to get exact video id.
> Considering that exynos hdmi does not support video ids with same mode,
> I suggested video id in struct hdmi_conf.
> At the point of exynos drm, I can ack this patch.

You are right. This ambiguity is still present about the video code when
drm framework sets the mode to hdmi. hdmi_check_timing also doesn't care
about picture aspect ratio. I am not sure how to get exact vic from the mode.

I have submitted  another patch that where vic is provided the hdmi_conf.
I preferred 16:9 aspect ratio. Kindly review that.

regards,
Rahul Sharma

>
>>
>> Secondly, mode to cea code translation is required by all platforms
>> for AVI packet
>> composition. By adding it to hdmi_conf, we are limiting its usage for exynos.
>
> I agree with you at this point. I quickly checked i915 and radeon and I
> found that they use fixed value for avi packet at sw level, but I don't
> have information hw can properly build avi packet. If they also need
> video id for building avi packet, video id translation can be used.
>
> Best Regards,
> - Seung-Woo Kim
>
>>
>> regards,
>> Rahul Sharma.
>>
>>>>       conf_idx = hdmi_conf_index(hdata, mode);
>>>>       if (conf_idx >= 0)
>>>>               hdata->cur_conf = conf_idx;
>>>
>>> <snip>
>>>
>>> Thanks and Regards,
>>> - Seung-Woo Kim
>>>
>>> --
>>> Seung-Woo Kim
>>> Samsung Software R&D Center
>>> --
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Thierry Reding]

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

On Fri, Nov 09, 2012 at 09:51:04PM +0530, Rahul Sharma wrote:
> This patch set adds provision for composing and sending AVI and AUI
> infoframes by exynos drm hdmi driver.
> 
> It also adds provision to get CEA Video ID Code through the display mode
> which is required for making AVI infoframe.
> 
> Based on exynos-drm-fixes branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
> 
> Rahul Sharma (1):
>   drm: exynos: compose and send avi and aui info frames
> 
> Stephane Marchesin (1):
>   drm: get cea video id code for a given display mode

Hi Rahul,

I'm currently working on a patchset that tries to unify the infoframe
code that is currently duplicated for each driver. The latest code that
I have adds support for packing AVI, audio and vendor infoframes, so it
should cover all of what you're trying to do here. Furthermore there's a
helper that fill in a basic set of fields in the AVI infoframe from a
given drm_display_mode, based on code very similar to Stephane's patch
from Paulo Zanoni (Cc'ed).

Maybe we can try and make this work for Exynos as well. The code that I
have is tested on NVIDIA Tegra and should be easily portable to i915,
radeon and nouveau as well. I plan to do that as part of the patchset,
so I thought you may be interested in using the generic code as well.
The patches still need some cleanup but I think I can send them out
either later tonight or tomorrow.

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

Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Inki Dae]

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

2012/11/10 Rahul Sharma <rahul.sharma@samsung.com>

> This patch set adds provision for composing and sending AVI and AUI
> infoframes by exynos drm hdmi driver.
>
> It also adds provision to get CEA Video ID Code through the display mode
> which is required for making AVI infoframe.
>
> Based on exynos-drm-fixes branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>
>
Please rebase these patches to be based on exynos-drm-next. These should go
to -next.

Thanks,
Inki Dae



> Rahul Sharma (1):
>   drm: exynos: compose and send avi and aui info frames
>
> Stephane Marchesin (1):
>   drm: get cea video id code for a given display mode
>
>  drivers/gpu/drm/drm_edid.c           |   20 +++++++
>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97
> +++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>  include/drm/drm_crtc.h               |    1 +
>  5 files changed, 154 insertions(+), 4 deletions(-)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1912 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

Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames

[Rahul Sharma]

Thanks Thierry,

It is good idea to use generic code. I would like to see your patches.

regards,
Rahul Sharma

On Fri, Nov 23, 2012 at 11:45 AM, Inki Dae <inki.dae@samsung.com> wrote:
>
>
> 2012/11/10 Rahul Sharma <rahul.sharma@samsung.com>
>>
>> This patch set adds provision for composing and sending AVI and AUI
>> infoframes by exynos drm hdmi driver.
>>
>> It also adds provision to get CEA Video ID Code through the display mode
>> which is required for making AVI infoframe.
>>
>> Based on exynos-drm-fixes branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>>
>
> Please rebase these patches to be based on exynos-drm-next. These should go
> to -next.
>
> Thanks,
> Inki Dae
>
>
>>
>> Rahul Sharma (1):
>>   drm: exynos: compose and send avi and aui info frames
>>
>> Stephane Marchesin (1):
>>   drm: get cea video id code for a given display mode
>>
>>  drivers/gpu/drm/drm_edid.c           |   20 +++++++
>>  drivers/gpu/drm/exynos/exynos_hdmi.c |   97
>> +++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/exynos/exynos_hdmi.h |   23 ++++++++
>>  drivers/gpu/drm/exynos/regs-hdmi.h   |   17 +++++-
>>  include/drm/drm_crtc.h               |    1 +
>>  5 files changed, 154 insertions(+), 4 deletions(-)
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>