[edk2] EmbeddedPkg: fix PcdTimerPeriod value

Fix PcdTimerPeriod from 100,000 to 10,000. Since the unit base of
PcdTimerPeriod is 100ns.

100,000 * 100ns = 10ms (interval of hardware timer interrupt)
10,1000 * 100ns = 1ms (interval of hardware timer interrupt)

In GRUB, the required timer granularity is only 1ms. If interval of
hardware timer interrupt is 10ms in UEFI, GRUB will only get all
timer granularity >= 10ms. At least, the timeout of boot menu in
GRUB is much slower. Only the interval of hardware timer interrupt
is 1ms in UEFI, the timeout of boot menu in GRUB works well.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc   | 2 +-
 ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc | 2 +-
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc        | 2 +-
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc     | 2 +-
 BeagleBoardPkg/BeagleBoardPkg.dsc                           | 2 +-
 EmbeddedPkg/EmbeddedPkg.dec                                 | 2 +-
 EmbeddedPkg/EmbeddedPkg.dsc                                 | 2 +-
 Omap35xxPkg/Omap35xxPkg.dsc                                 | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

Thanks Haojian,

A few comments below. It looks sane to me, but the patches to
individual platforms may need some testing.

As a note to my future self, I should go through these platforms and
delete the default timer period from the platform-specific config
files and get it from a central config file instead.

/
    Leif

On Mon, Jul 13, 2015 at 10:25:12PM +0800, Haojian Zhuang wrote:
> Fix PcdTimerPeriod from 100,000 to 10,000. Since the unit base of
> PcdTimerPeriod is 100ns.
> 
> 100,000 * 100ns = 10ms (interval of hardware timer interrupt)
> 10,1000 * 100ns = 1ms (interval of hardware timer interrupt)
   10,000

> In GRUB, the required timer granularity is only 1ms. If interval of
> hardware timer interrupt is 10ms in UEFI, GRUB will only get all
> timer granularity >= 10ms. At least, the timeout of boot menu in
> GRUB is much slower. Only the interval of hardware timer interrupt
> is 1ms in UEFI, the timeout of boot menu in GRUB works well.

I would add that this is something that needs to be better handled in
GRUB as well (which I'm working on), but changing the default solves
the problem in the current versions of GRUB (and a 1ms timer
granularity is nice to have anyway).

Contributions to EDK2 must contain a contributed-under tag - like so:
Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---

Also, since EDK2 coding style mandates DOS-style line endings, these
get corrupted when sending a patch inline in an email. This is still
my preferred way of doing it, for reviewing purposes. But it can be
helpful to also put (after the ---) a link to the patch or patch
series in your personal git tree - so that people can access an
uncorrupted version of your patch.

>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc   | 2 +-
>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc | 2 +-
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc        | 2 +-
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc     | 2 +-
>  BeagleBoardPkg/BeagleBoardPkg.dsc                           | 2 +-
>  EmbeddedPkg/EmbeddedPkg.dec                                 | 2 +-
>  EmbeddedPkg/EmbeddedPkg.dsc                                 | 2 +-
>  Omap35xxPkg/Omap35xxPkg.dsc                                 | 2 +-
>  8 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc
> index a64f2df..2b13fd7 100644
> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc
> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A8.dsc
> @@ -105,7 +105,7 @@
>  
>    ## SP804 Timer
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|1000000
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000        # expressed in 100ns units, 100,000 x 100 ns = 10,000,000 ns = 10 ms
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000        # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicInterruptNum|36
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicBase|0x10011000
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerMetronomeBase|0x10011020
> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
> index a4bffb3..e5ec8d4 100644
> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb-RTSM-A9x2.dsc
> @@ -104,7 +104,7 @@
>  
>    ## SP804 Timer
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|1000000
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000        # expressed in 100ns units, 100,000 x 100 ns = 10,000,000 ns = 10 ms
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000        # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicInterruptNum|33
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicBase|0x10011000
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerMetronomeBase|0x10011020
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
> index ad4e7bf..248e54f 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA9x4.dsc
> @@ -138,7 +138,7 @@
>  
>    ## SP804 Timer
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|1000000
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000        # expressed in 100ns units, 100,000 x 100 ns = 10,000,000 ns = 10 ms
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000        # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicInterruptNum|34
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicBase|0x10011000
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPerformanceBase|0x10011020
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
> index 14d0cf5..d6870dc 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
> @@ -128,7 +128,7 @@
>  
>    ## SP804 Timer
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|1000000
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000        # expressed in 100ns units, 100,000 x 100 ns = 10,000,000 ns = 10 ms
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000        # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicInterruptNum|34
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPeriodicBase|0x1c110000
>    gArmPlatformTokenSpaceGuid.PcdSP804TimerPerformanceBase|0x1c110020
> diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
> index 9eb364e..375f03c 100644
> --- a/BeagleBoardPkg/BeagleBoardPkg.dsc
> +++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
> @@ -341,7 +341,7 @@
>    gArmTokenSpaceGuid.PcdCpuVectorBaseAddress|0x80008000
>    gArmTokenSpaceGuid.PcdCpuResetAddress|0x80008000
>  
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterPeriodInNanoseconds|77
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|13000000
>  
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index f557527..9f113fc 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -121,7 +121,7 @@
>  
>    gEmbeddedTokenSpaceGuid.PcdTimerBaseAddress|0x3c700000|UINT32|0x0000001c
>    gEmbeddedTokenSpaceGuid.PcdTimerVector|7|UINT32|0x0000001d
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000|UINT32|0x0000001e
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000|UINT32|0x0000001e
>    gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress|0x38e00000|UINT32|0x0000001f
>  
>    gEmbeddedTokenSpaceGuid.PcdMetronomeTickPeriod|100|UINT32|0x00000020
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index 6719eea..2005d8f 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -219,7 +219,7 @@
>  #
>    gEmbeddedTokenSpaceGuid.PcdTimerBaseAddress|0x3c700000
>    gEmbeddedTokenSpaceGuid.PcdTimerVector|7
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>  
>  [PcdsFixedAtBuild.IPF]
>    gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000
> diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
> index 8d8f521..cd963f3 100644
> --- a/Omap35xxPkg/Omap35xxPkg.dsc
> +++ b/Omap35xxPkg/Omap35xxPkg.dsc
> @@ -157,7 +157,7 @@
>    # Timers
>    gOmap35xxTokenSpaceGuid.PcdOmap35xxArchTimer|3
>    gOmap35xxTokenSpaceGuid.PcdOmap35xxFreeTimer|4
> -  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|100000
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterPeriodInNanoseconds|77
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPerformanceCounterFrequencyInHz|13000000
>  
> -- 
> 2.1.4
> 

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/

On Mon, 2015-07-13 at 16:04 +0100, Leif Lindholm wrote:
> Thanks Haojian,
> 
> A few comments below. It looks sane to me, but the patches to
> individual platforms may need some testing.
> 
> As a note to my future self, I should go through these platforms and
> delete the default timer period from the platform-specific config
> files and get it from a central config file instead.
> 
> /
>     Leif
> 
> On Mon, Jul 13, 2015 at 10:25:12PM +0800, Haojian Zhuang wrote:
> > Fix PcdTimerPeriod from 100,000 to 10,000. Since the unit base of
> > PcdTimerPeriod is 100ns.
> > 
> > 100,000 * 100ns = 10ms (interval of hardware timer interrupt)
> > 10,1000 * 100ns = 1ms (interval of hardware timer interrupt)
>    10,000
> 

Thanks for catching it. I'll deliver the next version to fix it.

> > In GRUB, the required timer granularity is only 1ms. If interval of
> > hardware timer interrupt is 10ms in UEFI, GRUB will only get all
> > timer granularity >= 10ms. At least, the timeout of boot menu in
> > GRUB is much slower. Only the interval of hardware timer interrupt
> > is 1ms in UEFI, the timeout of boot menu in GRUB works well.
> 
> I would add that this is something that needs to be better handled in
> GRUB as well (which I'm working on), but changing the default solves
> the problem in the current versions of GRUB (and a 1ms timer
> granularity is nice to have anyway).

After thinking a while, I think that we needn't to handle it in GRUB.

Aarch64 requires generic timer at least 1MHz. And SP804 is always
working in 1MHz in UEFI.

1 / 1MHz = 1us
1 / 1ms = 1KHz

And in current embedded SoC solutions, the lowest frequency of hardware
timer is 32.768KHz. So all of them could meet the requirement of timer
granularity = 1ms.

> 
> Contributions to EDK2 must contain a contributed-under tag - like so:
> Contributed-under: TianoCore Contribution Agreement 1.0
> 

Thanks. I'll append them.

> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > ---
> 
> Also, since EDK2 coding style mandates DOS-style line endings, these
> get corrupted when sending a patch inline in an email. This is still
> my preferred way of doing it, for reviewing purposes. But it can be
> helpful to also put (after the ---) a link to the patch or patch
> series in your personal git tree - so that people can access an
> uncorrupted version of your patch.
> 

I'll append the link.

Regards
Haojian


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/

On Tue, Jul 14, 2015 at 10:05:14AM +0800, Haojian Zhuang wrote:
> > > In GRUB, the required timer granularity is only 1ms. If interval of
> > > hardware timer interrupt is 10ms in UEFI, GRUB will only get all
> > > timer granularity >= 10ms. At least, the timeout of boot menu in
> > > GRUB is much slower. Only the interval of hardware timer interrupt
> > > is 1ms in UEFI, the timeout of boot menu in GRUB works well.
> > 
> > I would add that this is something that needs to be better handled in
> > GRUB as well (which I'm working on), but changing the default solves
> > the problem in the current versions of GRUB (and a 1ms timer
> > granularity is nice to have anyway).
> 
> After thinking a while, I think that we needn't to handle it in GRUB.
> 
> Aarch64 requires generic timer at least 1MHz. And SP804 is always
> working in 1MHz in UEFI.
> 
> 1 / 1MHz = 1us
> 1 / 1ms = 1KHz
> 
> And in current embedded SoC solutions, the lowest frequency of hardware
> timer is 32.768KHz. So all of them could meet the requirement of timer
> granularity = 1ms.

Yes, they certainly can.
However, we cannot force all platforms to comply - so GRUB still needs
to be able to do something sane (or at least warn that something is
amiss).

Regards,

Leif

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/