* [PATCH] drm/exynos/hdmi: refactor infoframe code
[not found] <CGME20161026123635eucas1p2ddd97eff849b9f633ea2d482fbd82c34@eucas1p2.samsung.com>
@ 2016-10-26 12:36 ` Andrzej Hajda
2016-11-07 1:45 ` Inki Dae
0 siblings, 1 reply; 5+ messages in thread
From: Andrzej Hajda @ 2016-10-26 12:36 UTC (permalink / raw)
To: Inki Dae, dri-devel; +Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
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)
+ 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))
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/exynos/hdmi: refactor infoframe code
2016-10-26 12:36 ` [PATCH] drm/exynos/hdmi: refactor infoframe code Andrzej Hajda
@ 2016-11-07 1:45 ` Inki Dae
2016-11-07 8:05 ` Andrzej Hajda
0 siblings, 1 reply; 5+ messages in thread
From: Inki Dae @ 2016-11-07 1:45 UTC (permalink / raw)
To: Andrzej Hajda, dri-devel; +Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
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))
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/exynos/hdmi: refactor infoframe code
2016-11-07 1:45 ` Inki Dae
@ 2016-11-07 8:05 ` Andrzej Hajda
2016-11-07 8:14 ` Inki Dae
0 siblings, 1 reply; 5+ messages in thread
From: Andrzej Hajda @ 2016-11-07 8:05 UTC (permalink / raw)
To: Inki Dae, dri-devel; +Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
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))
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/exynos/hdmi: refactor infoframe code
2016-11-07 8:05 ` Andrzej Hajda
@ 2016-11-07 8:14 ` Inki Dae
2016-11-07 15:04 ` Andrzej Hajda
0 siblings, 1 reply; 5+ messages in thread
From: Inki Dae @ 2016-11-07 8:14 UTC (permalink / raw)
To: Andrzej Hajda, dri-devel; +Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
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))
>>>
>>
>
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] drm/exynos/hdmi: refactor infoframe code
2016-11-07 8:14 ` Inki Dae
@ 2016-11-07 15:04 ` Andrzej Hajda
0 siblings, 0 replies; 5+ messages in thread
From: Andrzej Hajda @ 2016-11-07 15:04 UTC (permalink / raw)
To: inki.dae, dri-devel; +Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
Use core helpers to generate infoframes and generate vendor frame if necessary.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
- changed 'ret >= 0' checks to '!ret'
---
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..e39d557 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)
+ 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)
+ 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) {
+ 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))
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-07 15:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20161026123635eucas1p2ddd97eff849b9f633ea2d482fbd82c34@eucas1p2.samsung.com>
2016-10-26 12:36 ` [PATCH] drm/exynos/hdmi: refactor infoframe code Andrzej Hajda
2016-11-07 1:45 ` Inki Dae
2016-11-07 8:05 ` Andrzej Hajda
2016-11-07 8:14 ` Inki Dae
2016-11-07 15:04 ` Andrzej Hajda
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.