PATCH V10 2021-11-30 22:26:41: - patch 3/8: fix $id and $ref paths (found by robh@kernel.org)
PATCH V9 2021-11-24 22:29:14: - patch 6/8: remove optional <0> for assigned-clocks and unintentionally included "unwedge" setup (found by paul@crapouillou.net) - patch 4/8: some cosmetics make regulator enable/disable only if not NULL (found by paul@crapouillou.net) simplify/fix error handling and driver cleanup on remove (proposed by paul@crapouillou.net) - patch 3/8: fix #include path in example (found by paul@crapouillou.net) fix missing "i" in unevaluatedProperties (found by robh@kernel.org) fix 4 spaces indentation for required: property (found by robh@kernel.org)
PATCH V8 2021-11-23 19:14:00: - fix a bad editing result from patch 2/8 (found by paul@crapouillou.net)
PATCH V7 2021-11-23 18:46:23: - changed gpio polarity of hdmi_power to 0 (suggested by paul@crapouillou.net) - fixed LCD1 irq number (bug found by paul@crapouillou.net) - removed "- 4" for calculating max_register (suggested by paul@crapouillou.net) - use unevaluatedPropertes instead of additionalProperties (suggested by robh@kernel.org) - moved and renamed ingenic,jz4780-hdmi.yaml (suggested by robh@kernel.org) - adjusted assigned-clocks changes to upstream which added some for SSI (by hns@goldelico.com) - rebased and tested with v5.16-rc2 + patch set drm/ingenic by paul@crapouillou.net (by hns@goldelico.com)
PATCH V6 2021-11-10 20:43:33: - changed CONFIG_DRM_INGENIC_DW_HDMI to "m" (by hns@goldelico.com) - made ingenic-dw-hdmi an independent platform driver which can be compiled as module and removed error patch fixes for IPU (suggested by paul@crapouillou.net) - moved assigned-clocks from jz4780.dtsi to ci20.dts (suggested by paul@crapouillou.net) - fixed reg property in jz4780.dtsi to cover all registers incl. gamma and vee (by hns@goldelico.com) - added a base patch to calculate regmap size from DTS reg property (requested by paul@crapouillou.net) - restored resetting all bits except one in LCDOSDC (requested by paul@crapouillou.net) - clarified setting of cpos (suggested by paul@crapouillou.net) - moved bindings definition for ddc-i2c-bus (suggested by paul@crapouillou.net) - simplified mask definitions for JZ_LCD_DESSIZE (requested by paul@crapouillou.net) - removed setting alpha premultiplication (suggested by paul@crapouillou.net) - removed some comments (suggested by paul@crapouillou.net)
PATCH V5 2021-10-05 14:28:44: - dropped mode_fixup and timings support in dw-hdmi as it is no longer needed in this V5 (by hns@goldelico.com) - dropped "drm/ingenic: add some jz4780 specific features" (stimulated by paul@crapouillou.net) - fixed typo in commit subject: "synopsis" -> "synopsys" (by hns@goldelico.com) - swapped clocks in jz4780.dtsi to match synopsys,dw-hdmi.yaml (by hns@goldelico.com) - improved, simplified, fixed, dtbschecked ingenic-jz4780-hdmi.yaml and made dependent of bridge/synopsys,dw-hdmi.yaml (based on suggestions by maxime@cerno.tech) - fixed binding vs. driver&DTS use of hdmi-5v regulator (suggested by maxime@cerno.tech) - dropped "drm/bridge: synopsis: Fix to properly handle HPD" - was a no longer needed workaround for a previous version (suggested by maxime@cerno.tech)
PATCH V4 2021-09-27 18:44:38: - fix setting output_port = 1 (issue found by paul@crapouillou.net) - ci20.dts: convert to use hdmi-connector (by hns@goldelico.com) - add a hdmi-regulator to control +5V power (by hns@goldelico.com) - added a fix to dw-hdmi to call drm_kms_helper_hotplug_event on plugin event detection (by hns@goldelico.com) - always allocate extended descriptor but initialize only for jz4780 (by hns@goldelico.com) - updated to work on top of "[PATCH v3 0/6] drm/ingenic: Various improvements v3" (by paul@crapouillou.net) - rebased to v5.13-rc3
PATCH V3 2021-08-08 07:10:50: This series adds HDMI support for JZ4780 and CI20 board (and fixes one IPU related issue in registration error path) - [patch 1/8] switched from mode_fixup to atomic_check (suggested by robert.foss@linaro.org) - the call to the dw-hdmi specialization is still called mode_fixup - [patch 3/8] diverse fixes for ingenic-drm-drv (suggested by paul@crapouillou.net) - factor out some non-HDMI features of the jz4780 into a separate patch - multiple fixes around max height - do not change regmap config but a copy on stack - define some constants - factor out fixing of drm_init error path for IPU into separate patch - use FIELD_PREP() - [patch 8/8] conversion to component framework dropped (suggested by Laurent.pinchart@ideasonboard.com and paul@crapouillou.net)
PATCH V2 2021-08-05 16:08:05: - code and commit messages revisited for checkpatch warnings - rebased on v5.14-rc4 - include (failed, hence RFC 8/8) attempt to convert to component framework (was suggested by Paul Cercueil paul@crapouillou.net a while ago)
This series adds HDMI support for JZ4780 and CI20 board
H. Nikolaus Schaller (3): drm/ingenic: prepare ingenic drm for later addition of JZ4780 MIPS: defconfig: CI20: configure for DRM_DW_HDMI_JZ4780 [RFC] MIPS: DTS: Ingenic: adjust register size to available registers
Paul Boddie (4): drm/ingenic: Add support for JZ4780 and HDMI output drm/ingenic: Add dw-hdmi driver for jz4780 MIPS: DTS: jz4780: Account for Synopsys HDMI driver and LCD controllers MIPS: DTS: CI20: Add DT nodes for HDMI setup
Sam Ravnborg (1): dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema
.../display/bridge/ingenic,jz4780-hdmi.yaml | 76 ++++++++++ .../display/bridge/synopsys,dw-hdmi.yaml | 3 + arch/mips/boot/dts/ingenic/ci20.dts | 72 ++++++++- arch/mips/boot/dts/ingenic/jz4725b.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4780.dtsi | 40 +++++ arch/mips/configs/ci20_defconfig | 6 + drivers/gpu/drm/ingenic/Kconfig | 9 ++ drivers/gpu/drm/ingenic/Makefile | 1 + drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 62 +++++++- drivers/gpu/drm/ingenic/ingenic-drm.h | 38 +++++ drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 138 ++++++++++++++++++ 13 files changed, 443 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
This changes the way the regmap is allocated to prepare for the later addition of the JZ4780 which has more registers and bits than the others.
Therefore we make the regmap as big as the reg property in the device tree tells.
Suggested-by: Paul Cercueil paul@crapouillou.net Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 462bc0f35f1bf..0bb590c3910d9 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -173,7 +173,6 @@ static const struct regmap_config ingenic_drm_regmap_config = { .val_bits = 32, .reg_stride = 4,
- .max_register = JZ_REG_LCD_SIZE1, .writeable_reg = ingenic_drm_writeable_reg, };
@@ -1011,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) struct ingenic_drm_bridge *ib; struct drm_device *drm; void __iomem *base; + struct resource *res; + struct regmap_config regmap_config; long parent_rate; unsigned int i, clone_mask = 0; int ret, irq; @@ -1056,14 +1057,16 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) drm->mode_config.funcs = &ingenic_drm_mode_config_funcs; drm->mode_config.helper_private = &ingenic_drm_mode_config_helpers;
- base = devm_platform_ioremap_resource(pdev, 0); + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(base)) { dev_err(dev, "Failed to get memory resource\n"); return PTR_ERR(base); }
+ regmap_config = ingenic_drm_regmap_config; + regmap_config.max_register = res->end - res->start; priv->map = devm_regmap_init_mmio(dev, base, - &ingenic_drm_regmap_config); + ®map_config); if (IS_ERR(priv->map)) { dev_err(dev, "Failed to create regmap\n"); return PTR_ERR(priv->map);
From: Paul Boddie paul@boddie.org.uk
Add support for the LCD controller present on JZ4780 SoCs. This SoC uses 8-byte descriptors which extend the current 4-byte descriptors used for other Ingenic SoCs.
Tested on MIPS Creator CI20 board.
Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: Ezequiel Garcia ezequiel@collabora.com Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 53 +++++++++++++++++++++++ drivers/gpu/drm/ingenic/ingenic-drm.h | 38 ++++++++++++++++ 2 files changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 0bb590c3910d9..34089bc6e0fcd 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -6,6 +6,7 @@
#include "ingenic-drm.h"
+#include <linux/bitfield.h> #include <linux/component.h> #include <linux/clk.h> #include <linux/dma-mapping.h> @@ -49,6 +50,11 @@ struct ingenic_dma_hwdesc { u32 addr; u32 id; u32 cmd; + /* extended hw descriptor for jz4780 */ + u32 offsize; + u32 pagewidth; + u32 cpos; + u32 dessize; } __aligned(16);
struct ingenic_dma_hwdescs { @@ -60,6 +66,7 @@ struct jz_soc_info { bool needs_dev_clk; bool has_osd; bool map_noncoherent; + bool use_extended_hwdesc; unsigned int max_width, max_height; const u32 *formats_f0, *formats_f1; unsigned int num_formats_f0, num_formats_f1; @@ -446,6 +453,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, if (!crtc) return 0;
+ if (plane == &priv->f0) + return -EINVAL; + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); if (WARN_ON(!crtc_state)) @@ -662,6 +672,33 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4); hwdesc->next = dma_hwdesc_addr(priv, next_id);
+ if (priv->soc_info->use_extended_hwdesc) { + hwdesc->cmd |= JZ_LCD_CMD_FRM_ENABLE; + + /* Extended 8-byte descriptor */ + hwdesc->cpos = 0; + hwdesc->offsize = 0; + hwdesc->pagewidth = 0; + + switch (newstate->fb->format->format) { + case DRM_FORMAT_XRGB1555: + hwdesc->cpos |= JZ_LCD_CPOS_RGB555; + fallthrough; + case DRM_FORMAT_RGB565: + hwdesc->cpos |= JZ_LCD_CPOS_BPP_15_16; + break; + case DRM_FORMAT_XRGB8888: + hwdesc->cpos |= JZ_LCD_CPOS_BPP_18_24; + break; + } + hwdesc->cpos |= (JZ_LCD_CPOS_COEFFICIENT_1 << + JZ_LCD_CPOS_COEFFICIENT_OFFSET); + hwdesc->dessize = + (0xff << JZ_LCD_DESSIZE_ALPHA_OFFSET) | + FIELD_PREP(JZ_LCD_DESSIZE_HEIGHT_MASK, height - 1) | + FIELD_PREP(JZ_LCD_DESSIZE_WIDTH_MASK, width - 1); + } + if (drm_atomic_crtc_needs_modeset(crtc_state)) { fourcc = newstate->fb->format->format;
@@ -693,6 +730,9 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder, | JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE; }
+ if (priv->soc_info->use_extended_hwdesc) + cfg |= JZ_LCD_CFG_DESCRIPTOR_8; + if (mode->flags & DRM_MODE_FLAG_NHSYNC) cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW; if (mode->flags & DRM_MODE_FLAG_NVSYNC) @@ -1468,10 +1508,23 @@ static const struct jz_soc_info jz4770_soc_info = { .num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0), };
+static const struct jz_soc_info jz4780_soc_info = { + .needs_dev_clk = true, + .has_osd = true, + .use_extended_hwdesc = true, + .max_width = 4096, + .max_height = 2048, + .formats_f1 = jz4770_formats_f1, + .num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1), + .formats_f0 = jz4770_formats_f0, + .num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0), +}; + static const struct of_device_id ingenic_drm_of_match[] = { { .compatible = "ingenic,jz4740-lcd", .data = &jz4740_soc_info }, { .compatible = "ingenic,jz4725b-lcd", .data = &jz4725b_soc_info }, { .compatible = "ingenic,jz4770-lcd", .data = &jz4770_soc_info }, + { .compatible = "ingenic,jz4780-lcd", .data = &jz4780_soc_info }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, ingenic_drm_of_match); diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h index 22654ac1dde1c..cb1d09b625881 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.h +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h @@ -44,8 +44,11 @@ #define JZ_REG_LCD_XYP1 0x124 #define JZ_REG_LCD_SIZE0 0x128 #define JZ_REG_LCD_SIZE1 0x12c +#define JZ_REG_LCD_PCFG 0x2c0
#define JZ_LCD_CFG_SLCD BIT(31) +#define JZ_LCD_CFG_DESCRIPTOR_8 BIT(28) +#define JZ_LCD_CFG_RECOVER_FIFO_UNDERRUN BIT(25) #define JZ_LCD_CFG_PS_DISABLE BIT(23) #define JZ_LCD_CFG_CLS_DISABLE BIT(22) #define JZ_LCD_CFG_SPL_DISABLE BIT(21) @@ -63,6 +66,7 @@ #define JZ_LCD_CFG_DE_ACTIVE_LOW BIT(9) #define JZ_LCD_CFG_VSYNC_ACTIVE_LOW BIT(8) #define JZ_LCD_CFG_18_BIT BIT(7) +#define JZ_LCD_CFG_24_BIT BIT(6) #define JZ_LCD_CFG_PDW (BIT(5) | BIT(4))
#define JZ_LCD_CFG_MODE_GENERIC_16BIT 0 @@ -132,6 +136,7 @@ #define JZ_LCD_CMD_SOF_IRQ BIT(31) #define JZ_LCD_CMD_EOF_IRQ BIT(30) #define JZ_LCD_CMD_ENABLE_PAL BIT(28) +#define JZ_LCD_CMD_FRM_ENABLE BIT(26)
#define JZ_LCD_SYNC_MASK 0x3ff
@@ -153,6 +158,7 @@ #define JZ_LCD_RGBC_EVEN_BGR (0x5 << 0)
#define JZ_LCD_OSDC_OSDEN BIT(0) +#define JZ_LCD_OSDC_ALPHAEN BIT(2) #define JZ_LCD_OSDC_F0EN BIT(3) #define JZ_LCD_OSDC_F1EN BIT(4)
@@ -176,6 +182,38 @@ #define JZ_LCD_SIZE01_WIDTH_LSB 0 #define JZ_LCD_SIZE01_HEIGHT_LSB 16
+#define JZ_LCD_DESSIZE_ALPHA_OFFSET 24 +#define JZ_LCD_DESSIZE_HEIGHT_MASK GENMASK(23, 12) +#define JZ_LCD_DESSIZE_WIDTH_MASK GENMASK(11, 0) + +#define JZ_LCD_CPOS_BPP_15_16 (4 << 27) +#define JZ_LCD_CPOS_BPP_18_24 (5 << 27) +#define JZ_LCD_CPOS_BPP_30 (7 << 27) +#define JZ_LCD_CPOS_RGB555 BIT(30) +#define JZ_LCD_CPOS_PREMULTIPLY_LCD BIT(26) +#define JZ_LCD_CPOS_COEFFICIENT_OFFSET 24 +#define JZ_LCD_CPOS_COEFFICIENT_0 0 +#define JZ_LCD_CPOS_COEFFICIENT_1 1 +#define JZ_LCD_CPOS_COEFFICIENT_ALPHA1 2 +#define JZ_LCD_CPOS_COEFFICIENT_1_ALPHA1 3 + +#define JZ_LCD_RGBC_RGB_PADDING BIT(15) +#define JZ_LCD_RGBC_RGB_PADDING_FIRST BIT(14) +#define JZ_LCD_RGBC_422 BIT(8) +#define JZ_LCD_RGBC_RGB_FORMAT_ENABLE BIT(7) + +#define JZ_LCD_PCFG_PRI_MODE BIT(31) +#define JZ_LCD_PCFG_HP_BST_4 (0 << 28) +#define JZ_LCD_PCFG_HP_BST_8 (1 << 28) +#define JZ_LCD_PCFG_HP_BST_16 (2 << 28) +#define JZ_LCD_PCFG_HP_BST_32 (3 << 28) +#define JZ_LCD_PCFG_HP_BST_64 (4 << 28) +#define JZ_LCD_PCFG_HP_BST_16_CONT (5 << 28) +#define JZ_LCD_PCFG_HP_BST_DISABLE (7 << 28) +#define JZ_LCD_PCFG_THRESHOLD2_OFFSET 18 +#define JZ_LCD_PCFG_THRESHOLD1_OFFSET 9 +#define JZ_LCD_PCFG_THRESHOLD0_OFFSET 0 + struct device; struct drm_plane; struct drm_plane_state;
From: Sam Ravnborg sam@ravnborg.org
Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC. Based on .txt binding from Zubair Lutfullah Kakakhel
We also add generic ddc-i2c-bus to synopsys,dw-hdmi.yaml
Signed-off-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com Cc: Rob Herring robh@kernel.org Cc: devicetree@vger.kernel.org --- .../display/bridge/ingenic,jz4780-hdmi.yaml | 76 +++++++++++++++++++ .../display/bridge/synopsys,dw-hdmi.yaml | 3 + 2 files changed, 79 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml new file mode 100644 index 0000000000000..5f372bdea0f0d --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml @@ -0,0 +1,76 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/ingenic,jz4780-hdmi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Bindings for Ingenic JZ4780 HDMI Transmitter + +maintainers: + - H. Nikolaus Schaller hns@goldelico.com + +description: | + The HDMI Transmitter in the Ingenic JZ4780 is a Synopsys DesignWare HDMI 1.4 + TX controller IP with accompanying PHY IP. + +allOf: + - $ref: synopsys,dw-hdmi.yaml# + +properties: + compatible: + const: ingenic,jz4780-dw-hdmi + + reg-io-width: + const: 4 + + clocks: + maxItems: 2 + + hdmi-5v-supply: + description: Optional regulator to provide +5V at the connector + + ports: + $ref: /schemas/graph.yaml#/properties/ports + +required: + - compatible + - clocks + - clock-names + - ports + - reg-io-width + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/ingenic,jz4780-cgu.h> + + hdmi: hdmi@10180000 { + compatible = "ingenic,jz4780-dw-hdmi"; + reg = <0x10180000 0x8000>; + reg-io-width = <4>; + ddc-i2c-bus = <&i2c4>; + interrupt-parent = <&intc>; + interrupts = <3>; + clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>; + clock-names = "iahb", "isfr"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + hdmi_in: port@0 { + reg = <0>; + dw_hdmi_in: endpoint { + remote-endpoint = <&jz4780_lcd_out>; + }; + }; + hdmi_out: port@1 { + reg = <1>; + dw_hdmi_out: endpoint { + remote-endpoint = <&hdmi_con>; + }; + }; + }; + }; + +... diff --git a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml index 9be44a682e67a..9cbeabaee0968 100644 --- a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml +++ b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml @@ -50,6 +50,9 @@ properties: interrupts: maxItems: 1
+ ddc-i2c-bus: + description: An I2C interface if the internal DDC I2C driver is not to be used + additionalProperties: true
...
From: Paul Boddie paul@boddie.org.uk
A specialisation of the generic Synopsys HDMI driver is employed for JZ4780 HDMI support. This requires a new driver, plus device tree and configuration modifications.
Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.
Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: Ezequiel Garcia ezequiel@collabora.com Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/gpu/drm/ingenic/Kconfig | 9 ++ drivers/gpu/drm/ingenic/Makefile | 1 + drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 138 ++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 3b57f8be007c4..4efc709d77b0a 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig @@ -25,4 +25,13 @@ config DRM_INGENIC_IPU
The Image Processing Unit (IPU) will appear as a second primary plane.
+config DRM_INGENIC_DW_HDMI + tristate "Ingenic specific support for Synopsys DW HDMI" + depends on MACH_JZ4780 + select DRM_DW_HDMI + help + Choose this option to enable Synopsys DesignWare HDMI based driver. + If you want to enable HDMI on Ingenic JZ4780 based SoC, you should + select this option.. + endif diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile index d313326bdddbb..f10cc1c5a5f22 100644 --- a/drivers/gpu/drm/ingenic/Makefile +++ b/drivers/gpu/drm/ingenic/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o ingenic-drm-y = ingenic-drm-drv.o ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c new file mode 100644 index 0000000000000..199e39c227d29 --- /dev/null +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc. + * Copyright (C) 2019, 2020 Paul Boddie paul@boddie.org.uk + * + * Derived from dw_hdmi-imx.c with i.MX portions removed. + * Probe and remove operations derived from rcar_dw_hdmi.c. + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#include <drm/bridge/dw_hdmi.h> +#include <drm/drm_of.h> +#include <drm/drm_print.h> + +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = { + { 45250000, { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 0x0000 } } }, + { 92500000, { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } }, + { 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } }, + { 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } }, + { ~0UL, { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 0x0000 } } } +}; + +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = { + /*pixelclk bpp8 bpp10 bpp12 */ + { 54000000, { 0x091c, 0x091c, 0x06dc } }, + { 58400000, { 0x091c, 0x06dc, 0x06dc } }, + { 72000000, { 0x06dc, 0x06dc, 0x091c } }, + { 74250000, { 0x06dc, 0x0b5c, 0x091c } }, + { 118800000, { 0x091c, 0x091c, 0x06dc } }, + { 216000000, { 0x06dc, 0x0b5c, 0x091c } }, + { ~0UL, { 0x0000, 0x0000, 0x0000 } }, +}; + +/* + * Resistance term 133Ohm Cfg + * PREEMP config 0.00 + * TX/CK level 10 + */ +static const struct dw_hdmi_phy_config ingenic_phy_config[] = { + /*pixelclk symbol term vlev */ + { 216000000, 0x800d, 0x0005, 0x01ad}, + { ~0UL, 0x0000, 0x0000, 0x0000} +}; + +static enum drm_mode_status +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, + const struct drm_display_info *info, + const struct drm_display_mode *mode) +{ + if (mode->clock < 13500) + return MODE_CLOCK_LOW; + /* FIXME: Hardware is capable of 270MHz, but setup data is missing. */ + if (mode->clock > 216000) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = { + .mpll_cfg = ingenic_mpll_cfg, + .cur_ctr = ingenic_cur_ctr, + .phy_config = ingenic_phy_config, + .mode_valid = ingenic_dw_hdmi_mode_valid, + .output_port = 1, +}; + +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = { + { .compatible = "ingenic,jz4780-dw-hdmi" }, + { /* Sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids); + +static void ingenic_dw_hdmi_cleanup(void *data) +{ + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; + + dw_hdmi_remove(hdmi); +} + +static void ingenic_dw_hdmi_disable_regulator(void *data) +{ + struct regulator *regulator = (struct regulator *)data; + + regulator_disable(regulator); +} + +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) +{ + struct dw_hdmi *hdmi; + struct regulator *regulator; + int ret; + + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); + if (IS_ERR(hdmi)) + return PTR_ERR(hdmi); + + ret = devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi); + if (ret) + return ret; + + regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v"); + if (IS_ERR(regulator)) { + ret = PTR_ERR(regulator); + + DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %s (%d)\n", + "hdmi-5v", ret); + + return ret; + } + + if (!regulator) + return 0; + ret = regulator_enable(regulator); + if (ret) { + DRM_DEV_ERROR(&pdev->dev, "Failed to enable hpd regulator: %d\n", + ret); + + return ret; + } + + return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_disable_regulator, + regulator); +} + +static struct platform_driver ingenic_dw_hdmi_driver = { + .probe = ingenic_dw_hdmi_probe, + .driver = { + .name = "dw-hdmi-ingenic", + .of_match_table = ingenic_dw_hdmi_dt_ids, + }, +}; +module_platform_driver(ingenic_dw_hdmi_driver); + +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:dwhdmi-ingenic");
Hi Nikolaus,
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller hns@goldelico.com a écrit :
From: Paul Boddie paul@boddie.org.uk
A specialisation of the generic Synopsys HDMI driver is employed for JZ4780 HDMI support. This requires a new driver, plus device tree and configuration modifications.
Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.
Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: Ezequiel Garcia ezequiel@collabora.com Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
drivers/gpu/drm/ingenic/Kconfig | 9 ++ drivers/gpu/drm/ingenic/Makefile | 1 + drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 138 ++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 3b57f8be007c4..4efc709d77b0a 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig @@ -25,4 +25,13 @@ config DRM_INGENIC_IPU
The Image Processing Unit (IPU) will appear as a second primaryplane.
+config DRM_INGENIC_DW_HDMI
- tristate "Ingenic specific support for Synopsys DW HDMI"
- depends on MACH_JZ4780
- select DRM_DW_HDMI
- help
Choose this option to enable Synopsys DesignWare HDMI baseddriver.
If you want to enable HDMI on Ingenic JZ4780 based SoC, you shouldselect this option..endif diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile index d313326bdddbb..f10cc1c5a5f22 100644 --- a/drivers/gpu/drm/ingenic/Makefile +++ b/drivers/gpu/drm/ingenic/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o ingenic-drm-y = ingenic-drm-drv.o ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c new file mode 100644 index 0000000000000..199e39c227d29 --- /dev/null +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
- Copyright (C) 2019, 2020 Paul Boddie paul@boddie.org.uk
- Derived from dw_hdmi-imx.c with i.MX portions removed.
- Probe and remove operations derived from rcar_dw_hdmi.c.
- */
+#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h>
+#include <drm/bridge/dw_hdmi.h> +#include <drm/drm_of.h> +#include <drm/drm_print.h>
+static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = {
- { 45250000, { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2,
0x0000 } } },
- { 92500000, { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142,
0x0005 } } },
- { 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2,
0x000a } } },
- { 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002,
0x000f } } },
- { ~0UL, { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000,
0x0000 } } } +};
+static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = {
- /*pixelclk bpp8 bpp10 bpp12 */
- { 54000000, { 0x091c, 0x091c, 0x06dc } },
- { 58400000, { 0x091c, 0x06dc, 0x06dc } },
- { 72000000, { 0x06dc, 0x06dc, 0x091c } },
- { 74250000, { 0x06dc, 0x0b5c, 0x091c } },
- { 118800000, { 0x091c, 0x091c, 0x06dc } },
- { 216000000, { 0x06dc, 0x0b5c, 0x091c } },
- { ~0UL, { 0x0000, 0x0000, 0x0000 } },
+};
+/*
- Resistance term 133Ohm Cfg
- PREEMP config 0.00
- TX/CK level 10
- */
+static const struct dw_hdmi_phy_config ingenic_phy_config[] = {
- /*pixelclk symbol term vlev */
- { 216000000, 0x800d, 0x0005, 0x01ad},
- { ~0UL, 0x0000, 0x0000, 0x0000}
+};
+static enum drm_mode_status +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
const struct drm_display_info *info,const struct drm_display_mode *mode)+{
- if (mode->clock < 13500)
return MODE_CLOCK_LOW;- /* FIXME: Hardware is capable of 270MHz, but setup data is missing.
*/
- if (mode->clock > 216000)
return MODE_CLOCK_HIGH;- return MODE_OK;
+}
+static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = {
- .mpll_cfg = ingenic_mpll_cfg,
- .cur_ctr = ingenic_cur_ctr,
- .phy_config = ingenic_phy_config,
- .mode_valid = ingenic_dw_hdmi_mode_valid,
- .output_port = 1,
+};
+static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = {
- { .compatible = "ingenic,jz4780-dw-hdmi" },
- { /* Sentinel */ },
+}; +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids);
+static void ingenic_dw_hdmi_cleanup(void *data) +{
- struct dw_hdmi *hdmi = (struct dw_hdmi *)data;
- dw_hdmi_remove(hdmi);
+}
+static void ingenic_dw_hdmi_disable_regulator(void *data) +{
- struct regulator *regulator = (struct regulator *)data;
- regulator_disable(regulator);
+}
+static int ingenic_dw_hdmi_probe(struct platform_device *pdev) +{
- struct dw_hdmi *hdmi;
- struct regulator *regulator;
- int ret;
- hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data);
- if (IS_ERR(hdmi))
return PTR_ERR(hdmi);- ret = devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup,
hdmi);
- if (ret)
return ret;- regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
- if (IS_ERR(regulator)) {
ret = PTR_ERR(regulator);DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %s (%d)\n","hdmi-5v", ret);return ret;- }
- if (!regulator)
return 0;
Blank line here.
But I can add it myself when applying. I'll just wait for Rob's ack first.
-Paul
- ret = regulator_enable(regulator);
- if (ret) {
DRM_DEV_ERROR(&pdev->dev, "Failed to enable hpd regulator: %d\n",ret);return ret;- }
- return devm_add_action_or_reset(&pdev->dev,
ingenic_dw_hdmi_disable_regulator,
regulator);+}
+static struct platform_driver ingenic_dw_hdmi_driver = {
- .probe = ingenic_dw_hdmi_probe,
- .driver = {
.name = "dw-hdmi-ingenic",.of_match_table = ingenic_dw_hdmi_dt_ids,- },
+}; +module_platform_driver(ingenic_dw_hdmi_driver);
+MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); +MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dwhdmi-ingenic");
2.33.0
Hi Paul,
Am 01.12.2021 um 14:02 schrieb Paul Cercueil paul@crapouillou.net:
Hi Nikolaus,
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller hns@goldelico.com a écrit :
From: Paul Boddie paul@boddie.org.uk A specialisation of the generic Synopsys HDMI driver is employed for JZ4780 HDMI support. This requires a new driver, plus device tree and configuration modifications. Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code. Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: Ezequiel Garcia ezequiel@collabora.com Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
drivers/gpu/drm/ingenic/Kconfig | 9 ++ drivers/gpu/drm/ingenic/Makefile | 1 + drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 138 ++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 3b57f8be007c4..4efc709d77b0a 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig
- }
- if (!regulator)
return 0;Blank line here.
It is one of these cases where checkpatch doesn't complain although it should be improved...
But I can add it myself when applying.
Yes, please. So that we are not wasting mailing list bandwidth...
I'll just wait for Rob's ack first.
Indeed. Fortunately he had the right hint how to fix 3/8 quickly.
BR and thanks, NIkolaus
On Wed, Dec 01, 2021 at 01:02:45PM +0000, Paul Cercueil wrote:
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller
- regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
- if (IS_ERR(regulator)) {
ret = PTR_ERR(regulator);
Why is this using _optional()? This should only be done when the supply can be physically absent (in which case I'd expect to see special handling).
Hi,
Am 01.12.2021 um 14:39 schrieb Mark Brown broonie@kernel.org:
On Wed, Dec 01, 2021 at 01:02:45PM +0000, Paul Cercueil wrote:
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller
- regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
- if (IS_ERR(regulator)) {
ret = PTR_ERR(regulator);Why is this using _optional()? This should only be done when the supply can be physically absent
There can be +5V for HDMI but without a regulator that is visible to or controllable by the driver.
So hdmi-5v can be simply missing in DTS in which case the driver does not need to care about. The driver just can't turn it on or off.
(in which case I'd expect to see special handling).
The special case is to not enable/disable the regulator if it does not exist and assume that there is hardware providing it otherwise (the driver can't know that except by using get_optional). This is done by the code below
- if (IS_ERR(regulator)) {
...
if (!regulator)
return 0;ret = regulator_enable(regulator);
...
BR and thanks, Nikolaus
Hi Nikolaus, Mark,
Le mer., déc. 1 2021 at 14:51:51 +0100, H. Nikolaus Schaller hns@goldelico.com a écrit :
Hi,
Am 01.12.2021 um 14:39 schrieb Mark Brown broonie@kernel.org:
On Wed, Dec 01, 2021 at 01:02:45PM +0000, Paul Cercueil wrote:
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller
- regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
- if (IS_ERR(regulator)) {
ret = PTR_ERR(regulator);Why is this using _optional()? This should only be done when the supply can be physically absent
There can be +5V for HDMI but without a regulator that is visible to or controllable by the driver.
There is always a power supply though. Either a controllable one (through e.g. a GPIO), or it's just connected to the mains +5V; the pin is never left floating. In the second case, in DTS the "hdmi-5v" would be connected to some 5v regulator, even if it's just a dummy VCC-5V regulator. So Mark has a point.
So hdmi-5v can be simply missing in DTS in which case the driver does not need to care about. The driver just can't turn it on or off.
Please make it mandatory in DTS then, and use devm_regulator_get() in the driver.
Cheers, -Paul
(in which case I'd expect to see special handling).
The special case is to not enable/disable the regulator if it does not exist and assume that there is hardware providing it otherwise (the driver can't know that except by using get_optional). This is done by the code below
- if (IS_ERR(regulator)) {
...
if (!regulator)
return 0;ret = regulator_enable(regulator);
...
BR and thanks, Nikolaus
Hi Paul,
Am 01.12.2021 um 15:03 schrieb Paul Cercueil paul@crapouillou.net:
Hi Nikolaus, Mark,
Le mer., déc. 1 2021 at 14:51:51 +0100, H. Nikolaus Schaller hns@goldelico.com a écrit :
Hi,
Am 01.12.2021 um 14:39 schrieb Mark Brown broonie@kernel.org: On Wed, Dec 01, 2021 at 01:02:45PM +0000, Paul Cercueil wrote:
Le mar., nov. 30 2021 at 22:26:37 +0100, H. Nikolaus Schaller
- regulator = devm_regulator_get_optional(&pdev->dev, "hdmi-5v");
- if (IS_ERR(regulator)) {
ret = PTR_ERR(regulator);Why is this using _optional()? This should only be done when the supply can be physically absent
There can be +5V for HDMI but without a regulator that is visible to or controllable by the driver.
There is always a power supply though. Either a controllable one (through e.g. a GPIO), or it's just connected to the mains +5V; the pin is never left floating. In the second case, in DTS the "hdmi-5v" would be connected to some 5v regulator, even if it's just a dummy VCC-5V regulator. So Mark has a point.
So hdmi-5v can be simply missing in DTS in which case the driver does not need to care about. The driver just can't turn it on or off.
Please make it mandatory in DTS then, and use devm_regulator_get() in the driver.
Well, I just wonder why the elegant devm_regulator_get_optional() exists at all and seems to be used in ca. 80 places.
And if it is not allowed, why some DTS should be forced to add not physically existing dummy-regulators. AFAIR drivers should implement functionality defined by DTS but not the other way round: enforce DTS style. BTW: there is no +5 mains dummy regulator defined in ci20.dts.
What I fear is that if we always have to define the mains +5V (which is for example not defined in ci20.dts), which rules stops us from asking to add a dummy-regulator from 110/230V to +5V as well. In last consequence, it seems as if we have to describe all dummy regulators from the power plant to our hdmi-5v :)
Since I always follow the KISS principle I tend to leave out what is not relevant...
Of course adding a dummy regulator to the DTS allows to avoid the NULL pointer test in the driver code.
Anyways, you are maintainers :)
So should I spin a v11 for the series or just this patch or how should we do it?
BR and thanks, Nikolaus
Cheers, -Paul
(in which case I'd expect to see special handling).
The special case is to not enable/disable the regulator if it does not exist and assume that there is hardware providing it otherwise (the driver can't know that except by using get_optional). This is done by the code below
- if (IS_ERR(regulator)) {
...
- if (!regulator)
return 0;- ret = regulator_enable(regulator);
... BR and thanks, Nikolaus
On Wed, Dec 01, 2021 at 03:33:24PM +0100, H. Nikolaus Schaller wrote:
Am 01.12.2021 um 15:03 schrieb Paul Cercueil paul@crapouillou.net:
Please make it mandatory in DTS then, and use devm_regulator_get() in the driver.
Well, I just wonder why the elegant devm_regulator_get_optional() exists at all and seems to be used in ca. 80 places.
Frankly because half of them are broken usages like this since people seem determined to have the most fragile error handling they can :/ There are valid use cases for it, with things like SD cards where some supplies are genuinely optional and simply constrain what features are available if they're omitted from the design. You also see some devices with the ability to replace internal regulators with external ones.
And if it is not allowed, why some DTS should be forced to add not physically existing dummy-regulators.
Again, if the supply can be physically absent that is a sensible use case but that means completely absent, not just not software controllable. We can represent fixed voltage regulators just fine.
AFAIR drivers should implement functionality defined by DTS but not the other way round: enforce DTS style. BTW: there is no +5 mains dummy regulator defined in ci20.dts.
It wouldn't be the first time a DTS were incomplete, and I'm sure it won't be the last.
What I fear is that if we always have to define the mains +5V (which is for example not defined in ci20.dts), which rules stops us from asking to add a dummy-regulator from 110/230V to +5V as well.
It is good practice to specify the full tree of supplies all the way to the main supply rail of the board, this ensures that if we need the information for something we've got it (even if that thing is just that we've got to the root of the tree). There's potential applications in battery supplied devices for managing very low power situations.
Hi Mark,
Am 01.12.2021 um 16:10 schrieb Mark Brown broonie@kernel.org:
On Wed, Dec 01, 2021 at 03:33:24PM +0100, H. Nikolaus Schaller wrote:
Am 01.12.2021 um 15:03 schrieb Paul Cercueil paul@crapouillou.net:
Please make it mandatory in DTS then, and use devm_regulator_get() in the driver.
Well, I just wonder why the elegant devm_regulator_get_optional() exists at all and seems to be used in ca. 80 places.
Frankly because half of them are broken usages like this since people seem determined to have the most fragile error handling they can :/
I see. I had made the mistake myself to not check for NULL pointer on regulator_disable here...
There are valid use cases for it, with things like SD cards where some supplies are genuinely optional and simply constrain what features are available if they're omitted from the design. You also see some devices with the ability to replace internal regulators with external ones.
And if it is not allowed, why some DTS should be forced to add not physically existing dummy-regulators.
Again, if the supply can be physically absent that is a sensible use case but that means completely absent, not just not software controllable. We can represent fixed voltage regulators just fine.
The question may be how we can know for a more generic driver that there is always a regulator. In the present case we know the schematics but it is just one example.
AFAIR drivers should implement functionality defined by DTS but not the other way round: enforce DTS style. BTW: there is no +5 mains dummy regulator defined in ci20.dts.
It wouldn't be the first time a DTS were incomplete, and I'm sure it won't be the last.
What I fear is that if we always have to define the mains +5V (which is for example not defined in ci20.dts), which rules stops us from asking to add a dummy-regulator from 110/230V to +5V as well.
It is good practice to specify the full tree of supplies all the way to the main supply rail of the board, this ensures that if we need the information for something we've got it (even if that thing is just that we've got to the root of the tree). There's potential applications in battery supplied devices for managing very low power situations.
Indeed. So let's modify it as you have suggested.
BR and thanks, Nikolaus
On Wed, Dec 01, 2021 at 05:53:03PM +0100, H. Nikolaus Schaller wrote:
Am 01.12.2021 um 16:10 schrieb Mark Brown broonie@kernel.org:
Again, if the supply can be physically absent that is a sensible use case but that means completely absent, not just not software controllable. We can represent fixed voltage regulators just fine.
The question may be how we can know for a more generic driver that there is always a regulator. In the present case we know the schematics but it is just one example.
The datasheet will generally explicitly call out if a supply can be disconnected. In general it is astonishingly rare for this to be the case, supporting that case will tend to make designing the chip harder (you have to cope with what happens where the power domains meet) and results in whatever functionality the supplies power not working. If not otherwise specified it's safer to assume that the supplies must be connected.
From: Paul Boddie paul@boddie.org.uk
A specialisation of the generic Synopsys HDMI driver is employed for JZ4780 HDMI support. This requires a new driver, plus device tree and configuration modifications.
Here we add jz4780 device tree setup.
Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/mips/boot/dts/ingenic/jz4780.dtsi | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi index b0a4e2e019c36..3f9ea47a10cd2 100644 --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi @@ -444,6 +444,46 @@ i2c4: i2c@10054000 { status = "disabled"; };
+ hdmi: hdmi@10180000 { + compatible = "ingenic,jz4780-dw-hdmi"; + reg = <0x10180000 0x8000>; + reg-io-width = <4>; + + clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>; + clock-names = "iahb", "isfr"; + + interrupt-parent = <&intc>; + interrupts = <3>; + + status = "disabled"; + }; + + lcdc0: lcdc0@13050000 { + compatible = "ingenic,jz4780-lcd"; + reg = <0x13050000 0x1800>; + + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>; + clock-names = "lcd", "lcd_pclk"; + + interrupt-parent = <&intc>; + interrupts = <31>; + + status = "disabled"; + }; + + lcdc1: lcdc1@130a0000 { + compatible = "ingenic,jz4780-lcd"; + reg = <0x130a0000 0x1800>; + + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD1PIXCLK>; + clock-names = "lcd", "lcd_pclk"; + + interrupt-parent = <&intc>; + interrupts = <23>; + + status = "disabled"; + }; + nemc: nemc@13410000 { compatible = "ingenic,jz4780-nemc", "simple-mfd"; reg = <0x13410000 0x10000>;
From: Paul Boddie paul@boddie.org.uk
We need to hook up * HDMI connector * HDMI power regulator * JZ4780_CLK_HDMI @ 27 MHz * DDC pinmux * HDMI and LCDC endpoint connections
Signed-off-by: Paul Boddie paul@boddie.org.uk Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/mips/boot/dts/ingenic/ci20.dts | 72 ++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-)
diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts index b249a4f0f6b62..3e336b3dbb109 100644 --- a/arch/mips/boot/dts/ingenic/ci20.dts +++ b/arch/mips/boot/dts/ingenic/ci20.dts @@ -78,6 +78,18 @@ eth0_power: fixedregulator@0 { enable-active-high; };
+ hdmi_out: connector { + compatible = "hdmi-connector"; + label = "HDMI OUT"; + type = "a"; + + port { + hdmi_con: endpoint { + remote-endpoint = <&dw_hdmi_out>; + }; + }; + }; + ir: ir { compatible = "gpio-ir-receiver"; gpios = <&gpe 3 GPIO_ACTIVE_LOW>; @@ -102,6 +114,17 @@ otg_power: fixedregulator@2 { gpio = <&gpf 14 GPIO_ACTIVE_LOW>; enable-active-high; }; + + hdmi_power: fixedregulator@3 { + compatible = "regulator-fixed"; + + regulator-name = "hdmi_power"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + + gpio = <&gpa 25 0>; + enable-active-high; + }; };
&ext { @@ -114,11 +137,12 @@ &cgu { * precision. */ assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>, - <&cgu JZ4780_CLK_SSIPLL>, <&cgu JZ4780_CLK_SSI>; + <&cgu JZ4780_CLK_SSIPLL>, <&cgu JZ4780_CLK_SSI>, + <&cgu JZ4780_CLK_HDMI>; assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>, <&cgu JZ4780_CLK_MPLL>, <&cgu JZ4780_CLK_SSIPLL>; - assigned-clock-rates = <48000000>, <0>, <54000000>; + assigned-clock-rates = <48000000>, <0>, <54000000>, <0>, <27000000>; };
&tcu { @@ -509,6 +533,12 @@ pins_i2c4: i2c4 { bias-disable; };
+ pins_hdmi_ddc: hdmi_ddc { + function = "hdmi-ddc"; + groups = "hdmi-ddc"; + bias-disable; + }; + pins_nemc: nemc { function = "nemc"; groups = "nemc-data", "nemc-cle-ale", "nemc-rd-we", "nemc-frd-fwe"; @@ -539,3 +569,41 @@ pins_mmc1: mmc1 { bias-disable; }; }; + +&hdmi { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&pins_hdmi_ddc>; + + hdmi-5v-supply = <&hdmi_power>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dw_hdmi_in: endpoint { + remote-endpoint = <&lcd_out>; + }; + }; + + port@1 { + reg = <1>; + dw_hdmi_out: endpoint { + remote-endpoint = <&hdmi_con>; + }; + }; + }; +}; + +&lcdc0 { + status = "okay"; + + port { + lcd_out: endpoint { + remote-endpoint = <&dw_hdmi_in>; + }; + }; +};
Enable CONFIG options as modules.
Signed-off-by: Ezequiel Garcia ezequiel@collabora.com Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/mips/configs/ci20_defconfig | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig index ab7ebb0668340..cc69b215854ea 100644 --- a/arch/mips/configs/ci20_defconfig +++ b/arch/mips/configs/ci20_defconfig @@ -98,7 +98,13 @@ CONFIG_RC_DEVICES=y CONFIG_IR_GPIO_CIR=m CONFIG_IR_GPIO_TX=m CONFIG_MEDIA_SUPPORT=m +CONFIG_DRM=m +CONFIG_DRM_INGENIC=m +CONFIG_DRM_INGENIC_DW_HDMI=m +CONFIG_DRM_DISPLAY_CONNECTOR=m # CONFIG_VGA_CONSOLE is not set +CONFIG_FB=y +CONFIG_FRAMEBUFFER_CONSOLE=y # CONFIG_HID is not set CONFIG_USB=y CONFIG_USB_STORAGE=y
After getting the regmap size from the device tree we should reduce the ranges to the really available registers. This allows to read only existing registers from the debug fs and makes the regmap check out-of-bounds access.
For the jz4780 we have done this already.
Suggested-for: Paul Cercueil paul@crapouillou.net Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/mips/boot/dts/ingenic/jz4725b.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +- arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/boot/dts/ingenic/jz4725b.dtsi b/arch/mips/boot/dts/ingenic/jz4725b.dtsi index 0c6a5a4266f43..e9e48022f6316 100644 --- a/arch/mips/boot/dts/ingenic/jz4725b.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4725b.dtsi @@ -321,7 +321,7 @@ udc: usb@13040000 {
lcd: lcd-controller@13050000 { compatible = "ingenic,jz4725b-lcd"; - reg = <0x13050000 0x1000>; + reg = <0x13050000 0x130>; /* tbc */
interrupt-parent = <&intc>; interrupts = <31>; diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 772542e1f266a..7f76cba03a089 100644 --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi @@ -323,7 +323,7 @@ udc: usb@13040000 {
lcd: lcd-controller@13050000 { compatible = "ingenic,jz4740-lcd"; - reg = <0x13050000 0x1000>; + reg = <0x13050000 0x60>; /* LCDCMD1+4 */
interrupt-parent = <&intc>; interrupts = <30>; diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi b/arch/mips/boot/dts/ingenic/jz4770.dtsi index dfe74328ae5dc..bda0a3a86ed5f 100644 --- a/arch/mips/boot/dts/ingenic/jz4770.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi @@ -399,7 +399,7 @@ gpu: gpu@13040000 {
lcd: lcd-controller@13050000 { compatible = "ingenic,jz4770-lcd"; - reg = <0x13050000 0x300>; + reg = <0x13050000 0x130>; /* tbc */
interrupt-parent = <&intc>; interrupts = <31>;
dri-devel@lists.freedesktop.org
-
H. Nikolaus Schaller -
Mark Brown -
Paul Cercueil