RE: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
Hi,
> From: Marek Vasut
> Sent: jeudi 2 avril 2020 15:05
>
> On 4/2/20 9:44 AM, Gerald BAEZA wrote:
> > Hi Marek
> >
> >> From: Marek Vasut
> >> Sent: mercredi 1 avril 2020 20:49
> >>
> >> On 4/1/20 6:49 PM, Gerald BAEZA wrote:
> >>> Hi Marek and Patrick
> >>
> >> Hi,
> >>
> From: Marek Vasut
> Sent: mercredi 1 avril 2020 13:09
>
> On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> > Hi Gerald and Manivannan,
>
> Hi,
>
> >> From: Marek Vasut
> >> Sent: mardi 31 mars 2020 19:52
> >>
> >> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz
> and
> >> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces,
> >> which can not easily divide the clock down to e.g. 50 MHz for
> >> high speed SD and eMMC devices, so those devices end up running
> >> at 30 MHz as that is
> 120 MHz / 4.
> >> Adjust the PLL4 settings such that both PLL4P and PLL4R run at
> >> 100 MHz instead, which is easy to divide to 50MHz for optimal
> >> operation of both SD and eMMC, SPDIF clock are not that much
> >> slower and
> >> FDCAN
> >> is
> also unaffected.
> >>>
> >>> As far as I see, SPDIF is not supported on Avenger board so we don't
> >>> care
> >> for this one.
> >>> FDCAN can be supported via the expansion connector, so better to
> >>> keep it
> >> high (and < 100 MHz).
> >>
> >> Why < 100 MHz and not <= 100 MHz ?
> >
> > You're right, it is <= 100 MHz and it is important to be precise since
> > you were exactly on 100 MHz :)
> >
> >>> Be careful because the LTDC pixel clock also comes from the PLL4Q
> >>> and it is
> >> used for the HDMI on Avenger.
> >>> The pixel clock is very constraint and I am surprised by the initial
> >>> 40 MHz
> >> configuration (that becomes 50 MHz with your patch).
> >>
> >> Is that a problem that the LTDC pixel clock are 50 MHz ? The kernel
> >> will reconfigure them anyway, so the 50 MHz is not the final value.
> >
> > The kernel set_rate() changes the PLL output divisor, so it will indeed be
> > able to
> switch back to (600/15=) 40 MHz from an initial (600/12=) 50 MHz.
> >
> >>> I would recommend to align the Avenger configuration to ST boards
> >>> one,
> >> that is the best compromise found so far (99 MHz for SDMMC and 74.250
> >> MHz for HDMI):
> >>
> >> Why is this better than 100/50/100 ?
> >>
> >>> https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
> >>> /* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
> >>> pll4: st,pll@3 {
> >>> cfg = < 3 98 5 7 7 PQR(1,1,1) >;
> >>> u-boot,dm-pre-reloc;
> >>> };
> >> [...]
> >
> > The simplest explanation I found is here:
> > https://timetoexplore.net/blog/video-timings-vga-720p-1080p
> > (you can also look for "74.25" in the HDMI specification but there is
> > more text to read)
> >
> > So 74.250 MHz is the needed pixel clock for 720p resolution in HDMI, that is
> quite common, so this frequency is wished for interoperability with a maximum
> of
> TVs.
> > This frequency cannot be reached from the initial or your proposed PLL4
> configuration, that is why I proposed to change this and align on ST board
> clock
> tree.
> >
> > For the other displays we are supporting on ST boards, the kernel is tuning
> (down) this 74.250 MHz frequency via the set_rate (that changes the PLL output
> divisor).
>
> The article you found is focused on FPGA, where the FPGA is connected directly
> to the HDMI TMDS signals without transceiver, so the FPGA has to generate very
> precise clock. I don't think this is our case here.
>
> The ADV7513 datasheet says nothing about such specific clock rate, it only
> mentions maximum clock per resolution to be 82.5 MHz and that it can do pixel
> repeat to meet the timing requirements.
>
> (Note to self, PLL4R should run at 99 MHz too, to run FDCAN fast enough.)
Without more requirement on the clock tree, the new PLL4 is acceptable.
Applied to u-boot-stm/master, thanks!
Regards
Patrick
Re: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
On 4/2/20 9:44 AM, Gerald BAEZA wrote:
> Hi Marek
>
>> From: Marek Vasut
>> Sent: mercredi 1 avril 2020 20:49
>>
>> On 4/1/20 6:49 PM, Gerald BAEZA wrote:
>>> Hi Marek and Patrick
>>
>> Hi,
>>
From: Marek Vasut
Sent: mercredi 1 avril 2020 13:09
On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> Hi Gerald and Manivannan,
Hi,
>> From: Marek Vasut
>> Sent: mardi 31 mars 2020 19:52
>>
>> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
>> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
>> can not easily divide the clock down to e.g. 50 MHz for high speed
>> SD and eMMC devices, so those devices end up running at 30 MHz as
>> that is
120 MHz / 4.
>> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
>> MHz instead, which is easy to divide to 50MHz for optimal operation
>> of both SD and eMMC, SPDIF clock are not that much slower and
>> FDCAN
>> is
also unaffected.
>>>
>>> As far as I see, SPDIF is not supported on Avenger board so we don't care
>> for this one.
>>> FDCAN can be supported via the expansion connector, so better to keep it
>> high (and < 100 MHz).
>>
>> Why < 100 MHz and not <= 100 MHz ?
>
> You're right, it is <= 100 MHz and it is important to be precise since you
> were exactly on 100 MHz :)
>
>>> Be careful because the LTDC pixel clock also comes from the PLL4Q and it is
>> used for the HDMI on Avenger.
>>> The pixel clock is very constraint and I am surprised by the initial 40 MHz
>> configuration (that becomes 50 MHz with your patch).
>>
>> Is that a problem that the LTDC pixel clock are 50 MHz ? The kernel will
>> reconfigure them anyway, so the 50 MHz is not the final value.
>
> The kernel set_rate() changes the PLL output divisor, so it will indeed be
> able to switch back to (600/15=) 40 MHz from an initial (600/12=) 50 MHz.
>
>>> I would recommend to align the Avenger configuration to ST boards one,
>> that is the best compromise found so far (99 MHz for SDMMC and 74.250
>> MHz for HDMI):
>>
>> Why is this better than 100/50/100 ?
>>
>>> https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
>>> /* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
>>> pll4: st,pll@3 {
>>> cfg = < 3 98 5 7 7 PQR(1,1,1) >;
>>> u-boot,dm-pre-reloc;
>>> };
>> [...]
>
> The simplest explanation I found is here:
> https://timetoexplore.net/blog/video-timings-vga-720p-1080p
> (you can also look for "74.25" in the HDMI specification but there is more
> text to read)
>
> So 74.250 MHz is the needed pixel clock for 720p resolution in HDMI, that is
> quite common, so this frequency is wished for interoperability with a maximum
> of TVs.
> This frequency cannot be reached from the initial or your proposed PLL4
> configuration, that is why I proposed to change this and align on ST board
> clock tree.
>
> For the other displays we are supporting on ST boards, the kernel is tuning
> (down) this 74.250 MHz frequency via the set_rate (that changes the PLL
> output divisor).
The article you found is focused on FPGA, where the FPGA is connected
directly to the HDMI TMDS signals without transceiver, so the FPGA has
to generate very precise clock. I don't think this is our case here.
The ADV7513 datasheet says nothing about such specific clock rate, it
only mentions maximum clock per resolution to be 82.5 MHz and that it
can do pixel repeat to meet the timing requirements.
(Note to self, PLL4R should run at 99 MHz too, to run FDCAN fast enough.)
RE: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
Hi Marek
> From: Marek Vasut
> Sent: mercredi 1 avril 2020 20:49
>
> On 4/1/20 6:49 PM, Gerald BAEZA wrote:
> > Hi Marek and Patrick
>
> Hi,
>
> >> From: Marek Vasut
> >> Sent: mercredi 1 avril 2020 13:09
> >>
> >> On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> >>> Hi Gerald and Manivannan,
> >>
> >> Hi,
> >>
> From: Marek Vasut
> Sent: mardi 31 mars 2020 19:52
>
> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
> can not easily divide the clock down to e.g. 50 MHz for high speed
> SD and eMMC devices, so those devices end up running at 30 MHz as
> that is
> >> 120 MHz / 4.
> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
> MHz instead, which is easy to divide to 50MHz for optimal operation
> of both SD and eMMC, SPDIF clock are not that much slower and
> FDCAN
> is
> >> also unaffected.
> >
> > As far as I see, SPDIF is not supported on Avenger board so we don't care
> for this one.
> > FDCAN can be supported via the expansion connector, so better to keep it
> high (and < 100 MHz).
>
> Why < 100 MHz and not <= 100 MHz ?
You're right, it is <= 100 MHz and it is important to be precise since you were
exactly on 100 MHz :)
> > Be careful because the LTDC pixel clock also comes from the PLL4Q and it is
> used for the HDMI on Avenger.
> > The pixel clock is very constraint and I am surprised by the initial 40 MHz
> configuration (that becomes 50 MHz with your patch).
>
> Is that a problem that the LTDC pixel clock are 50 MHz ? The kernel will
> reconfigure them anyway, so the 50 MHz is not the final value.
The kernel set_rate() changes the PLL output divisor, so it will indeed be able
to switch back to (600/15=) 40 MHz from an initial (600/12=) 50 MHz.
> > I would recommend to align the Avenger configuration to ST boards one,
> that is the best compromise found so far (99 MHz for SDMMC and 74.250
> MHz for HDMI):
>
> Why is this better than 100/50/100 ?
>
> > https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
> > /* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
> > pll4: st,pll@3 {
> > cfg = < 3 98 5 7 7 PQR(1,1,1) >;
> > u-boot,dm-pre-reloc;
> > };
> [...]
The simplest explanation I found is here:
https://timetoexplore.net/blog/video-timings-vga-720p-1080p
(you can also look for "74.25" in the HDMI specification but there is more text
to read)
So 74.250 MHz is the needed pixel clock for 720p resolution in HDMI, that is
quite common, so this frequency is wished for interoperability with a maximum
of TVs.
This frequency cannot be reached from the initial or your proposed PLL4
configuration, that is why I proposed to change this and align on ST board
clock tree.
For the other displays we are supporting on ST boards, the kernel is tuning
(down) this 74.250 MHz frequency via the set_rate (that changes the PLL output
divisor).
Best regards
Gérald
RE: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
Hi Marek and Patrick
> From: Marek Vasut
> Sent: mercredi 1 avril 2020 13:09
>
> On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> > Hi Gerald and Manivannan,
>
> Hi,
>
> >> From: Marek Vasut
> >> Sent: mardi 31 mars 2020 19:52
> >>
> >> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
> >> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
> >> can not easily divide the clock down to e.g. 50 MHz for high speed SD
> >> and eMMC devices, so those devices end up running at 30 MHz as that is
> 120 MHz / 4.
> >> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
> >> MHz instead, which is easy to divide to 50MHz for optimal operation
> >> of both SD and eMMC, SPDIF clock are not that much slower and FDCAN is
> also unaffected.
As far as I see, SPDIF is not supported on Avenger board so we don't care for
this one.
FDCAN can be supported via the expansion connector, so better to keep it high
(and < 100 MHz).
Be careful because the LTDC pixel clock also comes from the PLL4Q and it is
used for the HDMI on Avenger.
The pixel clock is very constraint and I am surprised by the initial 40 MHz
configuration (that becomes 50 MHz with your patch).
I would recommend to align the Avenger configuration to ST boards one, that is
the best compromise found so far (99 MHz for SDMMC and 74.250 MHz for HDMI):
https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
/* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
pll4: st,pll@3 {
cfg = < 3 98 5 7 7 PQR(1,1,1) >;
u-boot,dm-pre-reloc;
};
> >> Reviewed-by: Patrice Chotard
> >> Reviewed-by: Patrick Delaunay
> >> Signed-off-by: Marek Vasut
> >> Cc: Manivannan Sadhasivam
> >> Cc: Patrick Delaunay
> >> Cc: Patrice Chotard
> >> ---
> >> V2: Move this patch before the split of AV96 into SoM and carrier
> >> V3: No change
> >> ---
> >
> > This patch update the PLL4 frequency used on AV96 board, with
> > different of reference clock tree used on ST board, this new setting
> > allow to optimize the SDMMC frequency (50MHz vs 30Mz).
> >
> > I don't know why the previous PLL4 frequency was chosen as a
> > compromise on reference clock-tree (PLL4 is used by mostly all the
> > peripheral, with display and audio requirements).
> >
> > Can you cross check the proposed clock tree and ack this patch if
> > these ST requirements are not applicable on AV96 board.
> >
> > Anyway the code is correct.
>
> Likely because these PLL settings are being copied from reference platform
> to other platforms etc.
As far as I remember, we never had this configuration for PLL4 on ST boards, so
the copy certainly comes from somewhere else.
> But I did notice one odd thing, which is when running the SD1 in SDR104, the
> read data transfers can be unstable, which I suspect is because the bus runs
> at actual 100 MHz instead of some 60 MHz. I need to look at that with a
> scope, so that's to be checked. For now I turned the SDR104 off.
Re: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
On 4/1/20 6:49 PM, Gerald BAEZA wrote:
> Hi Marek and Patrick
Hi,
>> From: Marek Vasut
>> Sent: mercredi 1 avril 2020 13:09
>>
>> On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
>>> Hi Gerald and Manivannan,
>>
>> Hi,
>>
From: Marek Vasut
Sent: mardi 31 mars 2020 19:52
The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
can not easily divide the clock down to e.g. 50 MHz for high speed SD
and eMMC devices, so those devices end up running at 30 MHz as that is
>> 120 MHz / 4.
Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
MHz instead, which is easy to divide to 50MHz for optimal operation
of both SD and eMMC, SPDIF clock are not that much slower and FDCAN is
>> also unaffected.
>
> As far as I see, SPDIF is not supported on Avenger board so we don't care for
> this one.
> FDCAN can be supported via the expansion connector, so better to keep it high
> (and < 100 MHz).
Why < 100 MHz and not <= 100 MHz ?
> Be careful because the LTDC pixel clock also comes from the PLL4Q and it is
> used for the HDMI on Avenger.
> The pixel clock is very constraint and I am surprised by the initial 40 MHz
> configuration (that becomes 50 MHz with your patch).
Is that a problem that the LTDC pixel clock are 50 MHz ? The kernel will
reconfigure them anyway, so the 50 MHz is not the final value.
> I would recommend to align the Avenger configuration to ST boards one, that
> is the best compromise found so far (99 MHz for SDMMC and 74.250 MHz for
> HDMI):
Why is this better than 100/50/100 ?
> https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
> /* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
> pll4: st,pll@3 {
> cfg = < 3 98 5 7 7 PQR(1,1,1) >;
> u-boot,dm-pre-reloc;
> };
[...]
Re: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
Hi,
On Wed, Apr 01, 2020 at 04:49:45PM +, Gerald BAEZA wrote:
> Hi Marek and Patrick
>
> > From: Marek Vasut
> > Sent: mercredi 1 avril 2020 13:09
> >
> > On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> > > Hi Gerald and Manivannan,
> >
> > Hi,
> >
> > >> From: Marek Vasut
> > >> Sent: mardi 31 mars 2020 19:52
> > >>
> > >> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
> > >> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
> > >> can not easily divide the clock down to e.g. 50 MHz for high speed SD
> > >> and eMMC devices, so those devices end up running at 30 MHz as that is
> > 120 MHz / 4.
> > >> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
> > >> MHz instead, which is easy to divide to 50MHz for optimal operation
> > >> of both SD and eMMC, SPDIF clock are not that much slower and FDCAN is
> > also unaffected.
>
> As far as I see, SPDIF is not supported on Avenger board so we don't care for
> this one.
> FDCAN can be supported via the expansion connector, so better to keep it high
> (and < 100 MHz).
> Be careful because the LTDC pixel clock also comes from the PLL4Q and it is
> used for the HDMI on Avenger.
> The pixel clock is very constraint and I am surprised by the initial 40 MHz
> configuration (that becomes 50 MHz with your patch).
> I would recommend to align the Avenger configuration to ST boards one, that
> is the best compromise found so far (99 MHz for SDMMC and 74.250 MHz for
> HDMI):
> https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
> /* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
> pll4: st,pll@3 {
> cfg = < 3 98 5 7 7 PQR(1,1,1) >;
> u-boot,dm-pre-reloc;
> };
>
This looks good to me.
> > >> Reviewed-by: Patrice Chotard
> > >> Reviewed-by: Patrick Delaunay
> > >> Signed-off-by: Marek Vasut
> > >> Cc: Manivannan Sadhasivam
> > >> Cc: Patrick Delaunay
> > >> Cc: Patrice Chotard
> > >> ---
> > >> V2: Move this patch before the split of AV96 into SoM and carrier
> > >> V3: No change
> > >> ---
> > >
> > > This patch update the PLL4 frequency used on AV96 board, with
> > > different of reference clock tree used on ST board, this new setting
> > > allow to optimize the SDMMC frequency (50MHz vs 30Mz).
> > >
> > > I don't know why the previous PLL4 frequency was chosen as a
> > > compromise on reference clock-tree (PLL4 is used by mostly all the
> > > peripheral, with display and audio requirements).
> > >
> > > Can you cross check the proposed clock tree and ack this patch if
> > > these ST requirements are not applicable on AV96 board.
> > >
> > > Anyway the code is correct.
> >
> > Likely because these PLL settings are being copied from reference platform
> > to other platforms etc.
>
> As far as I remember, we never had this configuration for PLL4 on ST boards,
> so the copy certainly comes from somewhere else.
>
I took the PLL settings from Downstream code done by Arrow at that time. Since
I never had access to PMIC info, I just used them blindly since that worked for
initial upstream bringup on old board. We had several discussions about PMIC
at that time but never touched PLL4.
So feel free to modify it as Gerald suggested.
Thanks,
Mani
> > But I did notice one odd thing, which is when running the SD1 in SDR104, the
> > read data transfers can be unstable, which I suspect is because the bus runs
> > at actual 100 MHz instead of some 60 MHz. I need to look at that with a
> > scope, so that's to be checked. For now I turned the SDR104 off.
>
Re: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
On 4/1/20 12:24 PM, Patrick DELAUNAY wrote: > Hi Gerald and Manivannan, Hi, >> From: Marek Vasut >> Sent: mardi 31 mars 2020 19:52 >> >> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and >> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which can not >> easily divide the clock down to e.g. 50 MHz for high speed SD and eMMC >> devices, so those devices end up running at 30 MHz as that is 120 MHz / 4. >> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100 MHz >> instead, which is easy to divide to 50MHz for optimal operation of both SD >> and >> eMMC, SPDIF clock are not that much slower and FDCAN is also unaffected. >> >> Reviewed-by: Patrice Chotard >> Reviewed-by: Patrick Delaunay >> Signed-off-by: Marek Vasut >> Cc: Manivannan Sadhasivam >> Cc: Patrick Delaunay >> Cc: Patrice Chotard >> --- >> V2: Move this patch before the split of AV96 into SoM and carrier >> V3: No change >> --- > > This patch update the PLL4 frequency used on AV96 board, > with different of reference clock tree used on ST board, > this new setting allow to optimize the SDMMC frequency (50MHz vs 30Mz). > > I don't know why the previous PLL4 frequency was chosen as a compromise > on reference clock-tree (PLL4 is used by mostly all the peripheral, > with display and audio requirements). > > Can you cross check the proposed clock tree and ack this patch > if these ST requirements are not applicable on AV96 board. > > Anyway the code is correct. Likely because these PLL settings are being copied from reference platform to other platforms etc. But I did notice one odd thing, which is when running the SD1 in SDR104, the read data transfers can be unstable, which I suspect is because the bus runs at actual 100 MHz instead of some 60 MHz. I need to look at that with a scope, so that's to be checked. For now I turned the SDR104 off.
RE: [PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96
Hi Gerald and Manivannan, > From: Marek Vasut > Sent: mardi 31 mars 2020 19:52 > > The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and > FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which can not > easily divide the clock down to e.g. 50 MHz for high speed SD and eMMC > devices, so those devices end up running at 30 MHz as that is 120 MHz / 4. > Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100 MHz > instead, which is easy to divide to 50MHz for optimal operation of both SD and > eMMC, SPDIF clock are not that much slower and FDCAN is also unaffected. > > Reviewed-by: Patrice Chotard > Reviewed-by: Patrick Delaunay > Signed-off-by: Marek Vasut > Cc: Manivannan Sadhasivam > Cc: Patrick Delaunay > Cc: Patrice Chotard > --- > V2: Move this patch before the split of AV96 into SoM and carrier > V3: No change > --- This patch update the PLL4 frequency used on AV96 board, with different of reference clock tree used on ST board, this new setting allow to optimize the SDMMC frequency (50MHz vs 30Mz). I don't know why the previous PLL4 frequency was chosen as a compromise on reference clock-tree (PLL4 is used by mostly all the peripheral, with display and audio requirements). Can you cross check the proposed clock tree and ack this patch if these ST requirements are not applicable on AV96 board. Anyway the code is correct. Reviewed-by: Patrick Delaunay Thanks Patrick
