* [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.