Re: [PATCH] tick: check if broadcast device could really be stopped

[Benjamin GAIGNARD <benjamin.gaignard@st.com>]

On 10/14/19 4:28 PM, Thomas Gleixner wrote:
> On Mon, 14 Oct 2019, Benjamin GAIGNARD wrote:
>> On 10/14/19 3:40 PM, Thomas Gleixner wrote:
>>> I don't understand what you are trying to achieve here. If the tick device,
>>> i.e. the comparator stops working in deep idle states, then the device has
>>> rightfully the CLOCK_EVT_FEAT_C3STOP (mis)feature set. Which means that the
>>> system needs a fallback device for the deep idle case. If there is no
>>> physical fallback device then you should enable the hrtimer based broadcast
>>> pseudo tick device.
>>>
>>> If the CPUs never go deep idle because there is no idle driver loaded or
>>> the deep power state in which the comparator stops working is never
>>> reached, then the broadcast hrtimer will never be used. It just eats a bit
>>> of memory and a few extra instructions.
>> Since CPUs won't go in deep idle I don't want to get a broadcast timer
>> but only an hrtimer to get accure latencies for the scheduler.
> What's wrong with the broadcast timer if it is never utilized? It's there,
> needs a few bytes of memory and that's it.
>
>> When arch arm timer doesn't set CLOCK_EVT_FEAT_C3STOP flag, my system got
>> a hrtimer and everything goes well.
> Sure, but that's applicable to your particular system only and not a
> generic solution. Neither your DT hack, nor the other one you posted.
>
>> If arch arm timer set CLOCK_EVT_FEAT_C3STOP hrtimer disappear (except if
>> I add an additional broadcast timer).
> Right.
>
>> What is the link between tick broadcast timer and hrtimer ? Does arch
>> arm timer could only implement them at the same time ?
> If the clock event device is affected by deep power states, then the core
> requires a fallback device, aka. broadcast timer, which makes sure that no
> event is lost in case that the CPU goes into a deep power state. If no CPU
> ever goes deep, the broadcast timer is there and unused.
>
> Obviously the system cannot enable high resolution timers if the clock
> event device is affected by power states.
>
> Unless you have a solution which works under all circumstances (and the
> current patch definitely does not) then you have to deal with the
> requirement of the broadcast timer (either physical or software based).

If I follow you I need, for my system, a software based broadcast timer 
(tick-broadcast-hrtimer ?).

If that is correct I 'just' need to add a call to 
tick_setup_hrtimer_broadcast() in arch/arm/kernel/time.c

Regards,

Benjamin

>
> "I don't want a broadcast timer falls" into the "I want a pony" realm.
>
> Thanks,
>
> 	tglx
>