| Message ID | cfce2276d172d3d9c4d34d966b58fd47f77c4e46.1599120059.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Support BCM2711 Display Pipeline | expand |
Hi Maxime, On 9/3/20 5:01 PM, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> Thanks for your v5 patch Reviewed-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> Best regards, Hoegeun > --- > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 48 +++++++++++- > arch/arm/boot/dts/bcm2711.dtsi | 122 ++++++++++++++++++++++++++- > 2 files changed, 169 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > index e94244a215af..09a1182c2936 100644 > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > @@ -70,6 +70,14 @@ > }; > }; > > +&ddc0 { > + status = "okay"; > +}; > + > +&ddc1 { > + status = "okay"; > +}; > + > &firmware { > firmware_clocks: clocks { > compatible = "raspberrypi,firmware-clocks"; > @@ -170,6 +178,38 @@ > "RGMII_TXD3"; > }; > > +&hdmi0 { > + clocks = <&firmware_clocks 13>, <&firmware_clocks 14>, <&dvp 0>, <&clk_27MHz>; > + clock-names = "hdmi", "bvb", "audio", "cec"; > + status = "okay"; > +}; > + > +&hdmi1 { > + clocks = <&firmware_clocks 13>, <&firmware_clocks 14>, <&dvp 1>, <&clk_27MHz>; > + clock-names = "hdmi", "bvb", "audio", "cec"; > + status = "okay"; > +}; > + > +&hvs { > + clocks = <&firmware_clocks 4>; > +}; > + > +&pixelvalve0 { > + status = "okay"; > +}; > + > +&pixelvalve1 { > + status = "okay"; > +}; > + > +&pixelvalve2 { > + status = "okay"; > +}; > + > +&pixelvalve4 { > + status = "okay"; > +}; > + > &pwm1 { > pinctrl-names = "default"; > pinctrl-0 = <&pwm1_0_gpio40 &pwm1_1_gpio41>; > @@ -253,3 +293,11 @@ > &vchiq { > interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>; > }; > + > +&vc4 { > + status = "okay"; > +}; > + > +&vec { > + status = "disabled"; > +}; > diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi > index 00bcaed1be32..4847dd305317 100644 > --- a/arch/arm/boot/dts/bcm2711.dtsi > +++ b/arch/arm/boot/dts/bcm2711.dtsi > @@ -12,6 +12,18 @@ > > interrupt-parent = <&gicv2>; > > + vc4: gpu { > + compatible = "brcm,bcm2711-vc5"; > + status = "disabled"; > + }; > + > + clk_27MHz: clk-27M { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <27000000>; > + clock-output-names = "27MHz-clock"; > + }; > + > clk_108MHz: clk-108M { > #clock-cells = <0>; > compatible = "fixed-clock"; > @@ -238,6 +250,27 @@ > status = "disabled"; > }; > > + pixelvalve0: pixelvalve@7e206000 { > + compatible = "brcm,bcm2711-pixelvalve0"; > + reg = <0x7e206000 0x100>; > + interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > + pixelvalve1: pixelvalve@7e207000 { > + compatible = "brcm,bcm2711-pixelvalve1"; > + reg = <0x7e207000 0x100>; > + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > + pixelvalve2: pixelvalve@7e20a000 { > + compatible = "brcm,bcm2711-pixelvalve2"; > + reg = <0x7e20a000 0x100>; > + interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > pwm1: pwm@7e20c800 { > compatible = "brcm,bcm2835-pwm"; > reg = <0x7e20c800 0x28>; > @@ -248,10 +281,25 @@ > status = "disabled"; > }; > > - hvs@7e400000 { > + pixelvalve4: pixelvalve@7e216000 { > + compatible = "brcm,bcm2711-pixelvalve4"; > + reg = <0x7e216000 0x100>; > + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > + hvs: hvs@7e400000 { > + compatible = "brcm,bcm2711-hvs"; > interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>; > }; > > + pixelvalve3: pixelvalve@7ec12000 { > + compatible = "brcm,bcm2711-pixelvalve3"; > + reg = <0x7ec12000 0x100>; > + interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > dvp: clock@7ef00000 { > compatible = "brcm,brcm2711-dvp"; > reg = <0x7ef00000 0x10>; > @@ -259,6 +307,78 @@ > #clock-cells = <1>; > #reset-cells = <1>; > }; > + > + hdmi0: hdmi@7ef00700 { > + compatible = "brcm,bcm2711-hdmi0"; > + reg = <0x7ef00700 0x300>, > + <0x7ef00300 0x200>, > + <0x7ef00f00 0x80>, > + <0x7ef00f80 0x80>, > + <0x7ef01b00 0x200>, > + <0x7ef01f00 0x400>, > + <0x7ef00200 0x80>, > + <0x7ef04300 0x100>, > + <0x7ef20000 0x100>; > + reg-names = "hdmi", > + "dvp", > + "phy", > + "rm", > + "packet", > + "metadata", > + "csc", > + "cec", > + "hd"; > + clock-names = "hdmi", "bvb", "audio", "cec"; > + resets = <&dvp 0>; > + ddc = <&ddc0>; > + dmas = <&dma 10>; > + dma-names = "audio-rx"; > + status = "disabled"; > + }; > + > + ddc0: i2c@7ef04500 { > + compatible = "brcm,bcm2711-hdmi-i2c"; > + reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>; > + reg-names = "bsc", "auto-i2c"; > + clock-frequency = <97500>; > + status = "disabled"; > + }; > + > + hdmi1: hdmi@7ef05700 { > + compatible = "brcm,bcm2711-hdmi1"; > + reg = <0x7ef05700 0x300>, > + <0x7ef05300 0x200>, > + <0x7ef05f00 0x80>, > + <0x7ef05f80 0x80>, > + <0x7ef06b00 0x200>, > + <0x7ef06f00 0x400>, > + <0x7ef00280 0x80>, > + <0x7ef09300 0x100>, > + <0x7ef20000 0x100>; > + reg-names = "hdmi", > + "dvp", > + "phy", > + "rm", > + "packet", > + "metadata", > + "csc", > + "cec", > + "hd"; > + ddc = <&ddc1>; > + clock-names = "hdmi", "bvb", "audio", "cec"; > + resets = <&dvp 1>; > + dmas = <&dma 17>; > + dma-names = "audio-rx"; > + status = "disabled"; > + }; > + > + ddc1: i2c@7ef09500 { > + compatible = "brcm,bcm2711-hdmi-i2c"; > + reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>; > + reg-names = "bsc", "auto-i2c"; > + clock-frequency = <97500>; > + status = "disabled"; > + }; > }; > > /*
On Thu, 2020-09-03 at 10:01 +0200, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Applied for-next. Thanks! Nicolas
On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> Apologies if this has already been reported or have a solution but this patch (and presumably series) breaks output to the serial console after a certain point during init. On Raspbian, I see systemd startup messages then the output just turns into complete garbage. It looks like this patch is merged first in linux-next, which is why my bisect fell on the DRM merge. I am happy to provide whatever information could be helpful for debugging this. I am on the latest version of the firmware (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). $ git bisect log # bad: [49e7e3e905e437a02782019570f70997e2da9101] Add linux-next specific files for 20200929 # good: [fb0155a09b0224a7147cb07a4ce6034c8d29667f] Merge tag 'nfs-for-5.9-3' of git://git.linux-nfs.org/projects/trondmy/linux-nfs git bisect start '49e7e3e905e437a02782019570f70997e2da9101' 'fb0155a09b0224a7147cb07a4ce6034c8d29667f' # good: [a07bf9042465c0e4ab28947daf70517f99ef021f] Merge remote-tracking branch 'bluetooth/master' into master git bisect good a07bf9042465c0e4ab28947daf70517f99ef021f # bad: [546d06883722dfc5823d53042b924f4b4f76a459] Merge remote-tracking branch 'spi/for-next' into master git bisect bad 546d06883722dfc5823d53042b924f4b4f76a459 # good: [06c14f5c2d311100a447caf60ecbcf558e4e60fe] Merge tag 'mediatek-drm-next-5.10' of https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux into drm-next git bisect good 06c14f5c2d311100a447caf60ecbcf558e4e60fe # bad: [606865c11f2ed6429c7eddcbc59d3295771d41a4] Merge remote-tracking branch 'sound-asoc/for-next' into master git bisect bad 606865c11f2ed6429c7eddcbc59d3295771d41a4 # bad: [7492f2f52acc72a1d08ad1f1d722237ba66189b9] Merge remote-tracking branch 'regmap/for-next' into master git bisect bad 7492f2f52acc72a1d08ad1f1d722237ba66189b9 # good: [0b7e44d39c8aa7536352b57af2265e92fc253e4f] integrity: Asymmetric digsig supports SM2-with-SM3 algorithm git bisect good 0b7e44d39c8aa7536352b57af2265e92fc253e4f # good: [2ce595ba1cd7a2bc053fcc937b7bbbf743c21818] Merge remote-tracking branch 'nand/nand/next' into master git bisect good 2ce595ba1cd7a2bc053fcc937b7bbbf743c21818 # good: [10e07ca312548f90d5e0fc1d862209285c9a858c] gpu/drm/radeon: fix spelling typo in comments git bisect good 10e07ca312548f90d5e0fc1d862209285c9a858c # bad: [be877462417f05af69729a9eeda1332b15e81de8] Merge remote-tracking branch 'imx-drm/imx-drm/next' into master git bisect bad be877462417f05af69729a9eeda1332b15e81de8 # bad: [7a80fa2ada067aa633a91bce67f6c3e39aad7504] Merge remote-tracking branch 'drm-intel/for-linux-next' into master git bisect bad 7a80fa2ada067aa633a91bce67f6c3e39aad7504 # good: [f4a336053725b689a65021edca26f8d058c0d6b4] drm/amdgpu/display: fix CFLAGS setup for DCN30 git bisect good f4a336053725b689a65021edca26f8d058c0d6b4 # bad: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master git bisect bad 64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba # good: [a4b0c1f80de8ec3f473b02918556650b683f044d] Merge remote-tracking branch 'crypto/master' into master git bisect good a4b0c1f80de8ec3f473b02918556650b683f044d # first bad commit: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master Cheers, Nathan
Hi Nathan, On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > Now that all the drivers have been adjusted for it, let's bring in the > > necessary device tree changes. > > > > The VEC and PV3 are left out for now, since it will require a more specific > > clock setup. > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > Apologies if this has already been reported or have a solution but this > patch (and presumably series) breaks output to the serial console after > a certain point during init. On Raspbian, I see systemd startup messages > then the output just turns into complete garbage. It looks like this > patch is merged first in linux-next, which is why my bisect fell on the > DRM merge. I am happy to provide whatever information could be helpful > for debugging this. I am on the latest version of the firmware > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). Unfortunately, the miniUART is in the same clock tree than the core clock and will thus have those kind of issues when the core clock is changed (which is also something that one should expect when using the DRM or other drivers). The only real workaround there would be to switch to one of the PL011 UARTs. I guess we can also somehow make the UART react to the core clock frequency changes, but that's going to require some effort Maxime
On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > Hi Nathan, > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > Now that all the drivers have been adjusted for it, let's bring in the > > > necessary device tree changes. > > > > > > The VEC and PV3 are left out for now, since it will require a more specific > > > clock setup. > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > > Apologies if this has already been reported or have a solution but this > > patch (and presumably series) breaks output to the serial console after > > a certain point during init. On Raspbian, I see systemd startup messages > > then the output just turns into complete garbage. It looks like this > > patch is merged first in linux-next, which is why my bisect fell on the > > DRM merge. I am happy to provide whatever information could be helpful > > for debugging this. I am on the latest version of the firmware > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > Unfortunately, the miniUART is in the same clock tree than the core > clock and will thus have those kind of issues when the core clock is > changed (which is also something that one should expect when using the > DRM or other drivers). > > The only real workaround there would be to switch to one of the PL011 > UARTs. I guess we can also somehow make the UART react to the core clock > frequency changes, but that's going to require some effort > > Maxime Ack, thank you for the reply! There does not really seem to be a whole ton of documentation around using one of the other PL011 UARTs so for now, I will just revert this commit locally. Cheers, Nathan
Hi, Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: >> Hi Nathan, >> >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: >>>> Now that all the drivers have been adjusted for it, let's bring in the >>>> necessary device tree changes. >>>> >>>> The VEC and PV3 are left out for now, since it will require a more specific >>>> clock setup. >>>> >>>> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> >>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com> >>>> Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> >>>> Tested-by: Stefan Wahren <stefan.wahren@i2se.com> >>>> Signed-off-by: Maxime Ripard <maxime@cerno.tech> >>> Apologies if this has already been reported or have a solution but this >>> patch (and presumably series) breaks output to the serial console after >>> a certain point during init. On Raspbian, I see systemd startup messages >>> then the output just turns into complete garbage. It looks like this >>> patch is merged first in linux-next, which is why my bisect fell on the >>> DRM merge. I am happy to provide whatever information could be helpful >>> for debugging this. I am on the latest version of the firmware >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). >> Unfortunately, the miniUART is in the same clock tree than the core >> clock and will thus have those kind of issues when the core clock is >> changed (which is also something that one should expect when using the >> DRM or other drivers). >> >> The only real workaround there would be to switch to one of the PL011 >> UARTs. I guess we can also somehow make the UART react to the core clock >> frequency changes, but that's going to require some effort >> >> Maxime > Ack, thank you for the reply! There does not really seem to be a whole > ton of documentation around using one of the other PL011 UARTs so for > now, I will just revert this commit locally. there was a patch series & discussion about this topic, but we finally didn't find a rock solid solution. You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier to follow clock changes" from 3.4.2019 on linux-rpi-kernel. Best regards > > Cheers, > Nathan
Hi Stefan, On Wed, Sep 30, 2020 at 06:52:13PM +0200, Stefan Wahren wrote: > Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > >> Hi Nathan, > >> > >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > >>>> Now that all the drivers have been adjusted for it, let's bring in the > >>>> necessary device tree changes. > >>>> > >>>> The VEC and PV3 are left out for now, since it will require a more specific > >>>> clock setup. > >>>> > >>>> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > >>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > >>>> Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > >>>> Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > >>>> Signed-off-by: Maxime Ripard <maxime@cerno.tech> > >>> Apologies if this has already been reported or have a solution but this > >>> patch (and presumably series) breaks output to the serial console after > >>> a certain point during init. On Raspbian, I see systemd startup messages > >>> then the output just turns into complete garbage. It looks like this > >>> patch is merged first in linux-next, which is why my bisect fell on the > >>> DRM merge. I am happy to provide whatever information could be helpful > >>> for debugging this. I am on the latest version of the firmware > >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > >> Unfortunately, the miniUART is in the same clock tree than the core > >> clock and will thus have those kind of issues when the core clock is > >> changed (which is also something that one should expect when using the > >> DRM or other drivers). > >> > >> The only real workaround there would be to switch to one of the PL011 > >> UARTs. I guess we can also somehow make the UART react to the core clock > >> frequency changes, but that's going to require some effort > >> > >> Maxime > > Ack, thank you for the reply! There does not really seem to be a whole > > ton of documentation around using one of the other PL011 UARTs so for > > now, I will just revert this commit locally. > > there was a patch series & discussion about this topic, but we finally > didn't find a rock solid solution. > > You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier > to follow clock changes" from 3.4.2019 on linux-rpi-kernel. I couldn't find that discussion on the archive, but based on the title I guess there's some patches that have been merged this cycle for the 8250 driver to do just that (868f3ee6e452 ("serial: 8250: Add 8250 port clock update method") and cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race condition")). However, I'm not entirely sure the clock notifier works in our case with the firmware / MMIO clocks duality Maxime
On Thu, Oct 01, 2020 at 08:48:43AM +0200, Maxime Ripard wrote: > Hi Stefan, > > On Wed, Sep 30, 2020 at 06:52:13PM +0200, Stefan Wahren wrote: > > Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > >> Hi Nathan, > > >> > > >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > >>>> Now that all the drivers have been adjusted for it, let's bring in the > > >>>> necessary device tree changes. > > >>>> > > >>>> The VEC and PV3 are left out for now, since it will require a more specific > > >>>> clock setup. > > >>>> > > >>>> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > >>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > >>>> Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > >>>> Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > >>>> Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > >>> Apologies if this has already been reported or have a solution but this > > >>> patch (and presumably series) breaks output to the serial console after > > >>> a certain point during init. On Raspbian, I see systemd startup messages > > >>> then the output just turns into complete garbage. It looks like this > > >>> patch is merged first in linux-next, which is why my bisect fell on the > > >>> DRM merge. I am happy to provide whatever information could be helpful > > >>> for debugging this. I am on the latest version of the firmware > > >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > >> Unfortunately, the miniUART is in the same clock tree than the core > > >> clock and will thus have those kind of issues when the core clock is > > >> changed (which is also something that one should expect when using the > > >> DRM or other drivers). > > >> > > >> The only real workaround there would be to switch to one of the PL011 > > >> UARTs. I guess we can also somehow make the UART react to the core clock > > >> frequency changes, but that's going to require some effort > > >> > > >> Maxime > > > Ack, thank you for the reply! There does not really seem to be a whole > > > ton of documentation around using one of the other PL011 UARTs so for > > > now, I will just revert this commit locally. > > > > there was a patch series & discussion about this topic, but we finally > > didn't find a rock solid solution. > > > > You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier > > to follow clock changes" from 3.4.2019 on linux-rpi-kernel. > > I couldn't find that discussion on the archive, but based on the title I > guess there's some patches that have been merged this cycle for the 8250 > driver to do just that (868f3ee6e452 ("serial: 8250: Add 8250 port clock > update method") and cc816969d7b5 ("serial: 8250_dw: Fix common clocks > usage race condition")). > > However, I'm not entirely sure the clock notifier works in our case with > the firmware / MMIO clocks duality I was a bit intrigued by this, so I looked into it, and it seems that it's worth that it used to be. The core clock is supposed to be running at 500 Mhz in most cases, and that's what we're setting so it shouldn't cause any pratical issue. However, it looks like on my board now the firmware reports that the core clock is running at either 311MHz or 233MHz with hdmi_enable_4k60 (which seems odd?) and that contradicts the documentation here: https://www.raspberrypi.org/documentation/configuration/config-txt/overclocking.md Linux then comes in, changes the frequency to 500MHz and breaks the UART. So either the doc is wrong, or the clock driver is. vcgencmd measure_clock core reports that it's indeed 233Mhz and I'm running a year-old firmware (built on the 2019-11-29), so I'd be inclined to think that the doc is wrong here or we're misinterpreting something. Dave, Tim, any idea? Thanks! Maxime
On Wed, 2020-09-30 at 09:38 -0700, Nathan Chancellor wrote: > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > Hi Nathan, > > > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > > Now that all the drivers have been adjusted for it, let's bring in the > > > > necessary device tree changes. > > > > > > > > The VEC and PV3 are left out for now, since it will require a more specific > > > > clock setup. > > > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > > > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > > > > Apologies if this has already been reported or have a solution but this > > > patch (and presumably series) breaks output to the serial console after > > > a certain point during init. On Raspbian, I see systemd startup messages > > > then the output just turns into complete garbage. It looks like this > > > patch is merged first in linux-next, which is why my bisect fell on the > > > DRM merge. I am happy to provide whatever information could be helpful > > > for debugging this. I am on the latest version of the firmware > > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > > > Unfortunately, the miniUART is in the same clock tree than the core > > clock and will thus have those kind of issues when the core clock is > > changed (which is also something that one should expect when using the > > DRM or other drivers). > > > > The only real workaround there would be to switch to one of the PL011 > > UARTs. I guess we can also somehow make the UART react to the core clock > > frequency changes, but that's going to require some effort > > > > Maxime > > Ack, thank you for the reply! There does not really seem to be a whole > ton of documentation around using one of the other PL011 UARTs so for > now, I will just revert this commit locally. Nathan, a less intrusive solution would be to add 'core_freq_min=500' into your config.txt. Funnily enough core_freq=500 doesn't seem to work in that regard. It might be related with what Maxime is commenting. Regards, Nicolas
On Thu, Oct 01, 2020 at 11:22:03AM +0200, Nicolas Saenz Julienne wrote: > On Wed, 2020-09-30 at 09:38 -0700, Nathan Chancellor wrote: > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > > Hi Nathan, > > > > > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > > > Now that all the drivers have been adjusted for it, let's bring in the > > > > > necessary device tree changes. > > > > > > > > > > The VEC and PV3 are left out for now, since it will require a more specific > > > > > clock setup. > > > > > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > > > > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > > > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > > > > > > Apologies if this has already been reported or have a solution but this > > > > patch (and presumably series) breaks output to the serial console after > > > > a certain point during init. On Raspbian, I see systemd startup messages > > > > then the output just turns into complete garbage. It looks like this > > > > patch is merged first in linux-next, which is why my bisect fell on the > > > > DRM merge. I am happy to provide whatever information could be helpful > > > > for debugging this. I am on the latest version of the firmware > > > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > > > > > Unfortunately, the miniUART is in the same clock tree than the core > > > clock and will thus have those kind of issues when the core clock is > > > changed (which is also something that one should expect when using the > > > DRM or other drivers). > > > > > > The only real workaround there would be to switch to one of the PL011 > > > UARTs. I guess we can also somehow make the UART react to the core clock > > > frequency changes, but that's going to require some effort > > > > > > Maxime > > > > Ack, thank you for the reply! There does not really seem to be a whole > > ton of documentation around using one of the other PL011 UARTs so for > > now, I will just revert this commit locally. > > Nathan, a less intrusive solution would be to add 'core_freq_min=500' into your > config.txt. > > Funnily enough core_freq=500 doesn't seem to work in that regard. It might be > related with what Maxime is commenting. Yeah, it fixes it here too Maxime
hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC VCO to support a core-frequency of 550 MHz which is the minimum frequency required by the HVS at 4Kp60. The side effect is that if the display clock requirements are lower than 4Kp60 then you will see different core frequencies selected by DVFS. If enable_uart=1 and the mini-uart is selected (default unless bluetooth is disabled) then the firmware will pin the core-frequency to either core_freq max (500 or 550). Although, I think there is a way of pinning it to a lower fixed frequency. The table in overclocking.md defines options for setting the maximum core frequency but unless core_freq_min is specified DVFS will automatically pick the lowest idle frequency required by the display resolution. On Thu, 1 Oct 2020 at 09:54, Maxime Ripard <maxime@cerno.tech> wrote: > > On Thu, Oct 01, 2020 at 08:48:43AM +0200, Maxime Ripard wrote: > > Hi Stefan, > > > > On Wed, Sep 30, 2020 at 06:52:13PM +0200, Stefan Wahren wrote: > > > Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > > > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > > >> Hi Nathan, > > > >> > > > >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > > >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > >>>> Now that all the drivers have been adjusted for it, let's bring in the > > > >>>> necessary device tree changes. > > > >>>> > > > >>>> The VEC and PV3 are left out for now, since it will require a more specific > > > >>>> clock setup. > > > >>>> > > > >>>> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > >>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > > >>>> Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > > >>>> Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > > >>>> Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > >>> Apologies if this has already been reported or have a solution but this > > > >>> patch (and presumably series) breaks output to the serial console after > > > >>> a certain point during init. On Raspbian, I see systemd startup messages > > > >>> then the output just turns into complete garbage. It looks like this > > > >>> patch is merged first in linux-next, which is why my bisect fell on the > > > >>> DRM merge. I am happy to provide whatever information could be helpful > > > >>> for debugging this. I am on the latest version of the firmware > > > >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > > >> Unfortunately, the miniUART is in the same clock tree than the core > > > >> clock and will thus have those kind of issues when the core clock is > > > >> changed (which is also something that one should expect when using the > > > >> DRM or other drivers). > > > >> > > > >> The only real workaround there would be to switch to one of the PL011 > > > >> UARTs. I guess we can also somehow make the UART react to the core clock > > > >> frequency changes, but that's going to require some effort > > > >> > > > >> Maxime > > > > Ack, thank you for the reply! There does not really seem to be a whole > > > > ton of documentation around using one of the other PL011 UARTs so for > > > > now, I will just revert this commit locally. > > > > > > there was a patch series & discussion about this topic, but we finally > > > didn't find a rock solid solution. > > > > > > You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier > > > to follow clock changes" from 3.4.2019 on linux-rpi-kernel. > > > > I couldn't find that discussion on the archive, but based on the title I > > guess there's some patches that have been merged this cycle for the 8250 > > driver to do just that (868f3ee6e452 ("serial: 8250: Add 8250 port clock > > update method") and cc816969d7b5 ("serial: 8250_dw: Fix common clocks > > usage race condition")). > > > > However, I'm not entirely sure the clock notifier works in our case with > > the firmware / MMIO clocks duality > > I was a bit intrigued by this, so I looked into it, and it seems that > it's worth that it used to be. The core clock is supposed to be running > at 500 Mhz in most cases, and that's what we're setting so it shouldn't > cause any pratical issue. > > However, it looks like on my board now the firmware reports that the > core clock is running at either 311MHz or 233MHz with hdmi_enable_4k60 > (which seems odd?) and that contradicts the documentation here: > https://www.raspberrypi.org/documentation/configuration/config-txt/overclocking.md > > Linux then comes in, changes the frequency to 500MHz and breaks the > UART. So either the doc is wrong, or the clock driver is. > > vcgencmd measure_clock core reports that it's indeed 233Mhz and I'm > running a year-old firmware (built on the 2019-11-29), so I'd be > inclined to think that the doc is wrong here or we're misinterpreting > something. > > Dave, Tim, any idea? > > Thanks! > Maxime
Hi Tim, thanks for the info! On Thu, 2020-10-01 at 11:15 +0100, Tim Gover wrote: > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > VCO to support a core-frequency of 550 MHz which is the minimum > frequency required by the HVS at 4Kp60. The side effect is that if the > display clock requirements are lower than 4Kp60 then you will see > different core frequencies selected by DVFS. > > If enable_uart=1 and the mini-uart is selected (default unless What is the actual test made to check if mini-uart is selected? I can't get firmware to trigger this behaviour with 64-bit upstream kernel/dts. Note that I see the core clk setup at 200MHz just before having VC4 set it to 500MHz. The only thing I've got on my config.txt is: enable_uart=1 arm_64bit=1 Maybe we're missing some kind of DT alias upstream? Regards, Nicolas > bluetooth is disabled) then the firmware will pin the core-frequency > to either core_freq max (500 or 550). Although, I think there is a way > of pinning it to a lower fixed frequency. > > The table in overclocking.md defines options for setting the maximum > core frequency but unless core_freq_min is specified DVFS will > automatically pick the lowest idle frequency required by the display > resolution.
On Thu, Oct 01, 2020 at 11:22:03AM +0200, Nicolas Saenz Julienne wrote: > On Wed, 2020-09-30 at 09:38 -0700, Nathan Chancellor wrote: > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > > Hi Nathan, > > > > > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > > > Now that all the drivers have been adjusted for it, let's bring in the > > > > > necessary device tree changes. > > > > > > > > > > The VEC and PV3 are left out for now, since it will require a more specific > > > > > clock setup. > > > > > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > > > > > Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > > > > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > > > > > > Apologies if this has already been reported or have a solution but this > > > > patch (and presumably series) breaks output to the serial console after > > > > a certain point during init. On Raspbian, I see systemd startup messages > > > > then the output just turns into complete garbage. It looks like this > > > > patch is merged first in linux-next, which is why my bisect fell on the > > > > DRM merge. I am happy to provide whatever information could be helpful > > > > for debugging this. I am on the latest version of the firmware > > > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > > > > > Unfortunately, the miniUART is in the same clock tree than the core > > > clock and will thus have those kind of issues when the core clock is > > > changed (which is also something that one should expect when using the > > > DRM or other drivers). > > > > > > The only real workaround there would be to switch to one of the PL011 > > > UARTs. I guess we can also somehow make the UART react to the core clock > > > frequency changes, but that's going to require some effort > > > > > > Maxime > > > > Ack, thank you for the reply! There does not really seem to be a whole > > ton of documentation around using one of the other PL011 UARTs so for > > now, I will just revert this commit locally. > > Nathan, a less intrusive solution would be to add 'core_freq_min=500' into your > config.txt. > > Funnily enough core_freq=500 doesn't seem to work in that regard. It might be > related with what Maxime is commenting. > > Regards, > Nicolas > Excellent, thank you for the tip, that worked well! Cheers, Nathan
Sorry, my previous statement was misleading. enable_uart will select the mini_uart for gpio14,15 unless the disable-bt device tree overlay is loaded. As well as disabling bluetooth disable-bt swaps the uart0 pin configs to point the regular UART to gpio 14,15. After resolving the DT overlays the firmware does the initial UART setup according to which controller is pointed at pins 14,15. I'll have to speak to others about exactly when the fixing of the core clock takes effect. There have been a few changes related to the initial turbo frequency configuration and how this is reported via the mbox APIs On Thu, 1 Oct 2020 at 17:47, Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Tim, thanks for the info! > > On Thu, 2020-10-01 at 11:15 +0100, Tim Gover wrote: > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > > VCO to support a core-frequency of 550 MHz which is the minimum > > frequency required by the HVS at 4Kp60. The side effect is that if the > > display clock requirements are lower than 4Kp60 then you will see > > different core frequencies selected by DVFS. > > > > If enable_uart=1 and the mini-uart is selected (default unless > > What is the actual test made to check if mini-uart is selected? I can't get > firmware to trigger this behaviour with 64-bit upstream kernel/dts. Note that I > see the core clk setup at 200MHz just before having VC4 set it to 500MHz. > > The only thing I've got on my config.txt is: > > enable_uart=1 > arm_64bit=1 > > Maybe we're missing some kind of DT alias upstream? > > Regards, > Nicolas > > > bluetooth is disabled) then the firmware will pin the core-frequency > > to either core_freq max (500 or 550). Although, I think there is a way > > of pinning it to a lower fixed frequency. > > > > The table in overclocking.md defines options for setting the maximum > > core frequency but unless core_freq_min is specified DVFS will > > automatically pick the lowest idle frequency required by the display > > resolution. >
Hi Tim, On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote: > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > VCO to support a core-frequency of 550 MHz which is the minimum > frequency required by the HVS at 4Kp60. The side effect is that if the > display clock requirements are lower than 4Kp60 then you will see > different core frequencies selected by DVFS. > > If enable_uart=1 and the mini-uart is selected (default unless > bluetooth is disabled) then the firmware will pin the core-frequency > to either core_freq max (500 or 550). Although, I think there is a way > of pinning it to a lower fixed frequency. > > The table in overclocking.md defines options for setting the maximum > core frequency but unless core_freq_min is specified DVFS will > automatically pick the lowest idle frequency required by the display > resolution. I'm wondering if there's some way to detect this from Linux? I guess it would be nice to be able to at least detect a broken config to warn / prevent an user that their situation is not going to be reliable / work really well (like if they have a 4k display without hdmi_enable_4kp60 set, or the issue we're discussing here) Thanks! Maxime
Hi Maxime On Fri, 2 Oct 2020 at 16:19, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Tim, > > On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote: > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > > VCO to support a core-frequency of 550 MHz which is the minimum > > frequency required by the HVS at 4Kp60. The side effect is that if the > > display clock requirements are lower than 4Kp60 then you will see > > different core frequencies selected by DVFS. > > > > If enable_uart=1 and the mini-uart is selected (default unless > > bluetooth is disabled) then the firmware will pin the core-frequency > > to either core_freq max (500 or 550). Although, I think there is a way > > of pinning it to a lower fixed frequency. > > > > The table in overclocking.md defines options for setting the maximum > > core frequency but unless core_freq_min is specified DVFS will > > automatically pick the lowest idle frequency required by the display > > resolution. > > I'm wondering if there's some way to detect this from Linux? I guess it > would be nice to be able to at least detect a broken config to warn / > prevent an user that their situation is not going to be reliable / work > really well (like if they have a 4k display without hdmi_enable_4kp60 > set, or the issue we're discussing here) The main filter in the firmware is the parameter hdmi_pixel_freq_limit. That can either be set manually from config.txt, or defaults appropriately based on hdmi_enable_4kp60. Under firmware_kms [1] I read back those values to use as a filter within crtc_mode_valid[2]. I can't think of a nice way of exposing that without the vc4 driver gaining a DT link to the firmware, and that starts to get ugly. Dave [1] https://github.com/raspberrypi/linux/blob/rpi-5.9.y/drivers/gpu/drm/vc4/vc4_firmware_kms.c#L1859 [2] https://github.com/raspberrypi/linux/blob/rpi-5.9.y/drivers/gpu/drm/vc4/vc4_firmware_kms.c#L1077
Hi Dave, On Fri, Oct 02, 2020 at 04:57:05PM +0100, Dave Stevenson wrote: > Hi Maxime > > On Fri, 2 Oct 2020 at 16:19, Maxime Ripard <maxime@cerno.tech> wrote: > > > > Hi Tim, > > > > On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote: > > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > > > VCO to support a core-frequency of 550 MHz which is the minimum > > > frequency required by the HVS at 4Kp60. The side effect is that if the > > > display clock requirements are lower than 4Kp60 then you will see > > > different core frequencies selected by DVFS. > > > > > > If enable_uart=1 and the mini-uart is selected (default unless > > > bluetooth is disabled) then the firmware will pin the core-frequency > > > to either core_freq max (500 or 550). Although, I think there is a way > > > of pinning it to a lower fixed frequency. > > > > > > The table in overclocking.md defines options for setting the maximum > > > core frequency but unless core_freq_min is specified DVFS will > > > automatically pick the lowest idle frequency required by the display > > > resolution. > > > > I'm wondering if there's some way to detect this from Linux? I guess it > > would be nice to be able to at least detect a broken config to warn / > > prevent an user that their situation is not going to be reliable / work > > really well (like if they have a 4k display without hdmi_enable_4kp60 > > set, or the issue we're discussing here) > > The main filter in the firmware is the parameter > hdmi_pixel_freq_limit. That can either be set manually from > config.txt, or defaults appropriately based on hdmi_enable_4kp60. > Under firmware_kms [1] I read back those values to use as a filter > within crtc_mode_valid[2]. > I can't think of a nice way of exposing that without the vc4 driver > gaining a DT link to the firmware, and that starts to get ugly. I had in mind something like if the clock driver can infer that somehow through some the boundaries reported by the firmware maybe? IIRC, hdmi_enable_4kp60 will already change the max frequency reported to 550MHz instead of 500MHz Maxime
Hi Maxime On Tue, 6 Oct 2020 at 16:26, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Dave, > > On Fri, Oct 02, 2020 at 04:57:05PM +0100, Dave Stevenson wrote: > > Hi Maxime > > > > On Fri, 2 Oct 2020 at 16:19, Maxime Ripard <maxime@cerno.tech> wrote: > > > > > > Hi Tim, > > > > > > On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote: > > > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > > > > VCO to support a core-frequency of 550 MHz which is the minimum > > > > frequency required by the HVS at 4Kp60. The side effect is that if the > > > > display clock requirements are lower than 4Kp60 then you will see > > > > different core frequencies selected by DVFS. > > > > > > > > If enable_uart=1 and the mini-uart is selected (default unless > > > > bluetooth is disabled) then the firmware will pin the core-frequency > > > > to either core_freq max (500 or 550). Although, I think there is a way > > > > of pinning it to a lower fixed frequency. > > > > > > > > The table in overclocking.md defines options for setting the maximum > > > > core frequency but unless core_freq_min is specified DVFS will > > > > automatically pick the lowest idle frequency required by the display > > > > resolution. > > > > > > I'm wondering if there's some way to detect this from Linux? I guess it > > > would be nice to be able to at least detect a broken config to warn / > > > prevent an user that their situation is not going to be reliable / work > > > really well (like if they have a 4k display without hdmi_enable_4kp60 > > > set, or the issue we're discussing here) > > > > The main filter in the firmware is the parameter > > hdmi_pixel_freq_limit. That can either be set manually from > > config.txt, or defaults appropriately based on hdmi_enable_4kp60. > > Under firmware_kms [1] I read back those values to use as a filter > > within crtc_mode_valid[2]. > > I can't think of a nice way of exposing that without the vc4 driver > > gaining a DT link to the firmware, and that starts to get ugly. > > I had in mind something like if the clock driver can infer that somehow > through some the boundaries reported by the firmware maybe? IIRC, > hdmi_enable_4kp60 will already change the max frequency reported to > 550MHz instead of 500MHz Yes, that's plausible, but I don't know enough about the clock infrastructure for advertising limits to know what works there. Tell me what you need from the mailbox service and I'll see what I can do. We do already have RPI_FIRMWARE_GET_MAX_CLOCK_RATE and RPI_FIRMWARE_GET_MIN_CLOCK_RATE. It'd take a few minutes of staring at the code (or a quick test) to confirm if they definitely are changed for CORE clock by hdmi_enable_4kp60 - I think it does. Dave
Hi Dave, sorry for the late reply. On Tue, 2020-10-06 at 18:14 +0100, Dave Stevenson wrote: > Hi Maxime > > On Tue, 6 Oct 2020 at 16:26, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Dave, > > > > On Fri, Oct 02, 2020 at 04:57:05PM +0100, Dave Stevenson wrote: > > > Hi Maxime > > > > > > On Fri, 2 Oct 2020 at 16:19, Maxime Ripard <maxime@cerno.tech> wrote: > > > > Hi Tim, > > > > > > > > On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote: > > > > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC > > > > > VCO to support a core-frequency of 550 MHz which is the minimum > > > > > frequency required by the HVS at 4Kp60. The side effect is that if the > > > > > display clock requirements are lower than 4Kp60 then you will see > > > > > different core frequencies selected by DVFS. > > > > > > > > > > If enable_uart=1 and the mini-uart is selected (default unless > > > > > bluetooth is disabled) then the firmware will pin the core-frequency > > > > > to either core_freq max (500 or 550). Although, I think there is a way > > > > > of pinning it to a lower fixed frequency. > > > > > > > > > > The table in overclocking.md defines options for setting the maximum > > > > > core frequency but unless core_freq_min is specified DVFS will > > > > > automatically pick the lowest idle frequency required by the display > > > > > resolution. > > > > > > > > I'm wondering if there's some way to detect this from Linux? I guess it > > > > would be nice to be able to at least detect a broken config to warn / > > > > prevent an user that their situation is not going to be reliable / work > > > > really well (like if they have a 4k display without hdmi_enable_4kp60 > > > > set, or the issue we're discussing here) > > > > > > The main filter in the firmware is the parameter > > > hdmi_pixel_freq_limit. That can either be set manually from > > > config.txt, or defaults appropriately based on hdmi_enable_4kp60. > > > Under firmware_kms [1] I read back those values to use as a filter > > > within crtc_mode_valid[2]. > > > I can't think of a nice way of exposing that without the vc4 driver > > > gaining a DT link to the firmware, and that starts to get ugly. > > > > I had in mind something like if the clock driver can infer that somehow > > through some the boundaries reported by the firmware maybe? IIRC, > > hdmi_enable_4kp60 will already change the max frequency reported to > > 550MHz instead of 500MHz > > Yes, that's plausible, but I don't know enough about the clock > infrastructure for advertising limits to know what works there. > Tell me what you need from the mailbox service and I'll see what I can do. > > We do already have RPI_FIRMWARE_GET_MAX_CLOCK_RATE and > RPI_FIRMWARE_GET_MIN_CLOCK_RATE. It'd take a few minutes of staring at > the code (or a quick test) to confirm if they definitely are changed > for CORE clock by hdmi_enable_4kp60 - I think it does. Tim commented earlier that you meant to pin CORE frequency when enable_uart=1. In practice it doesn't seem to be the case, but it would solve the UART problem for most use cases. Would that be feasible? If we have to change the CORE frequency during runtime we could, while we search for a proper solution, print a warning that we're about to break the low speed peripherals. Regards, Nicolas
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts index e94244a215af..09a1182c2936 100644 --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts @@ -70,6 +70,14 @@ }; }; +&ddc0 { + status = "okay"; +}; + +&ddc1 { + status = "okay"; +}; + &firmware { firmware_clocks: clocks { compatible = "raspberrypi,firmware-clocks"; @@ -170,6 +178,38 @@ "RGMII_TXD3"; }; +&hdmi0 { + clocks = <&firmware_clocks 13>, <&firmware_clocks 14>, <&dvp 0>, <&clk_27MHz>; + clock-names = "hdmi", "bvb", "audio", "cec"; + status = "okay"; +}; + +&hdmi1 { + clocks = <&firmware_clocks 13>, <&firmware_clocks 14>, <&dvp 1>, <&clk_27MHz>; + clock-names = "hdmi", "bvb", "audio", "cec"; + status = "okay"; +}; + +&hvs { + clocks = <&firmware_clocks 4>; +}; + +&pixelvalve0 { + status = "okay"; +}; + +&pixelvalve1 { + status = "okay"; +}; + +&pixelvalve2 { + status = "okay"; +}; + +&pixelvalve4 { + status = "okay"; +}; + &pwm1 { pinctrl-names = "default"; pinctrl-0 = <&pwm1_0_gpio40 &pwm1_1_gpio41>; @@ -253,3 +293,11 @@ &vchiq { interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>; }; + +&vc4 { + status = "okay"; +}; + +&vec { + status = "disabled"; +}; diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi index 00bcaed1be32..4847dd305317 100644 --- a/arch/arm/boot/dts/bcm2711.dtsi +++ b/arch/arm/boot/dts/bcm2711.dtsi @@ -12,6 +12,18 @@ interrupt-parent = <&gicv2>; + vc4: gpu { + compatible = "brcm,bcm2711-vc5"; + status = "disabled"; + }; + + clk_27MHz: clk-27M { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <27000000>; + clock-output-names = "27MHz-clock"; + }; + clk_108MHz: clk-108M { #clock-cells = <0>; compatible = "fixed-clock"; @@ -238,6 +250,27 @@ status = "disabled"; }; + pixelvalve0: pixelvalve@7e206000 { + compatible = "brcm,bcm2711-pixelvalve0"; + reg = <0x7e206000 0x100>; + interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + + pixelvalve1: pixelvalve@7e207000 { + compatible = "brcm,bcm2711-pixelvalve1"; + reg = <0x7e207000 0x100>; + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + + pixelvalve2: pixelvalve@7e20a000 { + compatible = "brcm,bcm2711-pixelvalve2"; + reg = <0x7e20a000 0x100>; + interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + pwm1: pwm@7e20c800 { compatible = "brcm,bcm2835-pwm"; reg = <0x7e20c800 0x28>; @@ -248,10 +281,25 @@ status = "disabled"; }; - hvs@7e400000 { + pixelvalve4: pixelvalve@7e216000 { + compatible = "brcm,bcm2711-pixelvalve4"; + reg = <0x7e216000 0x100>; + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + + hvs: hvs@7e400000 { + compatible = "brcm,bcm2711-hvs"; interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>; }; + pixelvalve3: pixelvalve@7ec12000 { + compatible = "brcm,bcm2711-pixelvalve3"; + reg = <0x7ec12000 0x100>; + interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + dvp: clock@7ef00000 { compatible = "brcm,brcm2711-dvp"; reg = <0x7ef00000 0x10>; @@ -259,6 +307,78 @@ #clock-cells = <1>; #reset-cells = <1>; }; + + hdmi0: hdmi@7ef00700 { + compatible = "brcm,bcm2711-hdmi0"; + reg = <0x7ef00700 0x300>, + <0x7ef00300 0x200>, + <0x7ef00f00 0x80>, + <0x7ef00f80 0x80>, + <0x7ef01b00 0x200>, + <0x7ef01f00 0x400>, + <0x7ef00200 0x80>, + <0x7ef04300 0x100>, + <0x7ef20000 0x100>; + reg-names = "hdmi", + "dvp", + "phy", + "rm", + "packet", + "metadata", + "csc", + "cec", + "hd"; + clock-names = "hdmi", "bvb", "audio", "cec"; + resets = <&dvp 0>; + ddc = <&ddc0>; + dmas = <&dma 10>; + dma-names = "audio-rx"; + status = "disabled"; + }; + + ddc0: i2c@7ef04500 { + compatible = "brcm,bcm2711-hdmi-i2c"; + reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>; + reg-names = "bsc", "auto-i2c"; + clock-frequency = <97500>; + status = "disabled"; + }; + + hdmi1: hdmi@7ef05700 { + compatible = "brcm,bcm2711-hdmi1"; + reg = <0x7ef05700 0x300>, + <0x7ef05300 0x200>, + <0x7ef05f00 0x80>, + <0x7ef05f80 0x80>, + <0x7ef06b00 0x200>, + <0x7ef06f00 0x400>, + <0x7ef00280 0x80>, + <0x7ef09300 0x100>, + <0x7ef20000 0x100>; + reg-names = "hdmi", + "dvp", + "phy", + "rm", + "packet", + "metadata", + "csc", + "cec", + "hd"; + ddc = <&ddc1>; + clock-names = "hdmi", "bvb", "audio", "cec"; + resets = <&dvp 1>; + dmas = <&dma 17>; + dma-names = "audio-rx"; + status = "disabled"; + }; + + ddc1: i2c@7ef09500 { + compatible = "brcm,bcm2711-hdmi-i2c"; + reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>; + reg-names = "bsc", "auto-i2c"; + clock-frequency = <97500>; + status = "disabled"; + }; }; /*