| Message ID | 1433760002-24120-7-git-send-email-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2015-06-08 at 11:40 +0100, Sudeep Holla wrote: [...] > + > + scpi { > + compatible = "arm,scpi"; > + mboxes = <&mailbox 1>; > + shmem = <&cpu_scp_hpri>; > + > + clocks { > + compatible = "arm,scpi-clocks"; > + > + scpi_dvfs: scpi_clocks@0 { > + compatible = "arm,scpi-dvfs-clocks"; > + #clock-cells = <1>; > + clock-indices = <0>, <1>, <2>; > + clock-output-names = "vbig", "vlittle", "vgpu"; From where do the clock names derive? They look more like names for voltage domains rather than clocks. My (admittedly very old) Juno docs, have the clocks as ATLCLK, APLCLK and GPUCLK. > + }; > + scpi_clk: scpi_clocks@3 { > + compatible = "arm,scpi-variable-clocks"; > + #clock-cells = <1>; > + clock-indices = <3>, <4>; > + clock-output-names = "pxlclk0", "pxlclk1"; Can we also have clock index 5, name 'i2s_clk', for used by audio? (I don't know what other clocks the SCP currently supports, but audio is one being currently used by the out-of-tree code). Also, I believe that both display outputs share the same clock, and so pxlclk0 and pxlclk1 can't be controlled independently. But I guess these device-tree entries are for the interface to the SCP firmware, not the hardware, and if that pretends the clocks are independent...
On 08/06/15 14:51, Jon Medhurst (Tixy) wrote: > On Mon, 2015-06-08 at 11:40 +0100, Sudeep Holla wrote: > [...] >> + >> + scpi { >> + compatible = "arm,scpi"; >> + mboxes = <&mailbox 1>; >> + shmem = <&cpu_scp_hpri>; >> + >> + clocks { >> + compatible = "arm,scpi-clocks"; >> + >> + scpi_dvfs: scpi_clocks@0 { >> + compatible = "arm,scpi-dvfs-clocks"; >> + #clock-cells = <1>; >> + clock-indices = <0>, <1>, <2>; >> + clock-output-names = "vbig", "vlittle", "vgpu"; > > From where do the clock names derive? They look more like names for > voltage domains rather than clocks. My (admittedly very old) Juno docs, > have the clocks as ATLCLK, APLCLK and GPUCLK. > I agree, I just copied it from SCPI spec which just deals with power domain names in the context of DVFS. I will update as per Juno doc. >> + }; >> + scpi_clk: scpi_clocks@3 { >> + compatible = "arm,scpi-variable-clocks"; >> + #clock-cells = <1>; >> + clock-indices = <3>, <4>; >> + clock-output-names = "pxlclk0", "pxlclk1"; > > Can we also have clock index 5, name 'i2s_clk', for used by audio? > (I don't know what other clocks the SCP currently supports, but audio is > one being currently used by the out-of-tree code). > I will update. > Also, I believe that both display outputs share the same clock, and so > pxlclk0 and pxlclk1 can't be controlled independently. But I guess these > device-tree entries are for the interface to the SCP firmware, not the > hardware, and if that pretends the clocks are independent... > Yes, this is bit tricky, I will let Liviu answer this. Regards, Sudeep
On Mon, Jun 08, 2015 at 02:51:19PM +0100, Jon Medhurst (Tixy) wrote: > On Mon, 2015-06-08 at 11:40 +0100, Sudeep Holla wrote: > [...] > > + > > + scpi { > > + compatible = "arm,scpi"; > > + mboxes = <&mailbox 1>; > > + shmem = <&cpu_scp_hpri>; > > + > > + clocks { > > + compatible = "arm,scpi-clocks"; > > + > > + scpi_dvfs: scpi_clocks@0 { > > + compatible = "arm,scpi-dvfs-clocks"; > > + #clock-cells = <1>; > > + clock-indices = <0>, <1>, <2>; > > + clock-output-names = "vbig", "vlittle", "vgpu"; > > From where do the clock names derive? They look more like names for > voltage domains rather than clocks. My (admittedly very old) Juno docs, > have the clocks as ATLCLK, APLCLK and GPUCLK. > > > + }; > > + scpi_clk: scpi_clocks@3 { > > + compatible = "arm,scpi-variable-clocks"; > > + #clock-cells = <1>; > > + clock-indices = <3>, <4>; > > + clock-output-names = "pxlclk0", "pxlclk1"; > > Can we also have clock index 5, name 'i2s_clk', for used by audio? > (I don't know what other clocks the SCP currently supports, but audio is > one being currently used by the out-of-tree code). > > Also, I believe that both display outputs share the same clock, and so > pxlclk0 and pxlclk1 can't be controlled independently. But I guess these > device-tree entries are for the interface to the SCP firmware, not the > hardware, and if that pretends the clocks are independent... Actually, they can be independent, but the other source for clock generation can only be used to drive a VGA resolution. We were just debating with Sudeep on how to expose this: at the moment the SCP is configured so that the request for a clock frequency always succeeds even if the other user of the clock is active. That means that if you have 2 monitors connected that have different resolutions or pixel clocks then one of the monitor will get out of sync (unless it is a VGA monitor). On the other hand the HDLCD driver (or more corectly the DRM KMS one) will default to 640x480 if no monitor is connected to an HDMI output, meaning the clock could already be used but by a driver that should really be inactive. It is very hard to detect in which situation we are, so the usual fix is "plug the monitor into the other HDMI output if you have problems with single monitors". Best regards, Liviu > > -- > Tixy > >
On Mon, Jun 08, 2015 at 11:40:00AM +0100, Sudeep Holla wrote: > This patch adds support for the MHU mailbox peripheral used on Juno by > application processors to communicate with remote SCP handling most of > the CPU/system power management. It also adds the SRAM reserving the > shared memory and SCPI message protocol using that shared memory. > > Cc: Liviu Dudau <Liviu.Dudau@arm.com> > Cc: Jon Medhurst (Tixy) <tixy@linaro.org> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > arch/arm64/boot/dts/arm/juno-base.dtsi | 31 +++++++++++++++++++++++++++++++ > arch/arm64/boot/dts/arm/juno-clocks.dtsi | 23 +++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi > index e3ee96036eca..f8069a98da25 100644 > --- a/arch/arm64/boot/dts/arm/juno-base.dtsi > +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi > @@ -17,6 +17,18 @@ > }; > }; > > + mailbox: mhu@2b1f0000 { > + compatible = "arm,mhu", "arm,primecell"; > + reg = <0x0 0x2b1f0000 0x0 0x1000>; > + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "mhu_lpri_rx", > + "mhu_hpri_rx"; > + #mbox-cells = <1>; > + clocks = <&soc_refclk100mhz>; > + clock-names = "apb_pclk"; > + }; > + > gic: interrupt-controller@2c010000 { > compatible = "arm,gic-400", "arm,cortex-a15-gic"; > reg = <0x0 0x2c010000 0 0x1000>, > @@ -44,6 +56,25 @@ > <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>; > }; > > + sram: sram@2e000000 { > + compatible = "arm,juno-sram-ns", "mmio-sram"; > + reg = <0x0 0x2e000000 0x0 0x8000>; > + > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x0 0x2e000000 0x8000>; > + > + cpu_scp_lpri: scp-shmem@0 { > + compatible = "arm,juno-scp-shmem"; > + reg = <0x0 0x200>; > + }; > + > + cpu_scp_hpri: scp-shmem@200 { > + compatible = "arm,juno-scp-shmem"; > + reg = <0x200 0x200>; > + }; > + }; > + > /include/ "juno-clocks.dtsi" > > dma@7ff00000 { > diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi > index 25352ed943e6..64af7370815a 100644 > --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi > +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi > @@ -42,3 +42,26 @@ > clock-frequency = <400000000>; > clock-output-names = "faxi_clk"; > }; > + > + scpi { > + compatible = "arm,scpi"; > + mboxes = <&mailbox 1>; > + shmem = <&cpu_scp_hpri>; > + > + clocks { > + compatible = "arm,scpi-clocks"; > + > + scpi_dvfs: scpi_clocks@0 { > + compatible = "arm,scpi-dvfs-clocks"; > + #clock-cells = <1>; > + clock-indices = <0>, <1>, <2>; > + clock-output-names = "vbig", "vlittle", "vgpu"; > + }; > + scpi_clk: scpi_clocks@3 { > + compatible = "arm,scpi-variable-clocks"; > + #clock-cells = <1>; > + clock-indices = <3>, <4>; Subject to you addressing Mark's comments regarding the indices values (maybe choose a different property to show the fact that the index is actually an SCPI index rather than the clock's), you can add my Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> Best regards, Liviu > + clock-output-names = "pxlclk0", "pxlclk1"; > + }; > + }; > + }; > -- > 1.9.1 >
On 22/07/15 14:28, Liviu Dudau wrote: > On Mon, Jun 08, 2015 at 11:40:00AM +0100, Sudeep Holla wrote: >> This patch adds support for the MHU mailbox peripheral used on Juno by >> application processors to communicate with remote SCP handling most of >> the CPU/system power management. It also adds the SRAM reserving the >> shared memory and SCPI message protocol using that shared memory. >> >> Cc: Liviu Dudau <Liviu.Dudau@arm.com> >> Cc: Jon Medhurst (Tixy) <tixy@linaro.org> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- >> arch/arm64/boot/dts/arm/juno-base.dtsi | 31 +++++++++++++++++++++++++++++++ >> arch/arm64/boot/dts/arm/juno-clocks.dtsi | 23 +++++++++++++++++++++++ >> 2 files changed, 54 insertions(+) >> [..] >> diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi >> index 25352ed943e6..64af7370815a 100644 >> --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi >> +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi >> @@ -42,3 +42,26 @@ >> clock-frequency = <400000000>; >> clock-output-names = "faxi_clk"; >> }; >> + >> + scpi { >> + compatible = "arm,scpi"; >> + mboxes = <&mailbox 1>; >> + shmem = <&cpu_scp_hpri>; >> + >> + clocks { >> + compatible = "arm,scpi-clocks"; >> + >> + scpi_dvfs: scpi_clocks@0 { >> + compatible = "arm,scpi-dvfs-clocks"; >> + #clock-cells = <1>; >> + clock-indices = <0>, <1>, <2>; >> + clock-output-names = "vbig", "vlittle", "vgpu"; >> + }; >> + scpi_clk: scpi_clocks@3 { >> + compatible = "arm,scpi-variable-clocks"; >> + #clock-cells = <1>; >> + clock-indices = <3>, <4>; > > Subject to you addressing Mark's comments regarding the indices values (maybe choose > a different property to show the fact that the index is actually an SCPI index > rather than the clock's), you can add my > I don't understand why we need to do that. I will anyway follow up on that thread. > Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> Thanks for all the ACKs. Regards, Sudeep
On Wed, Jul 22, 2015 at 04:40:30PM +0100, Sudeep Holla wrote: > > > On 22/07/15 14:28, Liviu Dudau wrote: > > On Mon, Jun 08, 2015 at 11:40:00AM +0100, Sudeep Holla wrote: > >> This patch adds support for the MHU mailbox peripheral used on Juno by > >> application processors to communicate with remote SCP handling most of > >> the CPU/system power management. It also adds the SRAM reserving the > >> shared memory and SCPI message protocol using that shared memory. > >> > >> Cc: Liviu Dudau <Liviu.Dudau@arm.com> > >> Cc: Jon Medhurst (Tixy) <tixy@linaro.org> > >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > >> --- > >> arch/arm64/boot/dts/arm/juno-base.dtsi | 31 +++++++++++++++++++++++++++++++ > >> arch/arm64/boot/dts/arm/juno-clocks.dtsi | 23 +++++++++++++++++++++++ > >> 2 files changed, 54 insertions(+) > >> > > [..] > > >> diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi > >> index 25352ed943e6..64af7370815a 100644 > >> --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi > >> +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi > >> @@ -42,3 +42,26 @@ > >> clock-frequency = <400000000>; > >> clock-output-names = "faxi_clk"; > >> }; > >> + > >> + scpi { > >> + compatible = "arm,scpi"; > >> + mboxes = <&mailbox 1>; > >> + shmem = <&cpu_scp_hpri>; > >> + > >> + clocks { > >> + compatible = "arm,scpi-clocks"; > >> + > >> + scpi_dvfs: scpi_clocks@0 { > >> + compatible = "arm,scpi-dvfs-clocks"; > >> + #clock-cells = <1>; > >> + clock-indices = <0>, <1>, <2>; > >> + clock-output-names = "vbig", "vlittle", "vgpu"; > >> + }; > >> + scpi_clk: scpi_clocks@3 { > >> + compatible = "arm,scpi-variable-clocks"; > >> + #clock-cells = <1>; > >> + clock-indices = <3>, <4>; > > > > Subject to you addressing Mark's comments regarding the indices values (maybe choose > > a different property to show the fact that the index is actually an SCPI index > > rather than the clock's), you can add my > > > > I don't understand why we need to do that. I will anyway follow up on > that thread. Because indices are per clock node, i.e. spi_clk should have clock-indices = <0>, <1>. Of course, you could have a gap in the indices, but that is both awkard and not clearly explained in this documentation. The index that you declare here is actually what you pass to SCPI. But the way the device tree is presented it declares that there are two clock blocks, one for DVFS and one for PXLCLK. As far as SCPI is concerned there is only one block of clocks, with 3 of them having a discrete set of values, so we are at the intersection of two concepts. BTW, for what is worth, the PXLCLK is not really that smooth in its coverage of the range. It might have more accepted frequency values, but the way it is implemented it tends to favour VESA clock values and falls back to a really slow algorithm to generate all other values. Even so, it can fail to find the correct parameters for the PLLs so it will generate a frequency that is different from the requested one. Best regards, Liviu > > > Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> > > Thanks for all the ACKs. > > Regards, > Sudeep >
On 22/07/15 17:06, Liviu Dudau wrote: > On Wed, Jul 22, 2015 at 04:40:30PM +0100, Sudeep Holla wrote: >> >> >> On 22/07/15 14:28, Liviu Dudau wrote: >>> On Mon, Jun 08, 2015 at 11:40:00AM +0100, Sudeep Holla wrote: >>>> This patch adds support for the MHU mailbox peripheral used on Juno by >>>> application processors to communicate with remote SCP handling most of >>>> the CPU/system power management. It also adds the SRAM reserving the >>>> shared memory and SCPI message protocol using that shared memory. >>>> >>>> Cc: Liviu Dudau <Liviu.Dudau@arm.com> >>>> Cc: Jon Medhurst (Tixy) <tixy@linaro.org> >>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>> --- >>>> arch/arm64/boot/dts/arm/juno-base.dtsi | 31 +++++++++++++++++++++++++++++++ >>>> arch/arm64/boot/dts/arm/juno-clocks.dtsi | 23 +++++++++++++++++++++++ >>>> 2 files changed, 54 insertions(+) >>>> >> >> [..] >> >>>> diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi >>>> index 25352ed943e6..64af7370815a 100644 >>>> --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi >>>> +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi >>>> @@ -42,3 +42,26 @@ >>>> clock-frequency = <400000000>; >>>> clock-output-names = "faxi_clk"; >>>> }; >>>> + >>>> + scpi { >>>> + compatible = "arm,scpi"; >>>> + mboxes = <&mailbox 1>; >>>> + shmem = <&cpu_scp_hpri>; >>>> + >>>> + clocks { >>>> + compatible = "arm,scpi-clocks"; >>>> + >>>> + scpi_dvfs: scpi_clocks@0 { >>>> + compatible = "arm,scpi-dvfs-clocks"; >>>> + #clock-cells = <1>; >>>> + clock-indices = <0>, <1>, <2>; >>>> + clock-output-names = "vbig", "vlittle", "vgpu"; >>>> + }; >>>> + scpi_clk: scpi_clocks@3 { >>>> + compatible = "arm,scpi-variable-clocks"; >>>> + #clock-cells = <1>; >>>> + clock-indices = <3>, <4>; >>> >>> Subject to you addressing Mark's comments regarding the indices values (maybe choose >>> a different property to show the fact that the index is actually an SCPI index >>> rather than the clock's), you can add my >>> >> >> I don't understand why we need to do that. I will anyway follow up on >> that thread. > > Because indices are per clock node, i.e. spi_clk should have clock-indices = <0>, <1>. > Of course, you could have a gap in the indices, but that is both awkard and not clearly > explained in this documentation. > Hmm, is it not clear ? I am fine to reword it. clock-indices: The *identifying number* for the clocks(clock_id) in the node as *expected by the firmware*. It can be non linear and hence provide the mapping of identifiers into the clock-output-names array. > The index that you declare here is actually what you pass to SCPI. But the way the device > tree is presented it declares that there are two clock blocks, one for DVFS and one for > PXLCLK. As far as SCPI is concerned there is only one block of clocks, with 3 of them > having a discrete set of values, so we are at the intersection of two concepts. > Agreed. The clock-indices is associated with the clock provider which in this case is SCPI. The clock specifier is associated with the clock consumer and all it needs to know is which one in the list of clock output am I referring to(i.e. the standard definition decoded using of_clk_src_onecell_get > BTW, for what is worth, the PXLCLK is not really that smooth in its coverage of the range. > It might have more accepted frequency values, but the way it is implemented it tends to > favour VESA clock values and falls back to a really slow algorithm to generate all other > values. Even so, it can fail to find the correct parameters for the PLLs so it will generate > a frequency that is different from the requested one. I don't quite get the context of this info here, but thanks for that :). I think most of the clock providers have this limitation and round_rate will handle it. Since SCPI specification doesn't provide a mechanism to query that, we can't handle it. Hopefully we get that in newer versions of the specification ;) Regards, Sudeep
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi index e3ee96036eca..f8069a98da25 100644 --- a/arch/arm64/boot/dts/arm/juno-base.dtsi +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi @@ -17,6 +17,18 @@ }; }; + mailbox: mhu@2b1f0000 { + compatible = "arm,mhu", "arm,primecell"; + reg = <0x0 0x2b1f0000 0x0 0x1000>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "mhu_lpri_rx", + "mhu_hpri_rx"; + #mbox-cells = <1>; + clocks = <&soc_refclk100mhz>; + clock-names = "apb_pclk"; + }; + gic: interrupt-controller@2c010000 { compatible = "arm,gic-400", "arm,cortex-a15-gic"; reg = <0x0 0x2c010000 0 0x1000>, @@ -44,6 +56,25 @@ <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>; }; + sram: sram@2e000000 { + compatible = "arm,juno-sram-ns", "mmio-sram"; + reg = <0x0 0x2e000000 0x0 0x8000>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x0 0x2e000000 0x8000>; + + cpu_scp_lpri: scp-shmem@0 { + compatible = "arm,juno-scp-shmem"; + reg = <0x0 0x200>; + }; + + cpu_scp_hpri: scp-shmem@200 { + compatible = "arm,juno-scp-shmem"; + reg = <0x200 0x200>; + }; + }; + /include/ "juno-clocks.dtsi" dma@7ff00000 { diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi index 25352ed943e6..64af7370815a 100644 --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi @@ -42,3 +42,26 @@ clock-frequency = <400000000>; clock-output-names = "faxi_clk"; }; + + scpi { + compatible = "arm,scpi"; + mboxes = <&mailbox 1>; + shmem = <&cpu_scp_hpri>; + + clocks { + compatible = "arm,scpi-clocks"; + + scpi_dvfs: scpi_clocks@0 { + compatible = "arm,scpi-dvfs-clocks"; + #clock-cells = <1>; + clock-indices = <0>, <1>, <2>; + clock-output-names = "vbig", "vlittle", "vgpu"; + }; + scpi_clk: scpi_clocks@3 { + compatible = "arm,scpi-variable-clocks"; + #clock-cells = <1>; + clock-indices = <3>, <4>; + clock-output-names = "pxlclk0", "pxlclk1"; + }; + }; + };
This patch adds support for the MHU mailbox peripheral used on Juno by application processors to communicate with remote SCP handling most of the CPU/system power management. It also adds the SRAM reserving the shared memory and SCPI message protocol using that shared memory. Cc: Liviu Dudau <Liviu.Dudau@arm.com> Cc: Jon Medhurst (Tixy) <tixy@linaro.org> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- arch/arm64/boot/dts/arm/juno-base.dtsi | 31 +++++++++++++++++++++++++++++++ arch/arm64/boot/dts/arm/juno-clocks.dtsi | 23 +++++++++++++++++++++++ 2 files changed, 54 insertions(+)