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 [email protected])
- dropped "drm/ingenic: add some jz4780 specific features" (stimulated by [email protected])
- fixed typo in commit subject: "synopsis" -> "synopsys" (by [email protected])
- swapped clocks in jz4780.dtsi to match synopsys,dw-hdmi.yaml (by [email protected])
- improved, simplified, fixed, dtbschecked ingenic-jz4780-hdmi.yaml and made dependent of bridge/synopsys,dw-hdmi.yaml (based on suggestions by [email protected])
- fixed binding vs. driver&DTS use of hdmi-5v regulator (suggested by [email protected])
- dropped "drm/bridge: synopsis: Fix to properly handle HPD" - was a no longer needed workaround for a previous version
(suggested by [email protected])
PATCH V4 2021-09-27 18:44:38:
- fix setting output_port = 1 (issue found by [email protected])
- ci20.dts: convert to use hdmi-connector (by [email protected])
- add a hdmi-regulator to control +5V power (by [email protected])
- added a fix to dw-hdmi to call drm_kms_helper_hotplug_event on plugin event detection (by [email protected])
- always allocate extended descriptor but initialize only for jz4780 (by [email protected])
- updated to work on top of "[PATCH v3 0/6] drm/ingenic: Various improvements v3" (by [email protected])
- 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 [email protected])
- the call to the dw-hdmi specialization is still called mode_fixup
- [patch 3/8] diverse fixes for ingenic-drm-drv (suggested by [email protected])
- 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 [email protected] and [email protected])
PATCH V2 2021-08-05 16:08:05:
This series adds HDMI support for JZ4780 and CI20 board
V2:
- 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 <[email protected]> a while ago)
H. Nikolaus Schaller (1):
MIPS: defconfig: CI20: configure for DRM_DW_HDMI_JZ4780
Paul Boddie (5):
drm/ingenic: Fix drm_init error path if IPU was registered
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
.../bindings/display/ingenic-jz4780-hdmi.yaml | 79 +++++++++++
arch/mips/boot/dts/ingenic/ci20.dts | 67 ++++++++++
arch/mips/boot/dts/ingenic/jz4780.dtsi | 45 +++++++
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 | 96 +++++++++++++-
drivers/gpu/drm/ingenic/ingenic-drm.h | 42 ++++++
drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 125 ++++++++++++++++++
9 files changed, 464 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-hdmi.yaml
create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
--
2.33.0
From: Paul Boddie <[email protected]>
If ingenic drm driver can not be registered, the IPU driver won't be
deregistered.
Code structure is chosen in preparation to add hdmi unregistration
in error case following the same pattern by a later patch.
Signed-off-by: Paul Boddie <[email protected]>
Signed-off-by: H. Nikolaus Schaller <[email protected]>
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 35b61657d9f6..f73522bdacaa 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1498,7 +1498,16 @@ static int ingenic_drm_init(void)
return err;
}
- return platform_driver_register(&ingenic_drm_driver);
+ err = platform_driver_register(&ingenic_drm_driver);
+ if (err)
+ goto err_ipu_unreg;
+
+ return 0;
+
+err_ipu_unreg:
+ if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU))
+ platform_driver_unregister(ingenic_ipu_driver_ptr);
+ return err;
}
module_init(ingenic_drm_init);
--
2.33.0
From: Paul Boddie <[email protected]>
We need to hook up
* HDMI connector
* HDMI power regulator
* DDC pinmux
* HDMI and LCDC endpoint connections
Signed-off-by: Paul Boddie <[email protected]>
Signed-off-by: H. Nikolaus Schaller <[email protected]>
---
arch/mips/boot/dts/ingenic/ci20.dts | 67 +++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index a688809beebc..4776be35b14d 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 GPIO_ACTIVE_LOW>;
+ enable-active-high;
+ };
};
&ext {
@@ -506,6 +529,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";
@@ -536,3 +565,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>;
+ };
+ };
+};
--
2.33.0
Enable CONFIG options as modules.
Signed-off-by: Ezequiel Garcia <[email protected]>
Signed-off-by: H. Nikolaus Schaller <[email protected]>
---
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 ab7ebb066834..9c9c649d385b 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=y
+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
--
2.33.0
From: Paul Boddie <[email protected]>
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.
Signed-off-by: Paul Boddie <[email protected]>
Signed-off-by: H. Nikolaus Schaller <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 45 ++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 9e34f433b9b5..c3c18a59c377 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -424,6 +424,51 @@ 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";
+
+ assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
+ assigned-clock-rates = <27000000>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <3>;
+
+ /* ddc-i2c-bus = <&i2c4>; */
+
+ 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 = <31>;
+
+ status = "disabled";
+ };
+
nemc: nemc@13410000 {
compatible = "ingenic,jz4780-nemc", "simple-mfd";
reg = <0x13410000 0x10000>;
--
2.33.0
Hi Nikolaus & Paul,
Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller
<[email protected]> a ?crit :
> From: Paul Boddie <[email protected]>
>
> 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.
>
> Signed-off-by: Paul Boddie <[email protected]>
> Signed-off-by: H. Nikolaus Schaller <[email protected]>
> ---
> arch/mips/boot/dts/ingenic/jz4780.dtsi | 45
> ++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index 9e34f433b9b5..c3c18a59c377 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -424,6 +424,51 @@ 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";
> +
> + assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
> + assigned-clock-rates = <27000000>;
Any reason why this is set to 27 MHz? Is it even required? Because with
the current ci20.dts, it won't be clocked at anything but 48 MHz.
> +
> + interrupt-parent = <&intc>;
> + interrupts = <3>;
> +
> + /* ddc-i2c-bus = <&i2c4>; */
> +
> + 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";
I think you can keep lcdc0 enabled by default (not lcdc1 though), since
it is highly likely that you'd want that.
Cheers,
-Paul
> + };
> +
> + 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 = <31>;
> +
> + status = "disabled";
> + };
> +
> nemc: nemc@13410000 {
> compatible = "ingenic,jz4780-nemc", "simple-mfd";
> reg = <0x13410000 0x10000>;
> --
> 2.33.0
>
On Tuesday, 5 October 2021 22:50:12 CEST Paul Cercueil wrote:
> Hi Nikolaus & Paul,
>
> Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller
<[email protected]> a ?crit :
> >
> > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > index 9e34f433b9b5..c3c18a59c377 100644
> > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > @@ -424,6 +424,51 @@ 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";
> > +
> > + assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
> > + assigned-clock-rates = <27000000>;
>
> Any reason why this is set to 27 MHz? Is it even required? Because with
> the current ci20.dts, it won't be clocked at anything but 48 MHz.
EXCLK will be 48MHz, but the aim is to set the HDMI peripheral clock to 27MHz,
which is supposedly required. I vaguely recall a conversation about whether we
were doing this right, but I don't recall any conclusion.
> > +
> > + interrupt-parent = <&intc>;
> > + interrupts = <3>;
> > +
> > + /* ddc-i2c-bus = <&i2c4>; */
> > +
> > + 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";
>
> I think you can keep lcdc0 enabled by default (not lcdc1 though), since
> it is highly likely that you'd want that.
As far as I know, the clock gating for the LCD controllers acts like a series
circuit, meaning that they both need to be enabled. Some testing seemed to
confirm this. Indeed, I seem to remember only enabling one clock and not
getting any output until I figured this weird arrangement out.
Paul
Hi Paul,
Le mar., oct. 5 2021 at 23:44:12 +0200, Paul Boddie
<[email protected]> a ?crit :
> On Tuesday, 5 October 2021 22:50:12 CEST Paul Cercueil wrote:
>> Hi Nikolaus & Paul,
>>
>> Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller
> <[email protected]> a ?crit :
>> >
>> > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> > b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> > index 9e34f433b9b5..c3c18a59c377 100644
>> > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> > @@ -424,6 +424,51 @@ 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";
>> > +
>> > + assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
>> > + assigned-clock-rates = <27000000>;
>>
>> Any reason why this is set to 27 MHz? Is it even required? Because
>> with
>> the current ci20.dts, it won't be clocked at anything but 48 MHz.
>
> EXCLK will be 48MHz, but the aim is to set the HDMI peripheral clock
> to 27MHz,
> which is supposedly required. I vaguely recall a conversation about
> whether we
> were doing this right, but I don't recall any conclusion.
But right now your HDMI clock is 48 MHz and HDMI works.
>> > +
>> > + interrupt-parent = <&intc>;
>> > + interrupts = <3>;
>> > +
>> > + /* ddc-i2c-bus = <&i2c4>; */
>> > +
>> > + 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";
>>
>> I think you can keep lcdc0 enabled by default (not lcdc1 though),
>> since
>> it is highly likely that you'd want that.
>
> As far as I know, the clock gating for the LCD controllers acts like
> a series
> circuit, meaning that they both need to be enabled. Some testing
> seemed to
> confirm this. Indeed, I seem to remember only enabling one clock and
> not
> getting any output until I figured this weird arrangement out.
I'm not talking about clocks though, but about LCDC0 and LCDC1.
Cheers,
-Paul
Hi Paul,
> Am 05.10.2021 um 23:52 schrieb Paul Cercueil <[email protected]>:
>
> Hi Paul,
>
> Le mar., oct. 5 2021 at 23:44:12 +0200, Paul Boddie <[email protected]> a écrit :
>> On Tuesday, 5 October 2021 22:50:12 CEST Paul Cercueil wrote:
>>> Hi Nikolaus & Paul,
>>> Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller
>> <[email protected]> a écrit :
>>> >
>>> > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> > b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> > index 9e34f433b9b5..c3c18a59c377 100644
>>> > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> > @@ -424,6 +424,51 @@ 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";
>>> > +
>>> > + assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
>>> > + assigned-clock-rates = <27000000>;
>>> Any reason why this is set to 27 MHz? Is it even required? Because with
>>> the current ci20.dts, it won't be clocked at anything but 48 MHz.
>> EXCLK will be 48MHz, but the aim is to set the HDMI peripheral clock to 27MHz,
>> which is supposedly required. I vaguely recall a conversation about whether we
>> were doing this right, but I don't recall any conclusion.
>
> But right now your HDMI clock is 48 MHz and HDMI works.
Is it? How did you find out?
And have you tried to remove assigned-clocks from jz4780.dtsi?
1. I read back:
root@letux:~# cat /sys/kernel/debug/clk/hdmi/clk_rate
26909090
root@letux:~#
So for me it seems to be running at ~27 MHz.
2. If I remove the assigned-clocks or assigned-clock-rates from DT
the boot process hangs shortly after initializing drm.
3. If I set assigned-clock-rates = <48000000>, HDMI also works.
I get it read back from /sys/kernel/debug/clk/hdmi/clk_rate
of 46736842.
4. Conclusions:
* assigned-clocks are required
* it does not matter if 27 or 48 MHz
* I have no idea which value is more correct
* so I'd stay on the safe side of 27 MHz
5. But despite that found, please look into the programming
manual section 18.1.2.16. There is an
"Import Note: The clock must be between 18M and 27M, it occurs
fatal error if exceeding the range. "
6. Therefore I think it *may* work overclocked with 48MHz
but is not guaranteed or reliable above 27 MHz.
So everything is ok here.
>
>>> > +
>>> > + interrupt-parent = <&intc>;
>>> > + interrupts = <3>;
>>> > +
>>> > + /* ddc-i2c-bus = <&i2c4>; */
>>> > +
>>> > + 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";
>>> I think you can keep lcdc0 enabled by default (not lcdc1 though), since
>>> it is highly likely that you'd want that.
>> As far as I know, the clock gating for the LCD controllers acts like a series
>> circuit, meaning that they both need to be enabled. Some testing seemed to
>> confirm this. Indeed, I seem to remember only enabling one clock and not
>> getting any output until I figured this weird arrangement out.
>
> I'm not talking about clocks though, but about LCDC0 and LCDC1.
Ah, you mean status = "okay"; vs. status = "disabled";
Well, IMHO it is common practise to keep SoC subsystems disabled by
default (to save power and boot time) unless a board specific DTS explicitly
requests the SoC feature to be active. See for example mmc0, mmc1 or i2c0..i2c4.
All these are disabled in jz4780.dtsi and partially enabled in ci20.dts.
Why should lcdc0 be an exception in jz4780.dtsi?
BR and thanks,
Nikolaus
Hi,
Le dim., nov. 7 2021 at 14:45:37 +0100, H. Nikolaus Schaller
<[email protected]> a ?crit :
> Hi Paul,
>
>> Am 05.10.2021 um 23:52 schrieb Paul Cercueil <[email protected]>:
>>
>> Hi Paul,
>>
>> Le mar., oct. 5 2021 at 23:44:12 +0200, Paul Boddie
>> <[email protected]> a ?crit :
>>> On Tuesday, 5 October 2021 22:50:12 CEST Paul Cercueil wrote:
>>>> Hi Nikolaus & Paul,
>>>> Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller
>>> <[email protected]> a ?crit :
>>>> >
>>>> > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> > b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> > index 9e34f433b9b5..c3c18a59c377 100644
>>>> > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> > @@ -424,6 +424,51 @@ 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";
>>>> > +
>>>> > + assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
>>>> > + assigned-clock-rates = <27000000>;
>>>> Any reason why this is set to 27 MHz? Is it even required?
>>>> Because with
>>>> the current ci20.dts, it won't be clocked at anything but 48 MHz.
>>> EXCLK will be 48MHz, but the aim is to set the HDMI peripheral
>>> clock to 27MHz,
>>> which is supposedly required. I vaguely recall a conversation
>>> about whether we
>>> were doing this right, but I don't recall any conclusion.
>>
>> But right now your HDMI clock is 48 MHz and HDMI works.
>
> Is it? How did you find out?
>
> And have you tried to remove assigned-clocks from jz4780.dtsi?
>
> 1. I read back:
>
> root@letux:~# cat /sys/kernel/debug/clk/hdmi/clk_rate
> 26909090
> root@letux:~#
>
> So for me it seems to be running at ~27 MHz.
>
> 2. If I remove the assigned-clocks or assigned-clock-rates from DT
> the boot process hangs shortly after initializing drm.
>
> 3. If I set assigned-clock-rates = <48000000>, HDMI also works.
>
> I get it read back from /sys/kernel/debug/clk/hdmi/clk_rate
> of 46736842.
>
> 4. Conclusions:
> * assigned-clocks are required
> * it does not matter if 27 or 48 MHz
> * I have no idea which value is more correct
> * so I'd stay on the safe side of 27 MHz
>
> 5. But despite that found, please look into the programming
> manual section 18.1.2.16. There is an
>
> "Import Note: The clock must be between 18M and 27M, it occurs
> fatal error if exceeding the range. "
Ok, that's the important information that was missing.
So 27 MHz is OK.
> 6. Therefore I think it *may* work overclocked with 48MHz
> but is not guaranteed or reliable above 27 MHz.
>
> So everything is ok here.
One thing though - the "assigned-clocks" and "assigned-clock-rates",
while it works here, should be moved to the CGU node, to respect the
YAML schemas.
Cheers,
-Paul
>
>>
>>>> > +
>>>> > + interrupt-parent = <&intc>;
>>>> > + interrupts = <3>;
>>>> > +
>>>> > + /* ddc-i2c-bus = <&i2c4>; */
>>>> > +
>>>> > + 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";
>>>> I think you can keep lcdc0 enabled by default (not lcdc1 though),
>>>> since
>>>> it is highly likely that you'd want that.
>>> As far as I know, the clock gating for the LCD controllers acts
>>> like a series
>>> circuit, meaning that they both need to be enabled. Some testing
>>> seemed to
>>> confirm this. Indeed, I seem to remember only enabling one clock
>>> and not
>>> getting any output until I figured this weird arrangement out.
>>
>> I'm not talking about clocks though, but about LCDC0 and LCDC1.
>
> Ah, you mean status = "okay"; vs. status = "disabled";
>
> Well, IMHO it is common practise to keep SoC subsystems disabled by
> default (to save power and boot time) unless a board specific DTS
> explicitly
> requests the SoC feature to be active. See for example mmc0, mmc1 or
> i2c0..i2c4.
>
> All these are disabled in jz4780.dtsi and partially enabled in
> ci20.dts.
>
> Why should lcdc0 be an exception in jz4780.dtsi?
>
> BR and thanks,
> Nikolaus
>
Hi Paul,
> Am 07.11.2021 um 20:05 schrieb Paul Cercueil <[email protected]>:
>
>> 6. Therefore I think it *may* work overclocked with 48MHz
>> but is not guaranteed or reliable above 27 MHz.
>> So everything is ok here.
>
> One thing though - the "assigned-clocks" and "assigned-clock-rates", while it works here, should be moved to the CGU node, to respect the YAML schemas.
Trying to do this seems to break boot.
I can boot up to
[ 8.312926] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
and
[ 11.366899] [drm] Initialized ingenic-drm 1.1.0 20200716 for 13050000.lcdc0 on minor 0
but then the boot process becomes slow and hangs. Last sign of activity is
[ 19.347659] hub 1-0:1.0: USB hub found
[ 19.353478] hub 1-0:1.0: 1 port detected
[ 32.321760] wlan0_power: disabling
What I did was to just move
assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
assigned-clock-rates = <27000000>;
from
hdmi: hdmi@10180000 {
to
cgu: jz4780-cgu@10000000 {
Does this mean the clock is assigned too early or too late?
Do you have any suggestions since I don't know the details of CGU.
BR and thanks,
Nikolaus
Hi Nikolaus,
Le mar., nov. 9 2021 at 21:19:17 +0100, H. Nikolaus Schaller
<[email protected]> a ?crit :
> Hi Paul,
>
>> Am 07.11.2021 um 20:05 schrieb Paul Cercueil <[email protected]>:
>>
>>> 6. Therefore I think it *may* work overclocked with 48MHz
>>> but is not guaranteed or reliable above 27 MHz.
>>> So everything is ok here.
>>
>> One thing though - the "assigned-clocks" and
>> "assigned-clock-rates", while it works here, should be moved to the
>> CGU node, to respect the YAML schemas.
>
> Trying to do this seems to break boot.
>
> I can boot up to
>
> [ 8.312926] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare
> HDMI I2C bus driver
>
> and
>
> [ 11.366899] [drm] Initialized ingenic-drm 1.1.0 20200716 for
> 13050000.lcdc0 on minor 0
>
> but then the boot process becomes slow and hangs. Last sign of
> activity is
>
> [ 19.347659] hub 1-0:1.0: USB hub found
> [ 19.353478] hub 1-0:1.0: 1 port detected
> [ 32.321760] wlan0_power: disabling
>
> What I did was to just move
>
> assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
> assigned-clock-rates = <27000000>;
>
> from
>
> hdmi: hdmi@10180000 {
>
> to
>
> cgu: jz4780-cgu@10000000 {
>
> Does this mean the clock is assigned too early or too late?
>
> Do you have any suggestions since I don't know the details of CGU.
These properties are already set for the CGU node in ci20.dts:
&cgu {
/*
* Use the 32.768 kHz oscillator as the parent of the RTC for a higher
* precision.
*/
assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>;
assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>;
assigned-clock-rates = <48000000>;
};
So you want to update these properties to add the HDMI clock setting,
like this:
assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>,
<&cgu JZ4780_CLK_HDMI>;
assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>;
assigned-clock-rates = <48000000>, <0>, <27000000>;
Cheers,
-Paul
> Am 09.11.2021 um 21:36 schrieb Paul Cercueil <[email protected]>:
>
> Hi Nikolaus,
>
> Le mar., nov. 9 2021 at 21:19:17 +0100, H. Nikolaus Schaller <[email protected]> a écrit :
>> Hi Paul,
>>> Am 07.11.2021 um 20:05 schrieb Paul Cercueil <[email protected]>:
>>>> 6. Therefore I think it *may* work overclocked with 48MHz
>>>> but is not guaranteed or reliable above 27 MHz.
>>>> So everything is ok here.
>>> One thing though - the "assigned-clocks" and "assigned-clock-rates", while it works here, should be moved to the CGU node, to respect the YAML schemas.
>> Trying to do this seems to break boot.
>> I can boot up to
>> [ 8.312926] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
>> and
>> [ 11.366899] [drm] Initialized ingenic-drm 1.1.0 20200716 for 13050000.lcdc0 on minor 0
>> but then the boot process becomes slow and hangs. Last sign of activity is
>> [ 19.347659] hub 1-0:1.0: USB hub found
>> [ 19.353478] hub 1-0:1.0: 1 port detected
>> [ 32.321760] wlan0_power: disabling
>> What I did was to just move
>> assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
>> assigned-clock-rates = <27000000>;
>> from
>> hdmi: hdmi@10180000 {
>> to
>> cgu: jz4780-cgu@10000000 {
>> Does this mean the clock is assigned too early or too late?
>> Do you have any suggestions since I don't know the details of CGU.
>
> These properties are already set for the CGU node in ci20.dts:
Ah, I didn't look into that. Maybe because I thought adding this should stay in jz4780.dtsi to be available for any board making use of it.
So it gets overwritten and is then completely missing.
>
> &cgu {
> /*
> * Use the 32.768 kHz oscillator as the parent of the RTC for a higher
> * precision.
> */
> assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>;
> assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>;
> assigned-clock-rates = <48000000>;
> };
>
> So you want to update these properties to add the HDMI clock setting, like this:
>
> assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>, <&cgu JZ4780_CLK_HDMI>;
> assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>;
> assigned-clock-rates = <48000000>, <0>, <27000000>;
Will give it a try.
I would prefer if it could sit in jz4780.dtsi and ci20.dts would just extend it but IMHO this is beyond DTS capabilities.
So we likely have to live with that.
BR and thanks,
Nikolaus
> Am 09.11.2021 um 21:42 schrieb H. Nikolaus Schaller <[email protected]>:
>
>> So you want to update these properties to add the HDMI clock setting, like this:
>>
>> assigned-clocks = <&cgu JZ4780_CLK_OTGPHY>, <&cgu JZ4780_CLK_RTC>, <&cgu JZ4780_CLK_HDMI>;
>> assigned-clock-parents = <0>, <&cgu JZ4780_CLK_RTCLK>;
>> assigned-clock-rates = <48000000>, <0>, <27000000>;
>
> Will give it a try.
Yes, works. So v6 is not far away.
BR,
Nikolaus