This set changes the hdmi device tree node to properly represent the version of the IP block. The first patch changes the drm hdmi driver to use the compatible field to determine which registers/functions to call, and removes the unfortunate v1.3/v1.4 naming convention. The second patch updates the arch- specific bits for exynos5.
Changes in v3: - Use compatible field in device tree instead of hdmi-version field - Change hdmi driver's naming to use IP version instead of hdmi version
Sean Paul (2): drm/exynos: Get HDMI version from device tree ARM: Change exynos5-hdmi references to exynos4-hdmi
.../devicetree/bindings/drm/exynos/hdmi.txt | 9 +- arch/arm/boot/dts/exynos5250.dtsi | 3 +- arch/arm/mach-exynos/clock-exynos5.c | 4 +- arch/arm/mach-exynos/mach-exynos5-dt.c | 4 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 354 ++++++++++---------- drivers/gpu/drm/exynos/regs-hdmi.h | 78 +++--- 6 files changed, 229 insertions(+), 223 deletions(-)
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
Signed-off-by: Sean Paul seanpaul@chromium.org --- .../devicetree/bindings/drm/exynos/hdmi.txt | 9 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 354 ++++++++++---------- drivers/gpu/drm/exynos/regs-hdmi.h | 78 +++--- 3 files changed, 223 insertions(+), 218 deletions(-)
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt index 589edee..781b6bc 100644 --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt @@ -1,7 +1,11 @@ Device-Tree bindings for drm hdmi driver
Required properties: -- compatible: value should be "samsung,exynos5-hdmi". +- compatible: suitable values are: + - "samsung,exynos5250-hdmi" + - "samsung,exynos4212-hdmi" + - "samsung,exynos4210-hdmi" + - "samsung,exynos4-hdmi" - reg: physical base address of the hdmi and length of memory mapped region. - interrupts: interrupt number to the cpu. @@ -15,7 +19,8 @@ Required properties: Example:
hdmi { - compatible = "samsung,exynos5-hdmi"; + compatible = "samsung,exynos5250-hdmi", + "samsung,exynos4212-hdmi", "samsung,exynos4-hdmi"; reg = <0x14530000 0x100000>; interrupts = <0 95 0>; hpd-gpio = <&gpx3 7 0xf 1 3>; diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 9e3c2ad..622637f 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -73,9 +73,10 @@ enum HDMI_PACKET_TYPE { HDMI_PACKET_TYPE_AUI = HDMI_PACKET_TYPE_INFOFRAME + 4 };
-enum hdmi_type { - HDMI_TYPE13, - HDMI_TYPE14, +enum hdmi_version { + HDMI_VER_EXYNOS4210 = 1 << 0, + HDMI_VER_EXYNOS4212 = 1 << 1, + HDMI_VER_EXYNOS5250 = 1 << 2, };
struct hdmi_resources { @@ -148,7 +149,7 @@ struct hdmi_core_regs { u8 vact_space_6[2]; };
-struct hdmi_v14_conf { +struct hdmi_4212_conf { int pixel_clock; struct hdmi_core_regs core; struct hdmi_tg_regs tg; @@ -173,52 +174,51 @@ struct hdmi_context {
/* current hdmiphy conf index */ int cur_conf; - struct hdmi_v14_conf mode_conf; + struct hdmi_4212_conf mode_conf;
struct hdmi_resources res;
int hpd_gpio;
- enum hdmi_type type; + int version; };
-/* HDMI Version 1.3 */ -static const u8 hdmiphy_v13_conf27[32] = { +static const u8 hdmiphy_4210_conf27[32] = { 0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30, 0x40, 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, };
-static const u8 hdmiphy_v13_conf27_027[32] = { +static const u8 hdmiphy_4210_conf27_027[32] = { 0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, 0x64, 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, };
-static const u8 hdmiphy_v13_conf74_175[32] = { +static const u8 hdmiphy_4210_conf74_175[32] = { 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, 0x5B, 0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, 0xb9, 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, 0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, 0x00, };
-static const u8 hdmiphy_v13_conf74_25[32] = { +static const u8 hdmiphy_4210_conf74_25[32] = { 0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, 0x40, 0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, 0xba, 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xe0, 0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, 0x00, };
-static const u8 hdmiphy_v13_conf148_5[32] = { +static const u8 hdmiphy_4210_conf148_5[32] = { 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, 0x40, 0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, 0xba, 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xE0, 0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, 0x00, };
-struct hdmi_v13_tg_regs { +struct hdmi_4210_tg_regs { u8 cmd; u8 h_fsz_l; u8 h_fsz_h; @@ -250,7 +250,7 @@ struct hdmi_v13_tg_regs { u8 field_bot_hdmi_h; };
-struct hdmi_v13_core_regs { +struct hdmi_4210_core_regs { u8 h_blank[2]; u8 v_blank[3]; u8 h_v_line[3]; @@ -263,22 +263,22 @@ struct hdmi_v13_core_regs { u8 v_sync_gen3[3]; };
-struct hdmi_v13_preset_conf { - struct hdmi_v13_core_regs core; - struct hdmi_v13_tg_regs tg; +struct hdmi_4210_preset_conf { + struct hdmi_4210_core_regs core; + struct hdmi_4210_tg_regs tg; };
-struct hdmi_v13_conf { +struct hdmi_4210_conf { int width; int height; int vrefresh; bool interlace; int cea_video_id; const u8 *hdmiphy_data; - const struct hdmi_v13_preset_conf *conf; + const struct hdmi_4210_preset_conf *conf; };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_480p = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_480p = { .core = { .h_blank = {0x8a, 0x00}, .v_blank = {0x0d, 0x6a, 0x01}, @@ -304,7 +304,7 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_480p = { }, };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_720p60 = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_720p60 = { .core = { .h_blank = {0x72, 0x01}, .v_blank = {0xee, 0xf2, 0x00}, @@ -332,7 +332,7 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_720p60 = { }, };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080i50 = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_1080i50 = { .core = { .h_blank = {0xd0, 0x02}, .v_blank = {0x32, 0xB2, 0x00}, @@ -360,7 +360,7 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080i50 = { }, };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080p50 = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_1080p50 = { .core = { .h_blank = {0xd0, 0x02}, .v_blank = {0x65, 0x6c, 0x01}, @@ -388,7 +388,7 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080p50 = { }, };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080i60 = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_1080i60 = { .core = { .h_blank = {0x18, 0x01}, .v_blank = {0x32, 0xB2, 0x00}, @@ -416,7 +416,7 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080i60 = { }, };
-static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080p60 = { +static const struct hdmi_4210_preset_conf hdmi_4210_conf_1080p60 = { .core = { .h_blank = {0x18, 0x01}, .v_blank = {0x65, 0x6c, 0x01}, @@ -444,31 +444,31 @@ static const struct hdmi_v13_preset_conf hdmi_v13_conf_1080p60 = { }, };
-static const struct hdmi_v13_conf hdmi_v13_confs[] = { - { 1280, 720, 60, false, 4, hdmiphy_v13_conf74_25, - &hdmi_v13_conf_720p60 }, - { 1280, 720, 50, false, 19, hdmiphy_v13_conf74_25, - &hdmi_v13_conf_720p60 }, - { 720, 480, 60, false, 3, hdmiphy_v13_conf27_027, - &hdmi_v13_conf_480p }, - { 1920, 1080, 50, true, 20, hdmiphy_v13_conf74_25, - &hdmi_v13_conf_1080i50 }, - { 1920, 1080, 50, false, 31, hdmiphy_v13_conf148_5, - &hdmi_v13_conf_1080p50 }, - { 1920, 1080, 60, true, 5, hdmiphy_v13_conf74_25, - &hdmi_v13_conf_1080i60 }, - { 1920, 1080, 60, false, 16, hdmiphy_v13_conf148_5, - &hdmi_v13_conf_1080p60 }, +static const struct hdmi_4210_conf hdmi_4210_confs[] = { + { 1280, 720, 60, false, 4, hdmiphy_4210_conf74_25, + &hdmi_4210_conf_720p60 }, + { 1280, 720, 50, false, 19, hdmiphy_4210_conf74_25, + &hdmi_4210_conf_720p60 }, + { 720, 480, 60, false, 3, hdmiphy_4210_conf27_027, + &hdmi_4210_conf_480p }, + { 1920, 1080, 50, true, 20, hdmiphy_4210_conf74_25, + &hdmi_4210_conf_1080i50 }, + { 1920, 1080, 50, false, 31, hdmiphy_4210_conf148_5, + &hdmi_4210_conf_1080p50 }, + { 1920, 1080, 60, true, 5, hdmiphy_4210_conf74_25, + &hdmi_4210_conf_1080i60 }, + { 1920, 1080, 60, false, 16, hdmiphy_4210_conf148_5, + &hdmi_4210_conf_1080p60 }, };
-/* HDMI Version 1.4 */ +/* For exynos4212-hdmi */ struct hdmiphy_config { int pixel_clock; u8 conf[32]; };
/* list of all required phy config settings */ -static const struct hdmiphy_config hdmiphy_v14_configs[] = { +static const struct hdmiphy_config hdmiphy_4212_configs[] = { { .pixel_clock = 25200000, .conf = { @@ -613,7 +613,7 @@ static inline void hdmi_reg_writemask(struct hdmi_context *hdata, writel(value, hdata->regs + reg_id); }
-static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix) +static void hdmi_4210_regs_dump(struct hdmi_context *hdata, char *prefix) { #define DUMPREG(reg_id) \ DRM_DEBUG_KMS("%s:" #reg_id " = %08x\n", prefix, \ @@ -622,50 +622,50 @@ static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix) DUMPREG(HDMI_INTC_FLAG); DUMPREG(HDMI_INTC_CON); DUMPREG(HDMI_HPD_STATUS); - DUMPREG(HDMI_V13_PHY_RSTOUT); - DUMPREG(HDMI_V13_PHY_VPLL); - DUMPREG(HDMI_V13_PHY_CMU); - DUMPREG(HDMI_V13_CORE_RSTOUT); + DUMPREG(HDMI_4210_PHY_RSTOUT); + DUMPREG(HDMI_4210_PHY_VPLL); + DUMPREG(HDMI_4210_PHY_CMU); + DUMPREG(HDMI_4210_CORE_RSTOUT);
DRM_DEBUG_KMS("%s: ---- CORE REGISTERS ----\n", prefix); DUMPREG(HDMI_CON_0); DUMPREG(HDMI_CON_1); DUMPREG(HDMI_CON_2); DUMPREG(HDMI_SYS_STATUS); - DUMPREG(HDMI_V13_PHY_STATUS); + DUMPREG(HDMI_4210_PHY_STATUS); DUMPREG(HDMI_STATUS_EN); DUMPREG(HDMI_HPD); DUMPREG(HDMI_MODE_SEL); - DUMPREG(HDMI_V13_HPD_GEN); - DUMPREG(HDMI_V13_DC_CONTROL); - DUMPREG(HDMI_V13_VIDEO_PATTERN_GEN); + DUMPREG(HDMI_4210_HPD_GEN); + DUMPREG(HDMI_4210_DC_CONTROL); + DUMPREG(HDMI_4210_VIDEO_PATTERN_GEN);
DRM_DEBUG_KMS("%s: ---- CORE SYNC REGISTERS ----\n", prefix); DUMPREG(HDMI_H_BLANK_0); DUMPREG(HDMI_H_BLANK_1); - DUMPREG(HDMI_V13_V_BLANK_0); - DUMPREG(HDMI_V13_V_BLANK_1); - DUMPREG(HDMI_V13_V_BLANK_2); - DUMPREG(HDMI_V13_H_V_LINE_0); - DUMPREG(HDMI_V13_H_V_LINE_1); - DUMPREG(HDMI_V13_H_V_LINE_2); + DUMPREG(HDMI_4210_V_BLANK_0); + DUMPREG(HDMI_4210_V_BLANK_1); + DUMPREG(HDMI_4210_V_BLANK_2); + DUMPREG(HDMI_4210_H_V_LINE_0); + DUMPREG(HDMI_4210_H_V_LINE_1); + DUMPREG(HDMI_4210_H_V_LINE_2); DUMPREG(HDMI_VSYNC_POL); DUMPREG(HDMI_INT_PRO_MODE); - DUMPREG(HDMI_V13_V_BLANK_F_0); - DUMPREG(HDMI_V13_V_BLANK_F_1); - DUMPREG(HDMI_V13_V_BLANK_F_2); - DUMPREG(HDMI_V13_H_SYNC_GEN_0); - DUMPREG(HDMI_V13_H_SYNC_GEN_1); - DUMPREG(HDMI_V13_H_SYNC_GEN_2); - DUMPREG(HDMI_V13_V_SYNC_GEN_1_0); - DUMPREG(HDMI_V13_V_SYNC_GEN_1_1); - DUMPREG(HDMI_V13_V_SYNC_GEN_1_2); - DUMPREG(HDMI_V13_V_SYNC_GEN_2_0); - DUMPREG(HDMI_V13_V_SYNC_GEN_2_1); - DUMPREG(HDMI_V13_V_SYNC_GEN_2_2); - DUMPREG(HDMI_V13_V_SYNC_GEN_3_0); - DUMPREG(HDMI_V13_V_SYNC_GEN_3_1); - DUMPREG(HDMI_V13_V_SYNC_GEN_3_2); + DUMPREG(HDMI_4210_V_BLANK_F_0); + DUMPREG(HDMI_4210_V_BLANK_F_1); + DUMPREG(HDMI_4210_V_BLANK_F_2); + DUMPREG(HDMI_4210_H_SYNC_GEN_0); + DUMPREG(HDMI_4210_H_SYNC_GEN_1); + DUMPREG(HDMI_4210_H_SYNC_GEN_2); + DUMPREG(HDMI_4210_V_SYNC_GEN_1_0); + DUMPREG(HDMI_4210_V_SYNC_GEN_1_1); + DUMPREG(HDMI_4210_V_SYNC_GEN_1_2); + DUMPREG(HDMI_4210_V_SYNC_GEN_2_0); + DUMPREG(HDMI_4210_V_SYNC_GEN_2_1); + DUMPREG(HDMI_4210_V_SYNC_GEN_2_2); + DUMPREG(HDMI_4210_V_SYNC_GEN_3_0); + DUMPREG(HDMI_4210_V_SYNC_GEN_3_1); + DUMPREG(HDMI_4210_V_SYNC_GEN_3_2);
DRM_DEBUG_KMS("%s: ---- TG REGISTERS ----\n", prefix); DUMPREG(HDMI_TG_CMD); @@ -700,7 +700,7 @@ static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix) #undef DUMPREG }
-static void hdmi_v14_regs_dump(struct hdmi_context *hdata, char *prefix) +static void hdmi_4212_regs_dump(struct hdmi_context *hdata, char *prefix) { int i;
@@ -869,21 +869,21 @@ static void hdmi_v14_regs_dump(struct hdmi_context *hdata, char *prefix)
static void hdmi_regs_dump(struct hdmi_context *hdata, char *prefix) { - if (hdata->type == HDMI_TYPE13) - hdmi_v13_regs_dump(hdata, prefix); + if (hdata->version & HDMI_VER_EXYNOS4210) + hdmi_4210_regs_dump(hdata, prefix); else - hdmi_v14_regs_dump(hdata, prefix); + hdmi_4212_regs_dump(hdata, prefix); }
-static int hdmi_v13_conf_index(struct drm_display_mode *mode) +static int hdmi_4210_conf_index(struct drm_display_mode *mode) { int i;
- for (i = 0; i < ARRAY_SIZE(hdmi_v13_confs); ++i) - if (hdmi_v13_confs[i].width == mode->hdisplay && - hdmi_v13_confs[i].height == mode->vdisplay && - hdmi_v13_confs[i].vrefresh == mode->vrefresh && - hdmi_v13_confs[i].interlace == + for (i = 0; i < ARRAY_SIZE(hdmi_4210_confs); ++i) + if (hdmi_4210_confs[i].width == mode->hdisplay && + hdmi_4210_confs[i].height == mode->vdisplay && + hdmi_4210_confs[i].vrefresh == mode->vrefresh && + hdmi_4210_confs[i].interlace == ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? true : false)) return i; @@ -945,8 +945,8 @@ static void hdmi_reg_infoframe(struct hdmi_context *hdata, hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio | AVI_SAME_AS_PIC_ASPECT_RATIO);
- if (hdata->type == HDMI_TYPE13) - vic = hdmi_v13_confs[hdata->cur_conf].cea_video_id; + if (hdata->version & HDMI_VER_EXYNOS4210) + vic = hdmi_4210_confs[hdata->cur_conf].cea_video_id; else vic = hdata->mode_conf.cea_video_id;
@@ -1002,7 +1002,7 @@ static struct edid *hdmi_get_edid(void *ctx, struct drm_connector *connector) return raw_edid; }
-static int hdmi_v13_check_timing(struct fb_videomode *check_timing) +static int hdmi_4210_check_timing(struct fb_videomode *check_timing) { int i;
@@ -1011,11 +1011,11 @@ static int hdmi_v13_check_timing(struct fb_videomode *check_timing) check_timing->refresh, (check_timing->vmode & FB_VMODE_INTERLACED) ? true : false);
- for (i = 0; i < ARRAY_SIZE(hdmi_v13_confs); ++i) - if (hdmi_v13_confs[i].width == check_timing->xres && - hdmi_v13_confs[i].height == check_timing->yres && - hdmi_v13_confs[i].vrefresh == check_timing->refresh && - hdmi_v13_confs[i].interlace == + for (i = 0; i < ARRAY_SIZE(hdmi_4210_confs); ++i) + if (hdmi_4210_confs[i].width == check_timing->xres && + hdmi_4210_confs[i].height == check_timing->yres && + hdmi_4210_confs[i].vrefresh == check_timing->refresh && + hdmi_4210_confs[i].interlace == ((check_timing->vmode & FB_VMODE_INTERLACED) ? true : false)) return 0; @@ -1025,12 +1025,12 @@ static int hdmi_v13_check_timing(struct fb_videomode *check_timing) return -EINVAL; }
-static int hdmi_v14_find_phy_conf(int pixel_clock) +static int hdmi_4212_find_phy_conf(int pixel_clock) { int i;
- for (i = 0; i < ARRAY_SIZE(hdmiphy_v14_configs); i++) { - if (hdmiphy_v14_configs[i].pixel_clock == pixel_clock) + for (i = 0; i < ARRAY_SIZE(hdmiphy_4212_configs); i++) { + if (hdmiphy_4212_configs[i].pixel_clock == pixel_clock) return i; }
@@ -1038,7 +1038,7 @@ static int hdmi_v14_find_phy_conf(int pixel_clock) return -EINVAL; }
-static int hdmi_v14_check_timing(struct fb_videomode *check_timing) +static int hdmi_4212_check_timing(struct fb_videomode *check_timing) { int i;
@@ -1048,8 +1048,8 @@ static int hdmi_v14_check_timing(struct fb_videomode *check_timing) (check_timing->vmode & FB_VMODE_INTERLACED) ? true : false);
- for (i = 0; i < ARRAY_SIZE(hdmiphy_v14_configs); i++) - if (hdmiphy_v14_configs[i].pixel_clock == + for (i = 0; i < ARRAY_SIZE(hdmiphy_4212_configs); i++) + if (hdmiphy_4212_configs[i].pixel_clock == check_timing->pixclock) return 0;
@@ -1066,10 +1066,10 @@ static int hdmi_check_timing(void *ctx, struct fb_videomode *timing) timing->yres, timing->refresh, timing->vmode);
- if (hdata->type == HDMI_TYPE13) - return hdmi_v13_check_timing(timing); + if (hdata->version & HDMI_VER_EXYNOS4210) + return hdmi_4210_check_timing(timing); else - return hdmi_v14_check_timing(timing); + return hdmi_4212_check_timing(timing); }
static void hdmi_set_acr(u32 freq, u8 *acr) @@ -1132,8 +1132,8 @@ static void hdmi_reg_acr(struct hdmi_context *hdata, u8 *acr) hdmi_reg_writeb(hdata, HDMI_ACR_CTS1, acr[2]); hdmi_reg_writeb(hdata, HDMI_ACR_CTS2, acr[1]);
- if (hdata->type == HDMI_TYPE13) - hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 4); + if (hdata->version & HDMI_VER_EXYNOS4210) + hdmi_reg_writeb(hdata, HDMI_4210_ACR_CON, 4); else hdmi_reg_writeb(hdata, HDMI_ACR_CON, 4); } @@ -1236,8 +1236,8 @@ static void hdmi_conf_reset(struct hdmi_context *hdata) { u32 reg;
- if (hdata->type == HDMI_TYPE13) - reg = HDMI_V13_CORE_RSTOUT; + if (hdata->version & HDMI_VER_EXYNOS4210) + reg = HDMI_4210_CORE_RSTOUT; else reg = HDMI_CORE_RSTOUT;
@@ -1270,21 +1270,21 @@ static void hdmi_conf_init(struct hdmi_context *hdata) HDMI_VID_PREAMBLE_DIS | HDMI_GUARD_BAND_DIS); }
- if (hdata->type == HDMI_TYPE13) { + if (hdata->version & HDMI_VER_EXYNOS4210) { /* choose bluescreen (fecal) color */ - hdmi_reg_writeb(hdata, HDMI_V13_BLUE_SCREEN_0, 0x12); - hdmi_reg_writeb(hdata, HDMI_V13_BLUE_SCREEN_1, 0x34); - hdmi_reg_writeb(hdata, HDMI_V13_BLUE_SCREEN_2, 0x56); + hdmi_reg_writeb(hdata, HDMI_4210_BLUE_SCREEN_0, 0x12); + hdmi_reg_writeb(hdata, HDMI_4210_BLUE_SCREEN_1, 0x34); + hdmi_reg_writeb(hdata, HDMI_4210_BLUE_SCREEN_2, 0x56);
/* enable AVI packet every vsync, fixes purple line problem */ - hdmi_reg_writeb(hdata, HDMI_V13_AVI_CON, 0x02); + hdmi_reg_writeb(hdata, HDMI_4210_AVI_CON, 0x02); /* force RGB, look to CEA-861-D, table 7 for more detail */ - hdmi_reg_writeb(hdata, HDMI_V13_AVI_BYTE(0), 0 << 5); + hdmi_reg_writeb(hdata, HDMI_4210_AVI_BYTE(0), 0 << 5); hdmi_reg_writemask(hdata, HDMI_CON_1, 0x10 << 5, 0x11 << 5);
- hdmi_reg_writeb(hdata, HDMI_V13_SPD_CON, 0x02); - hdmi_reg_writeb(hdata, HDMI_V13_AUI_CON, 0x02); - hdmi_reg_writeb(hdata, HDMI_V13_ACR_CON, 0x04); + hdmi_reg_writeb(hdata, HDMI_4210_SPD_CON, 0x02); + hdmi_reg_writeb(hdata, HDMI_4210_AUI_CON, 0x02); + hdmi_reg_writeb(hdata, HDMI_4210_ACR_CON, 0x04); } else { infoframe.type = HDMI_PACKET_TYPE_AVI; infoframe.ver = HDMI_AVI_VERSION; @@ -1301,40 +1301,40 @@ static void hdmi_conf_init(struct hdmi_context *hdata) } }
-static void hdmi_v13_timing_apply(struct hdmi_context *hdata) +static void hdmi_4210_timing_apply(struct hdmi_context *hdata) { - const struct hdmi_v13_preset_conf *conf = - hdmi_v13_confs[hdata->cur_conf].conf; - const struct hdmi_v13_core_regs *core = &conf->core; - const struct hdmi_v13_tg_regs *tg = &conf->tg; + const struct hdmi_4210_preset_conf *conf = + hdmi_4210_confs[hdata->cur_conf].conf; + const struct hdmi_4210_core_regs *core = &conf->core; + const struct hdmi_4210_tg_regs *tg = &conf->tg; int tries;
/* setting core registers */ hdmi_reg_writeb(hdata, HDMI_H_BLANK_0, core->h_blank[0]); hdmi_reg_writeb(hdata, HDMI_H_BLANK_1, core->h_blank[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_0, core->v_blank[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_1, core->v_blank[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_2, core->v_blank[2]); - hdmi_reg_writeb(hdata, HDMI_V13_H_V_LINE_0, core->h_v_line[0]); - hdmi_reg_writeb(hdata, HDMI_V13_H_V_LINE_1, core->h_v_line[1]); - hdmi_reg_writeb(hdata, HDMI_V13_H_V_LINE_2, core->h_v_line[2]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_0, core->v_blank[0]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_1, core->v_blank[1]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_2, core->v_blank[2]); + hdmi_reg_writeb(hdata, HDMI_4210_H_V_LINE_0, core->h_v_line[0]); + hdmi_reg_writeb(hdata, HDMI_4210_H_V_LINE_1, core->h_v_line[1]); + hdmi_reg_writeb(hdata, HDMI_4210_H_V_LINE_2, core->h_v_line[2]); hdmi_reg_writeb(hdata, HDMI_VSYNC_POL, core->vsync_pol[0]); hdmi_reg_writeb(hdata, HDMI_INT_PRO_MODE, core->int_pro_mode[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_F_0, core->v_blank_f[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_F_1, core->v_blank_f[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_BLANK_F_2, core->v_blank_f[2]); - hdmi_reg_writeb(hdata, HDMI_V13_H_SYNC_GEN_0, core->h_sync_gen[0]); - hdmi_reg_writeb(hdata, HDMI_V13_H_SYNC_GEN_1, core->h_sync_gen[1]); - hdmi_reg_writeb(hdata, HDMI_V13_H_SYNC_GEN_2, core->h_sync_gen[2]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_1_0, core->v_sync_gen1[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_1_1, core->v_sync_gen1[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_1_2, core->v_sync_gen1[2]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_2_0, core->v_sync_gen2[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_2_1, core->v_sync_gen2[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_2_2, core->v_sync_gen2[2]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_3_0, core->v_sync_gen3[0]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_3_1, core->v_sync_gen3[1]); - hdmi_reg_writeb(hdata, HDMI_V13_V_SYNC_GEN_3_2, core->v_sync_gen3[2]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_F_0, core->v_blank_f[0]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_F_1, core->v_blank_f[1]); + hdmi_reg_writeb(hdata, HDMI_4210_V_BLANK_F_2, core->v_blank_f[2]); + hdmi_reg_writeb(hdata, HDMI_4210_H_SYNC_GEN_0, core->h_sync_gen[0]); + hdmi_reg_writeb(hdata, HDMI_4210_H_SYNC_GEN_1, core->h_sync_gen[1]); + hdmi_reg_writeb(hdata, HDMI_4210_H_SYNC_GEN_2, core->h_sync_gen[2]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_1_0, core->v_sync_gen1[0]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_1_1, core->v_sync_gen1[1]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_1_2, core->v_sync_gen1[2]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_2_0, core->v_sync_gen2[0]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_2_1, core->v_sync_gen2[1]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_2_2, core->v_sync_gen2[2]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_3_0, core->v_sync_gen3[0]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_3_1, core->v_sync_gen3[1]); + hdmi_reg_writeb(hdata, HDMI_4210_V_SYNC_GEN_3_2, core->v_sync_gen3[2]); /* Timing generator registers */ hdmi_reg_writeb(hdata, HDMI_TG_H_FSZ_L, tg->h_fsz_l); hdmi_reg_writeb(hdata, HDMI_TG_H_FSZ_H, tg->h_fsz_h); @@ -1367,7 +1367,7 @@ static void hdmi_v13_timing_apply(struct hdmi_context *hdata)
/* waiting for HDMIPHY's PLL to get to steady state */ for (tries = 100; tries; --tries) { - u32 val = hdmi_reg_read(hdata, HDMI_V13_PHY_STATUS); + u32 val = hdmi_reg_read(hdata, HDMI_4210_PHY_STATUS); if (val & HDMI_PHY_STATUS_READY) break; usleep_range(1000, 2000); @@ -1391,7 +1391,7 @@ static void hdmi_v13_timing_apply(struct hdmi_context *hdata) hdmi_reg_writemask(hdata, HDMI_TG_CMD, ~0, HDMI_TG_EN); }
-static void hdmi_v14_timing_apply(struct hdmi_context *hdata) +static void hdmi_4212_timing_apply(struct hdmi_context *hdata) { struct hdmi_core_regs *core = &hdata->mode_conf.core; struct hdmi_tg_regs *tg = &hdata->mode_conf.tg; @@ -1559,10 +1559,10 @@ static void hdmi_v14_timing_apply(struct hdmi_context *hdata)
static void hdmi_timing_apply(struct hdmi_context *hdata) { - if (hdata->type == HDMI_TYPE13) - hdmi_v13_timing_apply(hdata); + if (hdata->version & HDMI_VER_EXYNOS4210) + hdmi_4210_timing_apply(hdata); else - hdmi_v14_timing_apply(hdata); + hdmi_4212_timing_apply(hdata); }
static void hdmiphy_conf_reset(struct hdmi_context *hdata) @@ -1581,8 +1581,8 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata) if (hdata->hdmiphy_port) i2c_master_send(hdata->hdmiphy_port, buffer, 2);
- if (hdata->type == HDMI_TYPE13) - reg = HDMI_V13_PHY_RSTOUT; + if (hdata->version & HDMI_VER_EXYNOS4210) + reg = HDMI_4210_PHY_RSTOUT; else reg = HDMI_PHY_RSTOUT;
@@ -1597,7 +1597,7 @@ static void hdmiphy_poweron(struct hdmi_context *hdata) { DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
- if (hdata->type == HDMI_TYPE14) + if (hdata->version & HDMI_VER_EXYNOS4212) hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0, HDMI_PHY_POWER_OFF_EN); } @@ -1606,7 +1606,7 @@ static void hdmiphy_poweroff(struct hdmi_context *hdata) { DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
- if (hdata->type == HDMI_TYPE14) + if (hdata->version & HDMI_VER_EXYNOS4212) hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0, HDMI_PHY_POWER_OFF_EN); } @@ -1626,16 +1626,16 @@ static void hdmiphy_conf_apply(struct hdmi_context *hdata) }
/* pixel clock */ - if (hdata->type == HDMI_TYPE13) { - hdmiphy_data = hdmi_v13_confs[hdata->cur_conf].hdmiphy_data; + if (hdata->version & HDMI_VER_EXYNOS4210) { + hdmiphy_data = hdmi_4210_confs[hdata->cur_conf].hdmiphy_data; } else { - i = hdmi_v14_find_phy_conf(hdata->mode_conf.pixel_clock); + i = hdmi_4212_find_phy_conf(hdata->mode_conf.pixel_clock); if (i < 0) { DRM_ERROR("failed to find hdmiphy conf\n"); return; }
- hdmiphy_data = hdmiphy_v14_configs[i].conf; + hdmiphy_data = hdmiphy_4212_configs[i].conf; }
memcpy(buffer, hdmiphy_data, 32); @@ -1701,10 +1701,10 @@ static void hdmi_mode_fixup(void *ctx, struct drm_connector *connector,
drm_mode_set_crtcinfo(adjusted_mode, 0);
- if (hdata->type == HDMI_TYPE13) - index = hdmi_v13_conf_index(adjusted_mode); + if (hdata->version & HDMI_VER_EXYNOS4210) + index = hdmi_4210_conf_index(adjusted_mode); else - index = hdmi_v14_find_phy_conf(adjusted_mode->clock * 1000); + index = hdmi_4212_find_phy_conf(adjusted_mode->clock * 1000);
/* just return if user desired mode exists. */ if (index >= 0) @@ -1715,10 +1715,10 @@ static void hdmi_mode_fixup(void *ctx, struct drm_connector *connector, * to adjusted_mode. */ list_for_each_entry(m, &connector->modes, head) { - if (hdata->type == HDMI_TYPE13) - index = hdmi_v13_conf_index(m); + if (hdata->version & HDMI_VER_EXYNOS4210) + index = hdmi_4210_conf_index(m); else - index = hdmi_v14_find_phy_conf(m->clock * 1000); + index = hdmi_4212_find_phy_conf(m->clock * 1000);
if (index >= 0) { struct drm_mode_object base; @@ -1749,7 +1749,7 @@ static void hdmi_set_reg(u8 *reg_pair, int num_bytes, u32 value) reg_pair[i] = (value >> (8 * i)) & 0xff; }
-static void hdmi_v14_mode_set(struct hdmi_context *hdata, +static void hdmi_4212_mode_set(struct hdmi_context *hdata, struct drm_display_mode *m) { struct hdmi_core_regs *core = &hdata->mode_conf.core; @@ -1864,14 +1864,14 @@ static void hdmi_mode_set(void *ctx, void *mode)
DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
- if (hdata->type == HDMI_TYPE13) { - conf_idx = hdmi_v13_conf_index(mode); + if (hdata->version & HDMI_VER_EXYNOS4210) { + conf_idx = hdmi_4210_conf_index(mode); if (conf_idx >= 0) hdata->cur_conf = conf_idx; else DRM_DEBUG_KMS("not supported mode\n"); } else { - hdmi_v14_mode_set(hdata, mode); + hdmi_4212_mode_set(hdata, mode); } }
@@ -2145,16 +2145,16 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata static struct platform_device_id hdmi_driver_types[] = { { .name = "s5pv210-hdmi", - .driver_data = HDMI_TYPE13, + .driver_data = HDMI_VER_EXYNOS4210, }, { .name = "exynos4-hdmi", - .driver_data = HDMI_TYPE13, + .driver_data = HDMI_VER_EXYNOS4210, }, { .name = "exynos4-hdmi14", - .driver_data = HDMI_TYPE14, + .driver_data = HDMI_VER_EXYNOS4212, }, { .name = "exynos5-hdmi", - .driver_data = HDMI_TYPE14, + .driver_data = HDMI_VER_EXYNOS4212 | HDMI_VER_EXYNOS5250, }, { /* end node */ } @@ -2163,8 +2163,7 @@ static struct platform_device_id hdmi_driver_types[] = { #ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { { - .compatible = "samsung,exynos5-hdmi", - .data = (void *)HDMI_TYPE14, + .compatible = "samsung,exynos4-hdmi", }, { /* end node */ } @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, drm_hdmi_ctx);
- if (dev->of_node) { - const struct of_device_id *match; - match = of_match_node(of_match_ptr(hdmi_match_types), - pdev->dev.of_node); - if (match == NULL) - return -ENODEV; - hdata->type = (enum hdmi_type)match->data; - } else { - hdata->type = (enum hdmi_type)platform_get_device_id - (pdev)->driver_data; + hdata->version = 0; + if (!dev->of_node) { + hdata->version = (enum hdmi_version) + platform_get_device_id(pdev)->driver_data; } + + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi")) + hdata->version |= HDMI_VER_EXYNOS4210; + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi")) + hdata->version |= HDMI_VER_EXYNOS4212; + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi")) + hdata->version |= HDMI_VER_EXYNOS5250;
hdata->hpd_gpio = pdata->hpd_gpio; hdata->dev = dev; diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h b/drivers/gpu/drm/exynos/regs-hdmi.h index ef1b3eb..5b14fa3 100644 --- a/drivers/gpu/drm/exynos/regs-hdmi.h +++ b/drivers/gpu/drm/exynos/regs-hdmi.h @@ -19,7 +19,7 @@ * Register part */
-/* HDMI Version 1.3 & Common */ +/* Base addresses */ #define HDMI_CTRL_BASE(x) ((x) + 0x00000000) #define HDMI_CORE_BASE(x) ((x) + 0x00010000) #define HDMI_I2S_BASE(x) ((x) + 0x00040000) @@ -29,57 +29,57 @@ #define HDMI_INTC_CON HDMI_CTRL_BASE(0x0000) #define HDMI_INTC_FLAG HDMI_CTRL_BASE(0x0004) #define HDMI_HPD_STATUS HDMI_CTRL_BASE(0x000C) -#define HDMI_V13_PHY_RSTOUT HDMI_CTRL_BASE(0x0014) -#define HDMI_V13_PHY_VPLL HDMI_CTRL_BASE(0x0018) -#define HDMI_V13_PHY_CMU HDMI_CTRL_BASE(0x001C) -#define HDMI_V13_CORE_RSTOUT HDMI_CTRL_BASE(0x0020) +#define HDMI_4210_PHY_RSTOUT HDMI_CTRL_BASE(0x0014) +#define HDMI_4210_PHY_VPLL HDMI_CTRL_BASE(0x0018) +#define HDMI_4210_PHY_CMU HDMI_CTRL_BASE(0x001C) +#define HDMI_4210_CORE_RSTOUT HDMI_CTRL_BASE(0x0020)
/* Core registers */ #define HDMI_CON_0 HDMI_CORE_BASE(0x0000) #define HDMI_CON_1 HDMI_CORE_BASE(0x0004) #define HDMI_CON_2 HDMI_CORE_BASE(0x0008) #define HDMI_SYS_STATUS HDMI_CORE_BASE(0x0010) -#define HDMI_V13_PHY_STATUS HDMI_CORE_BASE(0x0014) +#define HDMI_4210_PHY_STATUS HDMI_CORE_BASE(0x0014) #define HDMI_STATUS_EN HDMI_CORE_BASE(0x0020) #define HDMI_HPD HDMI_CORE_BASE(0x0030) #define HDMI_MODE_SEL HDMI_CORE_BASE(0x0040) #define HDMI_ENC_EN HDMI_CORE_BASE(0x0044) -#define HDMI_V13_BLUE_SCREEN_0 HDMI_CORE_BASE(0x0050) -#define HDMI_V13_BLUE_SCREEN_1 HDMI_CORE_BASE(0x0054) -#define HDMI_V13_BLUE_SCREEN_2 HDMI_CORE_BASE(0x0058) +#define HDMI_4210_BLUE_SCREEN_0 HDMI_CORE_BASE(0x0050) +#define HDMI_4210_BLUE_SCREEN_1 HDMI_CORE_BASE(0x0054) +#define HDMI_4210_BLUE_SCREEN_2 HDMI_CORE_BASE(0x0058) #define HDMI_H_BLANK_0 HDMI_CORE_BASE(0x00A0) #define HDMI_H_BLANK_1 HDMI_CORE_BASE(0x00A4) -#define HDMI_V13_V_BLANK_0 HDMI_CORE_BASE(0x00B0) -#define HDMI_V13_V_BLANK_1 HDMI_CORE_BASE(0x00B4) -#define HDMI_V13_V_BLANK_2 HDMI_CORE_BASE(0x00B8) -#define HDMI_V13_H_V_LINE_0 HDMI_CORE_BASE(0x00C0) -#define HDMI_V13_H_V_LINE_1 HDMI_CORE_BASE(0x00C4) -#define HDMI_V13_H_V_LINE_2 HDMI_CORE_BASE(0x00C8) +#define HDMI_4210_V_BLANK_0 HDMI_CORE_BASE(0x00B0) +#define HDMI_4210_V_BLANK_1 HDMI_CORE_BASE(0x00B4) +#define HDMI_4210_V_BLANK_2 HDMI_CORE_BASE(0x00B8) +#define HDMI_4210_H_V_LINE_0 HDMI_CORE_BASE(0x00C0) +#define HDMI_4210_H_V_LINE_1 HDMI_CORE_BASE(0x00C4) +#define HDMI_4210_H_V_LINE_2 HDMI_CORE_BASE(0x00C8) #define HDMI_VSYNC_POL HDMI_CORE_BASE(0x00E4) #define HDMI_INT_PRO_MODE HDMI_CORE_BASE(0x00E8) -#define HDMI_V13_V_BLANK_F_0 HDMI_CORE_BASE(0x0110) -#define HDMI_V13_V_BLANK_F_1 HDMI_CORE_BASE(0x0114) -#define HDMI_V13_V_BLANK_F_2 HDMI_CORE_BASE(0x0118) -#define HDMI_V13_H_SYNC_GEN_0 HDMI_CORE_BASE(0x0120) -#define HDMI_V13_H_SYNC_GEN_1 HDMI_CORE_BASE(0x0124) -#define HDMI_V13_H_SYNC_GEN_2 HDMI_CORE_BASE(0x0128) -#define HDMI_V13_V_SYNC_GEN_1_0 HDMI_CORE_BASE(0x0130) -#define HDMI_V13_V_SYNC_GEN_1_1 HDMI_CORE_BASE(0x0134) -#define HDMI_V13_V_SYNC_GEN_1_2 HDMI_CORE_BASE(0x0138) -#define HDMI_V13_V_SYNC_GEN_2_0 HDMI_CORE_BASE(0x0140) -#define HDMI_V13_V_SYNC_GEN_2_1 HDMI_CORE_BASE(0x0144) -#define HDMI_V13_V_SYNC_GEN_2_2 HDMI_CORE_BASE(0x0148) -#define HDMI_V13_V_SYNC_GEN_3_0 HDMI_CORE_BASE(0x0150) -#define HDMI_V13_V_SYNC_GEN_3_1 HDMI_CORE_BASE(0x0154) -#define HDMI_V13_V_SYNC_GEN_3_2 HDMI_CORE_BASE(0x0158) -#define HDMI_V13_ACR_CON HDMI_CORE_BASE(0x0180) -#define HDMI_V13_AVI_CON HDMI_CORE_BASE(0x0300) -#define HDMI_V13_AVI_BYTE(n) HDMI_CORE_BASE(0x0320 + 4 * (n)) -#define HDMI_V13_DC_CONTROL HDMI_CORE_BASE(0x05C0) -#define HDMI_V13_VIDEO_PATTERN_GEN HDMI_CORE_BASE(0x05C4) -#define HDMI_V13_HPD_GEN HDMI_CORE_BASE(0x05C8) -#define HDMI_V13_AUI_CON HDMI_CORE_BASE(0x0360) -#define HDMI_V13_SPD_CON HDMI_CORE_BASE(0x0400) +#define HDMI_4210_V_BLANK_F_0 HDMI_CORE_BASE(0x0110) +#define HDMI_4210_V_BLANK_F_1 HDMI_CORE_BASE(0x0114) +#define HDMI_4210_V_BLANK_F_2 HDMI_CORE_BASE(0x0118) +#define HDMI_4210_H_SYNC_GEN_0 HDMI_CORE_BASE(0x0120) +#define HDMI_4210_H_SYNC_GEN_1 HDMI_CORE_BASE(0x0124) +#define HDMI_4210_H_SYNC_GEN_2 HDMI_CORE_BASE(0x0128) +#define HDMI_4210_V_SYNC_GEN_1_0 HDMI_CORE_BASE(0x0130) +#define HDMI_4210_V_SYNC_GEN_1_1 HDMI_CORE_BASE(0x0134) +#define HDMI_4210_V_SYNC_GEN_1_2 HDMI_CORE_BASE(0x0138) +#define HDMI_4210_V_SYNC_GEN_2_0 HDMI_CORE_BASE(0x0140) +#define HDMI_4210_V_SYNC_GEN_2_1 HDMI_CORE_BASE(0x0144) +#define HDMI_4210_V_SYNC_GEN_2_2 HDMI_CORE_BASE(0x0148) +#define HDMI_4210_V_SYNC_GEN_3_0 HDMI_CORE_BASE(0x0150) +#define HDMI_4210_V_SYNC_GEN_3_1 HDMI_CORE_BASE(0x0154) +#define HDMI_4210_V_SYNC_GEN_3_2 HDMI_CORE_BASE(0x0158) +#define HDMI_4210_ACR_CON HDMI_CORE_BASE(0x0180) +#define HDMI_4210_AVI_CON HDMI_CORE_BASE(0x0300) +#define HDMI_4210_AVI_BYTE(n) HDMI_CORE_BASE(0x0320 + 4 * (n)) +#define HDMI_4210_DC_CONTROL HDMI_CORE_BASE(0x05C0) +#define HDMI_4210_VIDEO_PATTERN_GEN HDMI_CORE_BASE(0x05C4) +#define HDMI_4210_HPD_GEN HDMI_CORE_BASE(0x05C8) +#define HDMI_4210_AUI_CON HDMI_CORE_BASE(0x0360) +#define HDMI_4210_SPD_CON HDMI_CORE_BASE(0x0400)
/* Timing generator registers */ #define HDMI_TG_CMD HDMI_TG_BASE(0x0000) @@ -155,7 +155,7 @@ #define HDMI_FIELD_EN (1 << 1)
-/* HDMI Version 1.4 */ +/* Exynos 4212 */ /* Control registers */ /* #define HDMI_INTC_CON HDMI_CTRL_BASE(0x0000) */ /* #define HDMI_INTC_FLAG HDMI_CTRL_BASE(0x0004) */
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi",
Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
- if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))
hdata->version |= HDMI_VER_EXYNOS4210;- if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))
hdata->version |= HDMI_VER_EXYNOS4212;- if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))
hdata->version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;
That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
Sean
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
On Wed, Feb 6, 2013 at 9:42 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
I would like to distinguish between 4210 and 4x12. since it has different features. e.g., HDMI v1.3 and v1.4. and I also want to use 4412 instead of 4212. there's no board to use 4212 at mainline until this time.
Thank you, Kyungmin Park
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
All devices that use this driver are compatible, at some level, with exynos4-hdmi, so I think its usage is correct here.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5250-hdmi", .data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212); }, { .compatible = "samsung,exynos4212-hdmi", .data = (void *)HDMI_VER_EXYNOS4212; }, { .compatible = "samsung,exynos4210-hdmi", .data = (void *)HDMI_VER_EXYNOS4210; }, { /* end node */ } };
In that case, it eliminates the benefit of using device tree to determine the compatible bits. I hope I'm just being thick and missing something.
Sean
On 02/06/2013 09:56 AM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version. diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c #ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
All devices that use this driver are compatible, at some level, with exynos4-hdmi, so I think its usage is correct here.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;
But this way can make unnecessary combinations, e.g. exynos4210-hdmi + exynos5250-hdmi.
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5250-hdmi", .data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212); }, { .compatible = "samsung,exynos4212-hdmi", .data = (void *)HDMI_VER_EXYNOS4212; }, { .compatible = "samsung,exynos4210-hdmi", .data = (void *)HDMI_VER_EXYNOS4210; }, { /* end node */ } };
I think this makes driver more clearly. We just see device tables and we can know device uses which version.
In that case, it eliminates the benefit of using device tree to determine the compatible bits. I hope I'm just being thick and missing something.
Sean _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 02/05/2013 05:56 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
All devices that use this driver are compatible, at some level, with exynos4-hdmi, so I think its usage is correct here.
But can a driver that only knows about the original exynos4-hdmi operate any of the HW correctly without any additional knowledge? If not, the new HW isn't compatible with the old.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5250-hdmi", .data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212); }, { .compatible = "samsung,exynos4212-hdmi", .data = (void *)HDMI_VER_EXYNOS4212; }, { .compatible = "samsung,exynos4210-hdmi", .data = (void *)HDMI_VER_EXYNOS4210; }, { /* end node */ } };
In that case, it eliminates the benefit of using device tree to determine the compatible bits. I hope I'm just being thick and missing something.
The table above looks /almost/ exactly correct to me, although I'm unsure why samsung,exynos5250-hdmi has *two* version values; surely there's a 1:1 mapping between the compatible values and the HW compatibility they represent? That's certainly the intent.
Perhaps the "two values" think is because you're representing quirks or features rather than HW versions? Compatible is supposed to represent HW versions. Each HW version has a set of features/quirks, and multiple HW versions can have intersecting sets of features/quirks. However, features/quirks aren't HW versions.
On 2013년 02월 06일 09:56, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
All devices that use this driver are compatible, at some level, with exynos4-hdmi, so I think its usage is correct here.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5250-hdmi", .data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212); }, { .compatible = "samsung,exynos4212-hdmi", .data = (void *)HDMI_VER_EXYNOS4212; }, { .compatible = "samsung,exynos4210-hdmi", .data = (void *)HDMI_VER_EXYNOS4210; }, { /* end node */ } };
Actually, I can't understand why there is HDMI_VER_EXYNOS5250 because it is not used anywhere in your patch. I'm not sure I missed something from your v2 patch thread, but to me, just hdmi version or hdmi ip version can be used as data field of struct of_device_id as like your v2 patch set. and then of_match_device() can be used without | in data field.
If I have missed some point from v2 thread, please let me know.
Best Regards, - Seung-Woo Kim
In that case, it eliminates the benefit of using device tree to determine the compatible bits. I hope I'm just being thick and missing something.
Sean _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Feb 5, 2013 at 6:56 PM, 김승우 sw0312.kim@samsung.com wrote:
On 2013년 02월 06일 09:56, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which registers/functions to use in the HDMI driver. Also changes the references from v13 to 4210 and v14 to 4212 to reflect the IP block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF static struct of_device_id hdmi_match_types[] = { {
.compatible = "samsung,exynos5-hdmi",.data = (void *)HDMI_TYPE14,
.compatible = "samsung,exynos4-hdmi", }, { /* end node */ }Why not fill in all the "base" compatible values there (I think you need this anyway so that DTs don't all have to be compatible with samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS* values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since it provides the base for the current driver. The driver uses 4210 and 4212 to differentiate between different register addresses and features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is actually compatible with. If the HW isn't compatible with exynos4-hdmi, that value shouldn't be in the compatible property, but instead whatever the "base" value that the HW really is compatible with. The driver can support multiple "base" compatible values from this table.
All devices that use this driver are compatible, at some level, with exynos4-hdmi, so I think its usage is correct here.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))hdata->version |= HDMI_VER_EXYNOS4210;if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))hdata->version |= HDMI_VER_EXYNOS4212;if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))hdata->version |= HDMI_VER_EXYNOS5250;Instead of that, do roughly:
match = of_match_device(hdmi_match_types, &pdev->dev); if (match) hdata->version |= (int)match->data;That way, it's all table-based. Any future additions to hdmi_match_types[] won't require another if statement to be added to probe().
I don't think it's that easy. of_match_device returns the first match from the device table, so I'd still need to iterate through the matches. I could still break this out into a table, but I don't think of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device() is supposed to return the match for the first entry in the compatible property, then if there was no match, move on to looking at the next entry in the compatible property, etc. In practice, I think it's still not implemented quite correctly for this, but you can make it work by putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5250-hdmi", .data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212); }, { .compatible = "samsung,exynos4212-hdmi", .data = (void *)HDMI_VER_EXYNOS4212; }, { .compatible = "samsung,exynos4210-hdmi", .data = (void *)HDMI_VER_EXYNOS4210; }, { /* end node */ } };
Actually, I can't understand why there is HDMI_VER_EXYNOS5250 because it is not used anywhere in your patch. I'm not sure I missed something from your v2 patch thread, but to me, just hdmi version or hdmi ip version can be used as data field of struct of_device_id as like your v2 patch set. and then of_match_device() can be used without | in data field.
If I have missed some point from v2 thread, please let me know.
Exactly. I think that's causing some of this confusion. It'd be easier to just leave any 5250 reference out of this patch. The _only_ place 5250 should be used now is in the 5250 dtsi file, as the most specific compatible field. Compatible there should be, in order:
5250, 4212, 4
(or maybe not 4 at all, if the driver can't successfully drive the hardware in degraded mode using the HDMI 1.3 register maps, etc).
... But there's no need to reference it in the driver. It might be useful in the future to set some quirk or feature bits, but until then it's just there in case.
-Olof
With the change "drm/exynos: Get HDMI version from device tree", exynos5-hdmi is no longer relevant. Update references to exynos4-hdmi and update the hdmi compatibility string to accurately reflect the hdmi driver.
Signed-off-by: Sean Paul seanpaul@chromium.org --- arch/arm/boot/dts/exynos5250.dtsi | 3 ++- arch/arm/mach-exynos/clock-exynos5.c | 4 ++-- arch/arm/mach-exynos/mach-exynos5-dt.c | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 3acf594..70464bf 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -573,7 +573,8 @@ };
hdmi { - compatible = "samsung,exynos5-hdmi"; + compatible = "samsung,exynos5250-hdmi", + "samsung,exynos4212-hdmi", "samsung,exynos4-hdmi"; reg = <0x14530000 0x70000>; interrupts = <0 95 0>; }; diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-exynos/clock-exynos5.c index e9d7b80..6eae39d 100644 --- a/arch/arm/mach-exynos/clock-exynos5.c +++ b/arch/arm/mach-exynos/clock-exynos5.c @@ -686,12 +686,12 @@ static struct clk exynos5_init_clocks_off[] = { .ctrlbit = (1 << 0), }, { .name = "hdmi", - .devname = "exynos5-hdmi", + .devname = "exynos4-hdmi", .enable = exynos5_clk_ip_disp1_ctrl, .ctrlbit = (1 << 6), }, { .name = "hdmiphy", - .devname = "exynos5-hdmi", + .devname = "exynos4-hdmi", .enable = exynos5_clk_hdmiphy_ctrl, .ctrlbit = (1 << 0), }, { diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c index e99d3d8..0ccda72 100644 --- a/arch/arm/mach-exynos/mach-exynos5-dt.c +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c @@ -97,8 +97,8 @@ static const struct of_dev_auxdata exynos5250_auxdata_lookup[] __initconst = { "exynos-gsc.2", NULL), OF_DEV_AUXDATA("samsung,exynos5-gsc", EXYNOS5_PA_GSC3, "exynos-gsc.3", NULL), - OF_DEV_AUXDATA("samsung,exynos5-hdmi", 0x14530000, - "exynos5-hdmi", NULL), + OF_DEV_AUXDATA("samsung,exynos4-hdmi", 0x14530000, + "exynos4-hdmi", NULL), OF_DEV_AUXDATA("samsung,exynos5-mixer", 0x14450000, "exynos5-mixer", NULL), OF_DEV_AUXDATA("samsung,mfc-v6", 0x11000000, "s5p-mfc-v6", NULL),
dri-devel@lists.freedesktop.org
-
Joonyoung Shim -
Kyungmin Park -
Olof Johansson -
Sean Paul -
Stephen Warren -
김승우