Hi,
This is v2 of the series. Changes compared to v1:
Dropped (Jyri will work on them): - drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix - drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes
Added: - drm/omap: avoid copy in mgr_fld_read/write - drm/omap: hdmi4: fix use of uninitialized var
Fixed the issues pointed out by Laurent.
Tomi
Alejandro Hernandez (1): drm/omap: tweak HDMI DDC timings
Jyri Sarha (1): drm/omap: dss: move platform_register_drivers() to dss.c and remove core.c
Tomi Valkeinen (5): drm/omap: drop unneeded locking from mgr_fld_write() drm/omap: avoid copy in mgr_fld_read/write drm/omap: fix missing scaler pixel fmt limitations drm/omap: hdmi5: automatically choose limited/full range output drm/omap: hdmi4: fix use of uninitialized var
drivers/gpu/drm/omapdrm/dss/Makefile | 2 +- drivers/gpu/drm/omapdrm/dss/core.c | 55 ---------- drivers/gpu/drm/omapdrm/dss/dispc.c | 46 ++++++--- drivers/gpu/drm/omapdrm/dss/dss.c | 37 +++++++ drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 5 +- drivers/gpu/drm/omapdrm/dss/hdmi5_core.c | 125 ++++++++++++----------- 6 files changed, 137 insertions(+), 133 deletions(-) delete mode 100644 drivers/gpu/drm/omapdrm/dss/core.c
Commit d49cd15550d9d4495f6187425318c245d58cb63f ("OMAPDSS: DISPC: lock access to DISPC_CONTROL & DISPC_CONFIG") added locking to mgr_fld_write(). This was needed in omapfb times due to lack of good locking, especially in the case of both V4L2 and fbdev layers using the DSS driver.
This is not needed for omapdrm, so we can remove the locking.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/dispc.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 785c5546067a..0dc0272569f6 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -184,9 +184,6 @@ struct dispc_device {
struct regmap *syscon_pol; u32 syscon_pol_offset; - - /* DISPC_CONTROL & DISPC_CONFIG lock*/ - spinlock_t control_lock; };
enum omap_color_component { @@ -377,16 +374,8 @@ static void mgr_fld_write(struct dispc_device *dispc, enum omap_channel channel, enum mgr_reg_fields regfld, int val) { const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld]; - const bool need_lock = rfld.reg == DISPC_CONTROL || rfld.reg == DISPC_CONFIG; - unsigned long flags;
- if (need_lock) { - spin_lock_irqsave(&dispc->control_lock, flags); - REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low); - spin_unlock_irqrestore(&dispc->control_lock, flags); - } else { - REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low); - } + REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low); }
static int dispc_get_num_ovls(struct dispc_device *dispc) @@ -4769,8 +4758,6 @@ static int dispc_bind(struct device *dev, struct device *master, void *data) platform_set_drvdata(pdev, dispc); dispc->dss = dss;
- spin_lock_init(&dispc->control_lock); - /* * The OMAP3-based models can't be told apart using the compatible * string, use SoC device matching.
Avoid unnecessary copy in mgr_fld_read/write by taking a pointer to the reg_resc and using that.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/dss/dispc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 0dc0272569f6..3c9315b17ef2 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -365,17 +365,17 @@ static inline u32 dispc_read_reg(struct dispc_device *dispc, u16 idx) static u32 mgr_fld_read(struct dispc_device *dispc, enum omap_channel channel, enum mgr_reg_fields regfld) { - const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld]; + const struct dispc_reg_field *rfld = &mgr_desc[channel].reg_desc[regfld];
- return REG_GET(dispc, rfld.reg, rfld.high, rfld.low); + return REG_GET(dispc, rfld->reg, rfld->high, rfld->low); }
static void mgr_fld_write(struct dispc_device *dispc, enum omap_channel channel, enum mgr_reg_fields regfld, int val) { - const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld]; + const struct dispc_reg_field *rfld = &mgr_desc[channel].reg_desc[regfld];
- REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low); + REG_FLD_MOD(dispc, rfld->reg, val, rfld->high, rfld->low); }
static int dispc_get_num_ovls(struct dispc_device *dispc)
Hi Tomi,
Thank you for the patch.
On Mon, Sep 30, 2019 at 01:38:35PM +0300, Tomi Valkeinen wrote:
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
From: Alejandro Hernandez ajhernandez@ti.com
A "HDMI I2C Master Error" is sometimes reported with the current DDC SCL timings. The current settings for a 10us SCL period (100 KHz) causes the error with some displays. This patch increases the SCL signal period from 10us to 10.2us, with the new settings the error is not observed
Signed-off-by: Alejandro Hernandez ajhernandez@ti.com Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/hdmi5_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c index 7400fb99d453..4c588ec7634a 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c @@ -39,8 +39,8 @@ static void hdmi_core_ddc_init(struct hdmi_core_data *core) { void __iomem *base = core->base; const unsigned long long iclk = 266000000; /* DSS L3 ICLK */ - const unsigned int ss_scl_high = 4600; /* ns */ - const unsigned int ss_scl_low = 5400; /* ns */ + const unsigned int ss_scl_high = 4700; /* ns */ + const unsigned int ss_scl_low = 5500; /* ns */ const unsigned int fs_scl_high = 600; /* ns */ const unsigned int fs_scl_low = 1300; /* ns */ const unsigned int sda_hold = 1000; /* ns */
OMAP2 and OMAP3/AM4 have limitations with the scaler: - OMAP2 can only scale XRGB8888 - OMAP3/AM4 can only scale XRGB8888, RGB565, YUYV and UYVY
The driver doesn't check these limitations, which leads to sync-lost floods.
This patch adds a check for the pixel formats when scaling.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/dispc.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 3c9315b17ef2..c19e0af33013 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -114,6 +114,7 @@ struct dispc_features { const unsigned int num_reg_fields; const enum omap_overlay_caps *overlay_caps; const u32 **supported_color_modes; + const u32 *supported_scaler_color_modes; unsigned int num_mgrs; unsigned int num_ovls; unsigned int buffer_size_unit; @@ -2499,6 +2500,19 @@ static int dispc_ovl_calc_scaling(struct dispc_device *dispc, if (width == out_width && height == out_height) return 0;
+ if (dispc->feat->supported_scaler_color_modes) { + const u32 *modes = dispc->feat->supported_scaler_color_modes; + unsigned int i; + + for (i = 0; modes[i]; ++i) { + if (modes[i] == fourcc) + break; + } + + if (modes[i] == 0) + return -EINVAL; + } + if (plane == OMAP_DSS_WB) { switch (fourcc) { case DRM_FORMAT_NV12: @@ -4214,6 +4228,12 @@ static const u32 *omap4_dispc_supported_color_modes[] = { DRM_FORMAT_RGBX8888), };
+static const u32 omap3_dispc_supported_scaler_color_modes[] = { + DRM_FORMAT_XRGB8888, DRM_FORMAT_RGB565, DRM_FORMAT_YUYV, + DRM_FORMAT_UYVY, + 0, +}; + static const struct dispc_features omap24xx_dispc_feats = { .sw_start = 5, .fp_start = 15, @@ -4242,6 +4262,7 @@ static const struct dispc_features omap24xx_dispc_feats = { .num_reg_fields = ARRAY_SIZE(omap2_dispc_reg_fields), .overlay_caps = omap2_dispc_overlay_caps, .supported_color_modes = omap2_dispc_supported_color_modes, + .supported_scaler_color_modes = COLOR_ARRAY(DRM_FORMAT_XRGB8888), .num_mgrs = 2, .num_ovls = 3, .buffer_size_unit = 1, @@ -4276,6 +4297,7 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats = { .num_reg_fields = ARRAY_SIZE(omap3_dispc_reg_fields), .overlay_caps = omap3430_dispc_overlay_caps, .supported_color_modes = omap3_dispc_supported_color_modes, + .supported_scaler_color_modes = omap3_dispc_supported_scaler_color_modes, .num_mgrs = 2, .num_ovls = 3, .buffer_size_unit = 1, @@ -4310,6 +4332,7 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats = { .num_reg_fields = ARRAY_SIZE(omap3_dispc_reg_fields), .overlay_caps = omap3430_dispc_overlay_caps, .supported_color_modes = omap3_dispc_supported_color_modes, + .supported_scaler_color_modes = omap3_dispc_supported_scaler_color_modes, .num_mgrs = 2, .num_ovls = 3, .buffer_size_unit = 1, @@ -4344,6 +4367,7 @@ static const struct dispc_features omap36xx_dispc_feats = { .num_reg_fields = ARRAY_SIZE(omap3_dispc_reg_fields), .overlay_caps = omap3630_dispc_overlay_caps, .supported_color_modes = omap3_dispc_supported_color_modes, + .supported_scaler_color_modes = omap3_dispc_supported_scaler_color_modes, .num_mgrs = 2, .num_ovls = 3, .buffer_size_unit = 1, @@ -4378,6 +4402,7 @@ static const struct dispc_features am43xx_dispc_feats = { .num_reg_fields = ARRAY_SIZE(omap3_dispc_reg_fields), .overlay_caps = omap3430_dispc_overlay_caps, .supported_color_modes = omap3_dispc_supported_color_modes, + .supported_scaler_color_modes = omap3_dispc_supported_scaler_color_modes, .num_mgrs = 1, .num_ovls = 3, .buffer_size_unit = 1,
From: Jyri Sarha jsarha@ti.com
The core.c just for registering the drivers is kind of useless. Let's get rid of it and register the dss drivers in dss.c.
Signed-off-by: Jyri Sarha jsarha@ti.com Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/Makefile | 2 +- drivers/gpu/drm/omapdrm/dss/core.c | 55 ---------------------------- drivers/gpu/drm/omapdrm/dss/dss.c | 37 +++++++++++++++++++ 3 files changed, 38 insertions(+), 56 deletions(-) delete mode 100644 drivers/gpu/drm/omapdrm/dss/core.c
diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile index 904101c5e79d..5950c3f52c2e 100644 --- a/drivers/gpu/drm/omapdrm/dss/Makefile +++ b/drivers/gpu/drm/omapdrm/dss/Makefile @@ -6,7 +6,7 @@ omapdss-base-y := base.o display.o dss-of.o output.o
obj-$(CONFIG_OMAP2_DSS) += omapdss.o # Core DSS files -omapdss-y := core.o dss.o dispc.o dispc_coefs.o \ +omapdss-y := dss.o dispc.o dispc_coefs.o \ pll.o video-pll.o omapdss-$(CONFIG_OMAP2_DSS_DPI) += dpi.o omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o diff --git a/drivers/gpu/drm/omapdrm/dss/core.c b/drivers/gpu/drm/omapdrm/dss/core.c deleted file mode 100644 index 6ac497b63711..000000000000 --- a/drivers/gpu/drm/omapdrm/dss/core.c +++ /dev/null @@ -1,55 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Copyright (C) 2009 Nokia Corporation - * Author: Tomi Valkeinen tomi.valkeinen@ti.com - * - * Some code and ideas taken from drivers/video/omap/ driver - * by Imre Deak. - */ - -#define DSS_SUBSYS_NAME "CORE" - -#include <linux/kernel.h> -#include <linux/module.h> -#include <linux/platform_device.h> - -#include "omapdss.h" -#include "dss.h" - -/* INIT */ -static struct platform_driver * const omap_dss_drivers[] = { - &omap_dsshw_driver, - &omap_dispchw_driver, -#ifdef CONFIG_OMAP2_DSS_DSI - &omap_dsihw_driver, -#endif -#ifdef CONFIG_OMAP2_DSS_VENC - &omap_venchw_driver, -#endif -#ifdef CONFIG_OMAP4_DSS_HDMI - &omapdss_hdmi4hw_driver, -#endif -#ifdef CONFIG_OMAP5_DSS_HDMI - &omapdss_hdmi5hw_driver, -#endif -}; - -static int __init omap_dss_init(void) -{ - return platform_register_drivers(omap_dss_drivers, - ARRAY_SIZE(omap_dss_drivers)); -} - -static void __exit omap_dss_exit(void) -{ - platform_unregister_drivers(omap_dss_drivers, - ARRAY_SIZE(omap_dss_drivers)); -} - -module_init(omap_dss_init); -module_exit(omap_dss_exit); - -MODULE_AUTHOR("Tomi Valkeinen tomi.valkeinen@ti.com"); -MODULE_DESCRIPTION("OMAP2/3 Display Subsystem"); -MODULE_LICENSE("GPL v2"); - diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c index e226324adb69..41d495a360d8 100644 --- a/drivers/gpu/drm/omapdrm/dss/dss.c +++ b/drivers/gpu/drm/omapdrm/dss/dss.c @@ -1598,3 +1598,40 @@ struct platform_driver omap_dsshw_driver = { .suppress_bind_attrs = true, }, }; + +/* INIT */ +static struct platform_driver * const omap_dss_drivers[] = { + &omap_dsshw_driver, + &omap_dispchw_driver, +#ifdef CONFIG_OMAP2_DSS_DSI + &omap_dsihw_driver, +#endif +#ifdef CONFIG_OMAP2_DSS_VENC + &omap_venchw_driver, +#endif +#ifdef CONFIG_OMAP4_DSS_HDMI + &omapdss_hdmi4hw_driver, +#endif +#ifdef CONFIG_OMAP5_DSS_HDMI + &omapdss_hdmi5hw_driver, +#endif +}; + +static int __init omap_dss_init(void) +{ + return platform_register_drivers(omap_dss_drivers, + ARRAY_SIZE(omap_dss_drivers)); +} + +static void __exit omap_dss_exit(void) +{ + platform_unregister_drivers(omap_dss_drivers, + ARRAY_SIZE(omap_dss_drivers)); +} + +module_init(omap_dss_init); +module_exit(omap_dss_exit); + +MODULE_AUTHOR("Tomi Valkeinen tomi.valkeinen@ti.com"); +MODULE_DESCRIPTION("OMAP2/3/4/5 Display Subsystem"); +MODULE_LICENSE("GPL v2");
Currently the HDMI driver uses always limited range RGB output. This patch improves the behavior by using limited range only if the output is identified as a HDMI display, and VIC > 1.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/hdmi5_core.c | 121 ++++++++++++----------- 1 file changed, 65 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c index 4c588ec7634a..96f5cd17768c 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c @@ -23,18 +23,6 @@
#include "hdmi5_core.h"
-/* only 24 bit color depth used for now */ -static const struct csc_table csc_table_deepcolor[] = { - /* HDMI_DEEP_COLOR_24BIT */ - [0] = { 7036, 0, 0, 32, 0, 7036, 0, 32, 0, 0, 7036, 32, }, - /* HDMI_DEEP_COLOR_30BIT */ - [1] = { 7015, 0, 0, 128, 0, 7015, 0, 128, 0, 0, 7015, 128, }, - /* HDMI_DEEP_COLOR_36BIT */ - [2] = { 7010, 0, 0, 512, 0, 7010, 0, 512, 0, 0, 7010, 512, }, - /* FULL RANGE */ - [3] = { 8192, 0, 0, 0, 0, 8192, 0, 0, 0, 0, 8192, 0, }, -}; - static void hdmi_core_ddc_init(struct hdmi_core_data *core) { void __iomem *base = core->base; @@ -397,14 +385,6 @@ static void hdmi_core_config_video_packetizer(struct hdmi_core_data *core) REG_FLD_MOD(base, HDMI_CORE_VP_CONF, clr_depth ? 0 : 2, 1, 0); }
-static void hdmi_core_config_csc(struct hdmi_core_data *core) -{ - int clr_depth = 0; /* 24 bit color depth */ - - /* CSC_COLORDEPTH */ - REG_FLD_MOD(core->base, HDMI_CORE_CSC_SCALE, clr_depth, 7, 4); -} - static void hdmi_core_config_video_sampler(struct hdmi_core_data *core) { int video_mapping = 1; /* for 24 bit color depth */ @@ -469,47 +449,67 @@ static void hdmi_core_write_avi_infoframe(struct hdmi_core_data *core, REG_FLD_MOD(base, HDMI_CORE_FC_PRCONF, pr, 3, 0); }
-static void hdmi_core_csc_config(struct hdmi_core_data *core, - struct csc_table csc_coeff) +static void hdmi_core_write_csc(struct hdmi_core_data *core, + const struct csc_table *csc_coeff) { void __iomem *base = core->base;
- REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_MSB, csc_coeff.a1 >> 8 , 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_LSB, csc_coeff.a1, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_MSB, csc_coeff.a2 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_LSB, csc_coeff.a2, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_MSB, csc_coeff.a3 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_LSB, csc_coeff.a3, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_MSB, csc_coeff.a4 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_LSB, csc_coeff.a4, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_MSB, csc_coeff.b1 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_LSB, csc_coeff.b1, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_MSB, csc_coeff.b2 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_LSB, csc_coeff.b2, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_MSB, csc_coeff.b3 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_LSB, csc_coeff.b3, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_MSB, csc_coeff.b4 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_LSB, csc_coeff.b4, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_MSB, csc_coeff.c1 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_LSB, csc_coeff.c1, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_MSB, csc_coeff.c2 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_LSB, csc_coeff.c2, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_MSB, csc_coeff.c3 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_LSB, csc_coeff.c3, 7, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_MSB, csc_coeff.c4 >> 8, 6, 0); - REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_LSB, csc_coeff.c4, 7, 0); - + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_MSB, csc_coeff->a1 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_LSB, csc_coeff->a1, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_MSB, csc_coeff->a2 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_LSB, csc_coeff->a2, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_MSB, csc_coeff->a3 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_LSB, csc_coeff->a3, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_MSB, csc_coeff->a4 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_LSB, csc_coeff->a4, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_MSB, csc_coeff->b1 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_LSB, csc_coeff->b1, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_MSB, csc_coeff->b2 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_LSB, csc_coeff->b2, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_MSB, csc_coeff->b3 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_LSB, csc_coeff->b3, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_MSB, csc_coeff->b4 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_LSB, csc_coeff->b4, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_MSB, csc_coeff->c1 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_LSB, csc_coeff->c1, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_MSB, csc_coeff->c2 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_LSB, csc_coeff->c2, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_MSB, csc_coeff->c3 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_LSB, csc_coeff->c3, 7, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_MSB, csc_coeff->c4 >> 8, 6, 0); + REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_LSB, csc_coeff->c4, 7, 0); + + /* enable CSC */ REG_FLD_MOD(base, HDMI_CORE_MC_FLOWCTRL, 0x1, 0, 0); }
-static void hdmi_core_configure_range(struct hdmi_core_data *core) +static void hdmi_core_configure_range(struct hdmi_core_data *core, + enum hdmi_quantization_range range) { - struct csc_table csc_coeff = { 0 }; + static const struct csc_table csc_limited_range = { + 7036, 0, 0, 32, 0, 7036, 0, 32, 0, 0, 7036, 32 + }; + static const struct csc_table csc_full_range = { + 8192, 0, 0, 0, 0, 8192, 0, 0, 0, 0, 8192, 0 + }; + const struct csc_table *csc_coeff; + + /* CSC_COLORDEPTH = 24 bits*/ + REG_FLD_MOD(core->base, HDMI_CORE_CSC_SCALE, 0, 7, 4); + + switch (range) { + case HDMI_QUANTIZATION_RANGE_FULL: + csc_coeff = &csc_full_range; + break;
- /* support limited range with 24 bit color depth for now */ - csc_coeff = csc_table_deepcolor[0]; + case HDMI_QUANTIZATION_RANGE_DEFAULT: + case HDMI_QUANTIZATION_RANGE_LIMITED: + default: + csc_coeff = &csc_limited_range; + break; + }
- hdmi_core_csc_config(core, csc_coeff); + hdmi_core_write_csc(core, csc_coeff); }
static void hdmi_core_enable_video_path(struct hdmi_core_data *core) @@ -600,9 +600,20 @@ void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp, struct videomode vm; struct hdmi_video_format video_format; struct hdmi_core_vid_config v_core_cfg; + enum hdmi_quantization_range range;
hdmi_core_mask_interrupts(core);
+ if (cfg->hdmi_dvi_mode == HDMI_HDMI) { + char vic = cfg->infoframe.video_code; + + /* All CEA modes other than VIC 1 use limited quantization range. */ + range = vic > 1 ? HDMI_QUANTIZATION_RANGE_LIMITED : + HDMI_QUANTIZATION_RANGE_FULL; + } else { + range = HDMI_QUANTIZATION_RANGE_FULL; + } + hdmi_core_init(&v_core_cfg, cfg);
hdmi_wp_init_vid_fmt_timings(&video_format, &vm, cfg); @@ -616,9 +627,8 @@ void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
hdmi_wp_video_config_interface(wp, &vm);
- /* support limited range with 24 bit color depth for now */ - hdmi_core_configure_range(core); - cfg->infoframe.quantization_range = HDMI_QUANTIZATION_RANGE_LIMITED; + hdmi_core_configure_range(core, range); + cfg->infoframe.quantization_range = range;
/* * configure core video part, set software reset in the core @@ -628,7 +638,6 @@ void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp, hdmi_core_video_config(core, &v_core_cfg);
hdmi_core_config_video_packetizer(core); - hdmi_core_config_csc(core); hdmi_core_config_video_sampler(core);
if (cfg->hdmi_dvi_mode == HDMI_HDMI)
If use_mclk is false, mclk_mode is written to a register without initialization. This doesn't cause any ill effects as the written value is not used when use_mclk is false.
To fix this, write use_mclk only when use_mclk is true.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c index 5d5d5588ebc1..c4ffe96e28bc 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c @@ -542,8 +542,9 @@ static void hdmi_core_audio_config(struct hdmi_core_data *core, }
/* Set ACR clock divisor */ - REG_FLD_MOD(av_base, - HDMI_CORE_AV_FREQ_SVAL, cfg->mclk_mode, 2, 0); + if (cfg->use_mclk) + REG_FLD_MOD(av_base, HDMI_CORE_AV_FREQ_SVAL, + cfg->mclk_mode, 2, 0);
r = hdmi_read_reg(av_base, HDMI_CORE_AV_ACR_CTRL); /*
Hi Tomi,
Thank you for the patch.
On Mon, Sep 30, 2019 at 01:38:40PM +0300, Tomi Valkeinen wrote:
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
* Tomi Valkeinen tomi.valkeinen@ti.com [190930 10:38]:
Hey nice catch. Based on a quick test looks like this fixes an issue where power consumption stays higher after using HDMI.
Would be nice to have merged in the v5.4-rc series:
Tested-by: Tony Lindgren tony@atomide.com
On 08/10/2019 17:13, Tony Lindgren wrote:
Really? Ok, well, then it was a good random find =).
I did already push this to drm-misc-next, as I thought it does not have any real effect. I'll check if it's ok to push to drm-misc-fixes too, with Cc stable.
Tomi
* Tomi Valkeinen tomi.valkeinen@ti.com [191008 14:17]:
Yeah so it seems :) Earlier I thought there's still some clkctrl setting wrong after using HDMI, but did not see anything diffing the clkctrl registers before and after and gave up.
OK great thanks.
Tony
On 08/10/2019 17:21, Tony Lindgren wrote:
Pushing this to fixes too would cause conflicts, so we shouldn't push without good reason. How much power saving you see?
I think this can still be sent to stable later, after it has been merged to mainline.
Tomi
* Tomi Valkeinen tomi.valkeinen@ti.com [191010 06:48]:
Sure no rush with this one. I should also test again that it really fixes the issue I'm seeing.
Hmm so what register does this clock actually change?
I'm seeing an increase of few tens of extra mW, which means at least one day of standby time less for me :) It does not happen always, maybe half of the time.
I think this can still be sent to stable later, after it has been merged to mainline.
Yes sure.
Thanks,
Tony
On 10/10/2019 16:24, Tony Lindgren wrote:
I have no idea why this would affect power consumption. As far as I can understand, the bits written here are a clk divider MCLK. I don't see why that would affect.
Maybe Jyri or Peter has an idea, I have never looked at the HDMI audio side.
Tomi
* Tomi Valkeinen tomi.valkeinen@ti.com [191011 10:25]:
Yeah you're right, and I just got lucky initially.
I have seen the power consumption stay higher already with the patch applied. The clocks seem just fine.
Maybe Jyri or Peter has an idea, I have never looked at the HDMI audio side.
I'll try dumping out the hdmi registers before and after when I get a chance.
Regards,
Tony
dri-devel@lists.freedesktop.org