| Message ID | 20181221115946.10095-4-rplsssn@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for PDC timer for wake-ups | expand |
Quoting Raju P.L.S.S.S.N (2018-12-21 03:59:44) > diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt > index 9b86d1eff219..f24afb45d0d9 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt > +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt > @@ -30,6 +30,12 @@ will be an aggregate of the sleep votes from each of those subsystems. Clients > may request a sleep value for their shared resources in addition to the active > mode requests. > > +When the processor enters deepest low power mode, the next wake-up timer value > +needs to be programmed to PDC (Power Domain Controller) through RSC registers > +dedicated for this purpose. PDC timer is specified as child node of RSC with > +register offets to program timer match value. That's great info, but I have no idea why it's in the DT binding document. > + > + > Properties: > > - compatible: > @@ -86,6 +92,20 @@ Properties: > Drivers that want to use the RSC to communicate with RPMH must specify their > bindings as child nodes of the RSC controllers they wish to communicate with. > > +If an RSC needs to program next wake-up in the PDC timer, it must specify the > +binding as child node with the following properties: > + > +Properties: > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be "qcom,pdc-timer". > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: Specifies the offset of the control register. > + > Example 1: > > For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the > @@ -103,6 +123,9 @@ TCS-OFFSET: 0xD00 > <0x179d0000 0x10000>, > <0x179e0000 0x10000>; > reg-names = "drv-0", "drv-1", "drv-2"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > @@ -112,6 +135,12 @@ TCS-OFFSET: 0xD00 > <SLEEP_TCS 3>, > <WAKE_TCS 3>, > <CONTROL_TCS 1>; > + > + pdc_timer@38 { > + compatible = "qcom,pdc-timer"; > + reg = <0x38 0x1>, > + <0x40 0x1>; I don't understand this whole binding. Why can't the pdc timer be programmed within the rpmh driver? This looks like a node is being added as a child just to make a platform driver and device match up in the linux kernel. And that in turn causes a regmap to need to be created? Sorry, it just looks really bad. At least for the regulators we have a semi-good reason to have a subnode because of the pmic-id property that we need to match up to the right pmic. The argument for the rpmh clock node is unclear to me so we should probably get rid of that node entirely. I can't figure out why that wasn't just a #clock-cells at the toplevel rsc node. The same goes for the interconnect and rpmhpd bindings. It's all sub-nodes to placate linux device driver model. And now that I've had to look at the rpmh binding again I'm disappointed that we have SoC data stored in there with the qcom,tcs-offset and qcom,tcs-config properties. Just make an SoC compatible like qcom,rpmh-rsc-sdm845" and figure these things out that way. And please tell hardware folks to stop moving the register regions around in the future so that there doesn't have to be so many variants in the driver and compatible string space.
On 12/22/2018 1:09 PM, Stephen Boyd wrote: >> +If an RSC needs to program next wake-up in the PDC timer, it must specify the >> +binding as child node with the following properties: >> + >> +Properties: >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: must be "qcom,pdc-timer". >> + >> +- reg: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: Specifies the offset of the control register. >> + >> Example 1: >> >> For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the >> @@ -103,6 +123,9 @@ TCS-OFFSET: 0xD00 >> <0x179d0000 0x10000>, >> <0x179e0000 0x10000>; >> reg-names = "drv-0", "drv-1", "drv-2"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; >> @@ -112,6 +135,12 @@ TCS-OFFSET: 0xD00 >> <SLEEP_TCS 3>, >> <WAKE_TCS 3>, >> <CONTROL_TCS 1>; >> + >> + pdc_timer@38 { >> + compatible = "qcom,pdc-timer"; >> + reg = <0x38 0x1>, >> + <0x40 0x1>; > I don't understand this whole binding. Why can't the pdc timer be > programmed within the rpmh driver? This looks like a node is being added > as a child just to make a platform driver and device match up in the > linux kernel. And that in turn causes a regmap to need to be created? > Sorry, it just looks really bad. There are two RSC devices in SoC one for application processor subsystem & other display subsystem. Both RSC contain registers for PDC timers (one for each subsystem). But only for application processor the PDC timer needs to be programmed when application processor enters sleep/suspend. As the driver is common between both RSC devices, this approach is taken. Do you have any other suggestions to distinguish between the two? Perhaps, by additional compatible string? Thanks for the review. - Raju
Quoting Raju P L S S S N (2018-12-26 01:44:43) > > > On 12/22/2018 1:09 PM, Stephen Boyd wrote: > >> +If an RSC needs to program next wake-up in the PDC timer, it must specify the > >> +binding as child node with the following properties: > >> + > >> +Properties: > >> +- compatible: > >> + Usage: required > >> + Value type: <string> > >> + Definition: must be "qcom,pdc-timer". > >> + > >> +- reg: > >> + Usage: required > >> + Value type: <prop-encoded-array> > >> + Definition: Specifies the offset of the control register. > >> + > >> Example 1: > >> > >> For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the > >> @@ -103,6 +123,9 @@ TCS-OFFSET: 0xD00 > >> <0x179d0000 0x10000>, > >> <0x179e0000 0x10000>; > >> reg-names = "drv-0", "drv-1", "drv-2"; > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + ranges; > >> interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, > >> <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, > >> <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > >> @@ -112,6 +135,12 @@ TCS-OFFSET: 0xD00 > >> <SLEEP_TCS 3>, > >> <WAKE_TCS 3>, > >> <CONTROL_TCS 1>; > >> + > >> + pdc_timer@38 { > >> + compatible = "qcom,pdc-timer"; > >> + reg = <0x38 0x1>, > >> + <0x40 0x1>; > > I don't understand this whole binding. Why can't the pdc timer be > > programmed within the rpmh driver? This looks like a node is being added > > as a child just to make a platform driver and device match up in the > > linux kernel. And that in turn causes a regmap to need to be created? > > Sorry, it just looks really bad. > > > There are two RSC devices in SoC one for application processor subsystem > & other display subsystem. Both RSC contain registers for PDC timers > (one for each subsystem). When is the timer programmed on the display subsystem's RSC? It's hard to give advice without all the information. > But only for application processor the PDC > timer needs to be programmed when application processor enters > sleep/suspend. As the driver is common between both RSC devices, this > approach is taken. Do you have any other suggestions to distinguish > between the two? Perhaps, by additional compatible string? > Maybe compatible? I sort of doubt it though. Do all RSCs have a PDC timer? I would think that it would make sense for the application processor's RSC timer to be programmed from the broadcast timer mechanism in the kernel so that timers during idle work and suspend turns off the timer appropriately with a shutdown hook. I guess the PDC can't tell you the time though? It looks like a shadow (and limited) version of the ARM architected MMIO timer that we already program for the broadcast timer mechanism. Is that because even the MMIO timer can't wakeup the system in deep idle? Assuming that's true, it means the ARM MMIO timer can't always be used as the system wide broadcast mechanism because we need to augment it with the PDC timer to get the actual wakeup. Maybe we should be adding hooks into the broadcast timer mechanism to program this wakeup event hardware in addition to the ARM MMIO timer. Or we should stop using the ARM MMIO timer on these systems and read the system register based physical time in the RSC timer driver and register this 64-bit PDC register as the broadcast timer. So the time reading would be through sysreg and the wakeup programming would be done by writing the PDC timer. The assumption would be that we have access to the physical time registers (which sounds like the assumption we have to make). Do we get an interrupt somewhere from the RSC hardware when the timer fires? Or does that just cause a system wakeup event without any pending irq and then another irq (like the ARM architected timer) just happens to be pending around the same time? If we get an interrupt somehow then I would prefer to drop the ARM MMIO timer and do this hybrid broadcast timer approach. How the RSC is used in general by other devices, like display, is not clear to me. We don't have a "wakeup event" framework in the kernel that device drivers like the display driver can grab a reference to and program some system wide wakeup for. That sounds like something new that could be handled entirely in the display driver with direct register writes, or it could be some qcom specific API/framework that eventually calls down into the same RSC driver that knows what offsets to write into in the display RSC's register space, or it could be an entirely generic framework like clk or regulator frameworks that could be used by anything. BTW, are we using the display RSC yet?
On 12/29/2018 3:08 AM, Stephen Boyd wrote: > Quoting Raju P L S S S N (2018-12-26 01:44:43) >> >> >> On 12/22/2018 1:09 PM, Stephen Boyd wrote: >>>> +If an RSC needs to program next wake-up in the PDC timer, it must specify the >>>> +binding as child node with the following properties: >>>> + >>>> +Properties: >>>> +- compatible: >>>> + Usage: required >>>> + Value type: <string> >>>> + Definition: must be "qcom,pdc-timer". >>>> + >>>> +- reg: >>>> + Usage: required >>>> + Value type: <prop-encoded-array> >>>> + Definition: Specifies the offset of the control register. >>>> + >>>> Example 1: >>>> >>>> For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the >>>> @@ -103,6 +123,9 @@ TCS-OFFSET: 0xD00 >>>> <0x179d0000 0x10000>, >>>> <0x179e0000 0x10000>; >>>> reg-names = "drv-0", "drv-1", "drv-2"; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, >>>> <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, >>>> <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; >>>> @@ -112,6 +135,12 @@ TCS-OFFSET: 0xD00 >>>> <SLEEP_TCS 3>, >>>> <WAKE_TCS 3>, >>>> <CONTROL_TCS 1>; >>>> + >>>> + pdc_timer@38 { >>>> + compatible = "qcom,pdc-timer"; >>>> + reg = <0x38 0x1>, >>>> + <0x40 0x1>; >>> I don't understand this whole binding. Why can't the pdc timer be >>> programmed within the rpmh driver? This looks like a node is being added >>> as a child just to make a platform driver and device match up in the >>> linux kernel. And that in turn causes a regmap to need to be created? >>> Sorry, it just looks really bad. >> >> >> There are two RSC devices in SoC one for application processor subsystem >> & other display subsystem. Both RSC contain registers for PDC timers >> (one for each subsystem). > > When is the timer programmed on the display subsystem's RSC? It's hard > to give advice without all the information. For display subsystem RSC, hardware sleep solver takes care of timer programming for wakeup when the subsystem goes to Power collapse. > >> But only for application processor the PDC >> timer needs to be programmed when application processor enters >> sleep/suspend. As the driver is common between both RSC devices, this >> approach is taken. Do you have any other suggestions to distinguish >> between the two? Perhaps, by additional compatible string? >> > > Maybe compatible? I sort of doubt it though. Do all RSCs have a PDC > timer? Yes. all RSCs have their own PDC timer. > > I would think that it would make sense for the application processor's > RSC timer to be programmed from the broadcast timer mechanism in the > kernel so that timers during idle work and suspend turns off the timer > appropriately with a shutdown hook. I guess the PDC can't tell you the > time though? It looks like a shadow (and limited) version of the ARM > architected MMIO timer that we already program for the broadcast timer > mechanism. Is that because even the MMIO timer can't wakeup the system > in deep idle? Assuming that's true, it means the ARM MMIO timer can't > always be used as the system wide broadcast mechanism because we need to > augment it with the PDC timer to get the actual wakeup. > Yes. this is correct. > Maybe we should be adding hooks into the broadcast timer mechanism to > program this wakeup event hardware in addition to the ARM MMIO timer. Or > we should stop using the ARM MMIO timer on these systems and read the > system register based physical time in the RSC timer driver and register > this 64-bit PDC register as the broadcast timer. So the time reading > would be through sysreg and the wakeup programming would be done by > writing the PDC timer. The assumption would be that we have access to > the physical time registers (which sounds like the assumption we have to > make). There are no physical timer registers available in RSC for this purpose. > > Do we get an interrupt somewhere from the RSC hardware when the timer > fires? Or does that just cause a system wakeup event without any pending > irq and then another irq (like the ARM architected timer) just happens > to be pending around the same time? If we get an interrupt somehow then > I would prefer to drop the ARM MMIO timer and do this hybrid broadcast > timer approach. There is no interrupt for PDC timeout. It just causes system wakeup without a pending irq. ARM MMIO is necessary for irq. > > How the RSC is used in general by other devices, like display, is not > clear to me. We don't have a "wakeup event" framework in the kernel that > device drivers like the display driver can grab a reference to and > program some system wide wakeup for. That sounds like something new that > could be handled entirely in the display driver with direct register > writes, or it could be some qcom specific API/framework that eventually > calls down into the same RSC driver that knows what offsets to write > into in the display RSC's register space, or it could be an entirely > generic framework like clk or regulator frameworks that could be used by > anything. BTW, are we using the display RSC yet? > Only display subsystem RSC is programmed along with CPU RSC in Linux. display RSC instance is not present in upstream but it is present in downstream and used for resource communication purpose only.
Quoting Raju P L S S S N (2019-01-03 04:22:58) > > On 12/29/2018 3:08 AM, Stephen Boyd wrote: > > Quoting Raju P L S S S N (2018-12-26 01:44:43) > >> > >> There are two RSC devices in SoC one for application processor subsystem > >> & other display subsystem. Both RSC contain registers for PDC timers > >> (one for each subsystem). > > > > When is the timer programmed on the display subsystem's RSC? It's hard > > to give advice without all the information. > > For display subsystem RSC, hardware sleep solver takes care of timer > programming for wakeup when the subsystem goes to Power collapse. So the display subsystem doesn't need to program their PDC in their RSC from software? > > > > I would think that it would make sense for the application processor's > > RSC timer to be programmed from the broadcast timer mechanism in the > > kernel so that timers during idle work and suspend turns off the timer > > appropriately with a shutdown hook. I guess the PDC can't tell you the > > time though? It looks like a shadow (and limited) version of the ARM > > architected MMIO timer that we already program for the broadcast timer > > mechanism. Is that because even the MMIO timer can't wakeup the system > > in deep idle? Assuming that's true, it means the ARM MMIO timer can't > > always be used as the system wide broadcast mechanism because we need to > > augment it with the PDC timer to get the actual wakeup. > > > > Yes. this is correct. > > > Maybe we should be adding hooks into the broadcast timer mechanism to > > program this wakeup event hardware in addition to the ARM MMIO timer. Or > > we should stop using the ARM MMIO timer on these systems and read the > > system register based physical time in the RSC timer driver and register > > this 64-bit PDC register as the broadcast timer. So the time reading > > would be through sysreg and the wakeup programming would be done by > > writing the PDC timer. The assumption would be that we have access to > > the physical time registers (which sounds like the assumption we have to > > make). > > There are no physical timer registers available in RSC for this purpose. > > > > > Do we get an interrupt somewhere from the RSC hardware when the timer > > fires? Or does that just cause a system wakeup event without any pending > > irq and then another irq (like the ARM architected timer) just happens > > to be pending around the same time? If we get an interrupt somehow then > > I would prefer to drop the ARM MMIO timer and do this hybrid broadcast > > timer approach. > > There is no interrupt for PDC timeout. It just causes system wakeup > without a pending irq. ARM MMIO is necessary for irq. Does that system wakeup cause the CPUs to be enabled? So the sysreg based timer in the CPU would start working? Or does it only make the system come out of a deep enough idle state to make an always on domain work that only contains the MMIO based ARM architected timer? I'd hope that each RSC's PDC timer wakes up the owner of the RSC so that we can use the sysreg based timers and ignore the MMIO based timers here. This isn't a very important distinction to make though, so if you have to use the MMIO timers then it just means some extra DT things need to be done to relate the MMIO timers with the RSC that has the timer that needs to be programmed too. Either way we would need a way to either hook the ARM architected timer driver in the kernel, or reimplement the bit of code needed to implement the clockevent based on the ARM architected timer that programs the ARM timer and the PDC timer together. > > > > > How the RSC is used in general by other devices, like display, is not > > clear to me. We don't have a "wakeup event" framework in the kernel that > > device drivers like the display driver can grab a reference to and > > program some system wide wakeup for. That sounds like something new that > > could be handled entirely in the display driver with direct register > > writes, or it could be some qcom specific API/framework that eventually > > calls down into the same RSC driver that knows what offsets to write > > into in the display RSC's register space, or it could be an entirely > > generic framework like clk or regulator frameworks that could be used by > > anything. BTW, are we using the display RSC yet? > > > > Only display subsystem RSC is programmed along with CPU RSC in Linux. > display RSC instance is not present in upstream but it is present in > downstream and used for resource communication purpose only. From the comment above it sounds like the display RSC handles the wakeup programming in hardware? So there isn't a need to add a 'wakeup event' framework or anything like that. Please correct me if I'm wrong.
On 1/4/2019 2:49 AM, Stephen Boyd wrote: > Quoting Raju P L S S S N (2019-01-03 04:22:58) >> >> On 12/29/2018 3:08 AM, Stephen Boyd wrote: >>> Quoting Raju P L S S S N (2018-12-26 01:44:43) >>>> >>>> There are two RSC devices in SoC one for application processor subsystem >>>> & other display subsystem. Both RSC contain registers for PDC timers >>>> (one for each subsystem). >>> >>> When is the timer programmed on the display subsystem's RSC? It's hard >>> to give advice without all the information. >> >> For display subsystem RSC, hardware sleep solver takes care of timer >> programming for wakeup when the subsystem goes to Power collapse. > > So the display subsystem doesn't need to program their PDC in their RSC > from software? Yes. > >>> >>> I would think that it would make sense for the application processor's >>> RSC timer to be programmed from the broadcast timer mechanism in the >>> kernel so that timers during idle work and suspend turns off the timer >>> appropriately with a shutdown hook. I guess the PDC can't tell you the >>> time though? It looks like a shadow (and limited) version of the ARM >>> architected MMIO timer that we already program for the broadcast timer >>> mechanism. Is that because even the MMIO timer can't wakeup the system >>> in deep idle? Assuming that's true, it means the ARM MMIO timer can't >>> always be used as the system wide broadcast mechanism because we need to >>> augment it with the PDC timer to get the actual wakeup. >>> >> >> Yes. this is correct. >> >>> Maybe we should be adding hooks into the broadcast timer mechanism to >>> program this wakeup event hardware in addition to the ARM MMIO timer. Or >>> we should stop using the ARM MMIO timer on these systems and read the >>> system register based physical time in the RSC timer driver and register >>> this 64-bit PDC register as the broadcast timer. So the time reading >>> would be through sysreg and the wakeup programming would be done by >>> writing the PDC timer. The assumption would be that we have access to >>> the physical time registers (which sounds like the assumption we have to >>> make). >> >> There are no physical timer registers available in RSC for this purpose. >> >>> >>> Do we get an interrupt somewhere from the RSC hardware when the timer >>> fires? Or does that just cause a system wakeup event without any pending >>> irq and then another irq (like the ARM architected timer) just happens >>> to be pending around the same time? If we get an interrupt somehow then >>> I would prefer to drop the ARM MMIO timer and do this hybrid broadcast >>> timer approach. >> >> There is no interrupt for PDC timeout. It just causes system wakeup >> without a pending irq. ARM MMIO is necessary for irq. > > Does that system wakeup cause the CPUs to be enabled? So the sysreg > based timer in the CPU would start working? Or does it only make the > system come out of a deep enough idle state to make an always on domain > work that only contains the MMIO based ARM architected timer? It only makes the system come out of deep sleep state for MMIO timer to function. > > I'd hope that each RSC's PDC timer wakes up the owner of the RSC so that > we can use the sysreg based timers and ignore the MMIO based timers > here. This isn't a very important distinction to make though, so if you > have to use the MMIO timers then it just means some extra DT things need > to be done to relate the MMIO timers with the RSC that has the timer > that needs to be programmed too. > > Either way we would need a way to either hook the ARM architected timer > driver in the kernel, or reimplement the bit of code needed to implement > the clockevent based on the ARM architected timer that programs the ARM > timer and the PDC timer together. > Could you please provide some more details on the implementation? >> >>> >>> How the RSC is used in general by other devices, like display, is not >>> clear to me. We don't have a "wakeup event" framework in the kernel that >>> device drivers like the display driver can grab a reference to and >>> program some system wide wakeup for. That sounds like something new that >>> could be handled entirely in the display driver with direct register >>> writes, or it could be some qcom specific API/framework that eventually >>> calls down into the same RSC driver that knows what offsets to write >>> into in the display RSC's register space, or it could be an entirely >>> generic framework like clk or regulator frameworks that could be used by >>> anything. BTW, are we using the display RSC yet? >>> >> >> Only display subsystem RSC is programmed along with CPU RSC in Linux. >> display RSC instance is not present in upstream but it is present in >> downstream and used for resource communication purpose only. > > From the comment above it sounds like the display RSC handles the wakeup > programming in hardware? So there isn't a need to add a 'wakeup event' > framework or anything like that. Please correct me if I'm wrong. Yes. There is no need for any framework. From RSC driver point of view there is a need to differentiate apps_rsc vs display_rsc as SW programs PDC timer only in apps_rsc case. How about having a dt property to capture this? Thanks, Raju
On 1/7/2019 9:47 PM, Raju P L S S S N wrote: > > > On 1/4/2019 2:49 AM, Stephen Boyd wrote: >> Quoting Raju P L S S S N (2019-01-03 04:22:58) >>> >>> On 12/29/2018 3:08 AM, Stephen Boyd wrote: >>>> Quoting Raju P L S S S N (2018-12-26 01:44:43) >>>>> >>>>> There are two RSC devices in SoC one for application processor >>>>> subsystem >>>>> & other display subsystem. Both RSC contain registers for PDC timers >>>>> (one for each subsystem). >>>> >>>> When is the timer programmed on the display subsystem's RSC? It's hard >>>> to give advice without all the information. >>> >>> For display subsystem RSC, hardware sleep solver takes care of timer >>> programming for wakeup when the subsystem goes to Power collapse. >> >> So the display subsystem doesn't need to program their PDC in their RSC >> from software? > Yes. > >> >>>> >>>> I would think that it would make sense for the application processor's >>>> RSC timer to be programmed from the broadcast timer mechanism in the >>>> kernel so that timers during idle work and suspend turns off the timer >>>> appropriately with a shutdown hook. I guess the PDC can't tell you the >>>> time though? It looks like a shadow (and limited) version of the ARM >>>> architected MMIO timer that we already program for the broadcast timer >>>> mechanism. Is that because even the MMIO timer can't wakeup the system >>>> in deep idle? Assuming that's true, it means the ARM MMIO timer can't >>>> always be used as the system wide broadcast mechanism because we >>>> need to >>>> augment it with the PDC timer to get the actual wakeup. >>>> >>> >>> Yes. this is correct. >>> >>>> Maybe we should be adding hooks into the broadcast timer mechanism to >>>> program this wakeup event hardware in addition to the ARM MMIO >>>> timer. Or >>>> we should stop using the ARM MMIO timer on these systems and read the >>>> system register based physical time in the RSC timer driver and >>>> register >>>> this 64-bit PDC register as the broadcast timer. So the time reading >>>> would be through sysreg and the wakeup programming would be done by >>>> writing the PDC timer. The assumption would be that we have access to >>>> the physical time registers (which sounds like the assumption we >>>> have to >>>> make). >>> >>> There are no physical timer registers available in RSC for this purpose. >>> >>>> >>>> Do we get an interrupt somewhere from the RSC hardware when the timer >>>> fires? Or does that just cause a system wakeup event without any >>>> pending >>>> irq and then another irq (like the ARM architected timer) just happens >>>> to be pending around the same time? If we get an interrupt somehow then >>>> I would prefer to drop the ARM MMIO timer and do this hybrid broadcast >>>> timer approach. >>> >>> There is no interrupt for PDC timeout. It just causes system wakeup >>> without a pending irq. ARM MMIO is necessary for irq. >> >> Does that system wakeup cause the CPUs to be enabled? So the sysreg >> based timer in the CPU would start working? Or does it only make the >> system come out of a deep enough idle state to make an always on domain >> work that only contains the MMIO based ARM architected timer? > > It only makes the system come out of deep sleep state for MMIO timer to > function. > >> >> I'd hope that each RSC's PDC timer wakes up the owner of the RSC so that >> we can use the sysreg based timers and ignore the MMIO based timers >> here. This isn't a very important distinction to make though, so if you >> have to use the MMIO timers then it just means some extra DT things need >> to be done to relate the MMIO timers with the RSC that has the timer >> that needs to be programmed too. > >> >> Either way we would need a way to either hook the ARM architected timer >> driver in the kernel, or reimplement the bit of code needed to implement >> the clockevent based on the ARM architected timer that programs the ARM >> timer and the PDC timer together. >> > > Could you please provide some more details on the implementation? Hi Stephen, Regardless of implementation options about application processor subsytem PDC timer, I think there is a need to differentiate the fact that for application processor subsystem PDC timer programming is done by SW but not for display processor subsystem as HW sleep solver takes care of PDC timer during sleep entry/exit. How about having a dt property like qcom,pdc-timer-mode = "solver"/"sw" ? The current patch introduced client based model using regmap to achieve this differentiation between these two subsystems. By using the dt property, once rpmh-src driver instance for application subsystem can have PDC timer programing implemented. Let me know if there is another way. For implementation of PDC timer, I see the following based on above discussion - 1. Take the existing cpu_pm_notify approach and move the current series approach of programing PDC timer registers to RSC driver. This wouldn't involve any changes in clock_event_framework/broadcast framework. 2. Add api hooks (like reading the next wake up programmed) to ARM architecture timer driver so that the value is copied to PDC timer using the api with in RSC driver based on cpu_pm_notifications. 3. Changes in clockevent to program both ARM mem timer & PDC timer together. Could you please share some more details on this? Please let me know if the first approach is ok. Thanks, Raju. > > >>> >>>> >>>> How the RSC is used in general by other devices, like display, is not >>>> clear to me. We don't have a "wakeup event" framework in the kernel >>>> that >>>> device drivers like the display driver can grab a reference to and >>>> program some system wide wakeup for. That sounds like something new >>>> that >>>> could be handled entirely in the display driver with direct register >>>> writes, or it could be some qcom specific API/framework that eventually >>>> calls down into the same RSC driver that knows what offsets to write >>>> into in the display RSC's register space, or it could be an entirely >>>> generic framework like clk or regulator frameworks that could be >>>> used by >>>> anything. BTW, are we using the display RSC yet? >>>> >>> >>> Only display subsystem RSC is programmed along with CPU RSC in Linux. >>> display RSC instance is not present in upstream but it is present in >>> downstream and used for resource communication purpose only. >> >> From the comment above it sounds like the display RSC handles the wakeup >> programming in hardware? So there isn't a need to add a 'wakeup event' >> framework or anything like that. Please correct me if I'm wrong. > > Yes. There is no need for any framework. From RSC driver point of view > there is a need to differentiate apps_rsc vs display_rsc as SW programs > PDC timer only in apps_rsc case. How about having a dt property to > capture this? > > Thanks, > Raju
Quoting Raju P L S S S N (2019-01-08 21:34:32) > > > On 1/7/2019 9:47 PM, Raju P L S S S N wrote: > > > > > > On 1/4/2019 2:49 AM, Stephen Boyd wrote: > >> > >> I'd hope that each RSC's PDC timer wakes up the owner of the RSC so that > >> we can use the sysreg based timers and ignore the MMIO based timers > >> here. This isn't a very important distinction to make though, so if you > >> have to use the MMIO timers then it just means some extra DT things need > >> to be done to relate the MMIO timers with the RSC that has the timer > >> that needs to be programmed too. > > > >> > >> Either way we would need a way to either hook the ARM architected timer > >> driver in the kernel, or reimplement the bit of code needed to implement > >> the clockevent based on the ARM architected timer that programs the ARM > >> timer and the PDC timer together. > >> > > > > Could you please provide some more details on the implementation? > > Hi Stephen, > > Regardless of implementation options about application processor > subsytem PDC timer, I think there is a need to differentiate the fact > that for application processor subsystem PDC timer programming is done > by SW but not for display processor subsystem as HW sleep solver takes > care of PDC timer during sleep entry/exit. How about having a dt > property like qcom,pdc-timer-mode = "solver"/"sw" ? The current patch > introduced client based model using regmap to achieve this > differentiation between these two subsystems. By using the dt property, > once rpmh-src driver instance for application subsystem can have PDC > timer programing implemented. Let me know if there is another way. > > For implementation of PDC timer, I see the following based on above > discussion - > 1. Take the existing cpu_pm_notify approach and move the current series > approach of programing PDC timer registers to RSC driver. This wouldn't > involve any changes in clock_event_framework/broadcast framework. > > 2. Add api hooks (like reading the next wake up programmed) to ARM > architecture timer driver so that the value is copied to PDC timer using > the api with in RSC driver based on cpu_pm_notifications. > > 3. Changes in clockevent to program both ARM mem timer & PDC timer > together. Could you please share some more details on this? > > > Please let me know if the first approach is ok. > The first approach requires that we expose internals of the clockevent and broadcast timer information to drivers. From my perspective it looks like a weird kludge to workaround the fact that the broadcast timer doesn't actually work on this platform. That's why I'm suggesting that you fix the broadcast timer on your platform to actually work, and do that within the clockevent/broadcast layers instead of indirecting that through cpu_pm notifiers. That could be done by making a PDC clockevent that has some DT binding of a property pointing to an MMIO timer frame and then reimplementing the MMIO timer code in the PDC driver on top of the frame/register region it pulls out of there. Or it could be written in reverse by having the generic MMIO timer driver point to the PDC somehow and implement some platform specific API to pass that information to the real wakeup programming part in PDC. I'm leaning toward the first approach where PDC is the clockevent and that uses the MMIO timer on the backend to do what it needs to program a wakeup. That way you can mandate the usage of the physical timer and keep this quirk away from the ARM timer driver. It also makes the idea of a qcom,pdc-timer-mode sort of useless because the PDC will have a property that points to the timer frame and that will mean "use this for broadcast wakeup". I'm not sure how the ARM folks feel about this though. It would probably require some sort of ARM timer API that lets us program the MMIO timer frame from the PDC driver. So exporting arch_timer_reg_write() and making that always inlined to optimize hot paths would be required.
On 1/9/2019 11:16 PM, Stephen Boyd wrote: > Quoting Raju P L S S S N (2019-01-08 21:34:32) >> >> >> On 1/7/2019 9:47 PM, Raju P L S S S N wrote: >>> >>> >>> On 1/4/2019 2:49 AM, Stephen Boyd wrote: >>>> >>>> I'd hope that each RSC's PDC timer wakes up the owner of the RSC so that >>>> we can use the sysreg based timers and ignore the MMIO based timers >>>> here. This isn't a very important distinction to make though, so if you >>>> have to use the MMIO timers then it just means some extra DT things need >>>> to be done to relate the MMIO timers with the RSC that has the timer >>>> that needs to be programmed too. >>> >>>> >>>> Either way we would need a way to either hook the ARM architected timer >>>> driver in the kernel, or reimplement the bit of code needed to implement >>>> the clockevent based on the ARM architected timer that programs the ARM >>>> timer and the PDC timer together. >>>> >>> >>> Could you please provide some more details on the implementation? >> >> Hi Stephen, >> >> Regardless of implementation options about application processor >> subsytem PDC timer, I think there is a need to differentiate the fact >> that for application processor subsystem PDC timer programming is done >> by SW but not for display processor subsystem as HW sleep solver takes >> care of PDC timer during sleep entry/exit. How about having a dt >> property like qcom,pdc-timer-mode = "solver"/"sw" ? The current patch >> introduced client based model using regmap to achieve this >> differentiation between these two subsystems. By using the dt property, >> once rpmh-src driver instance for application subsystem can have PDC >> timer programing implemented. Let me know if there is another way. >> >> For implementation of PDC timer, I see the following based on above >> discussion - >> 1. Take the existing cpu_pm_notify approach and move the current series >> approach of programing PDC timer registers to RSC driver. This wouldn't >> involve any changes in clock_event_framework/broadcast framework. >> >> 2. Add api hooks (like reading the next wake up programmed) to ARM >> architecture timer driver so that the value is copied to PDC timer using >> the api with in RSC driver based on cpu_pm_notifications. >> >> 3. Changes in clockevent to program both ARM mem timer & PDC timer >> together. Could you please share some more details on this? >> >> >> Please let me know if the first approach is ok. >> > > The first approach requires that we expose internals of the clockevent > and broadcast timer information to drivers. From my perspective it looks > like a weird kludge to workaround the fact that the broadcast timer > doesn't actually work on this platform. That's why I'm suggesting that > you fix the broadcast timer on your platform to actually work, and do > that within the clockevent/broadcast layers instead of indirecting that > through cpu_pm notifiers. > > That could be done by making a PDC clockevent that has some DT binding > of a property pointing to an MMIO timer frame and then reimplementing > the MMIO timer code in the PDC driver on top of the frame/register > region it pulls out of there. Or it could be written in reverse by > having the generic MMIO timer driver point to the PDC somehow and > implement some platform specific API to pass that information to the > real wakeup programming part in PDC. > > I'm leaning toward the first approach where PDC is the clockevent and > that uses the MMIO timer on the backend to do what it needs to program a > wakeup. That way you can mandate the usage of the physical timer and > keep this quirk away from the ARM timer driver. It also makes the idea > of a qcom,pdc-timer-mode sort of useless because the PDC will have a > property that points to the timer frame and that will mean "use this for > broadcast wakeup". I'm not sure how the ARM folks feel about this > though. It would probably require some sort of ARM timer API that lets > us program the MMIO timer frame from the PDC driver. So exporting > arch_timer_reg_write() and making that always inlined to optimize hot > paths would be required. > Regarding the first approach - If PDC clk_evt_dev is created and registered to broadcast framework, it means it will replace MMIO timer event device in broadcast framework (as only one broadcast event timer can exist). So in registration phase, the tick broadcast framework would configure broadcat event handler for PDC(clk_evt_dev). Even if we have some way of exporting arch_timer_reg_write() to program the wakeup for MMIO timer, the irq handler for MMIO timer is responsible to invoke event_handler. However, since MMIO timer is replaced by PDC timer event device, event handler will not be able to inform broadcast framework. What can be done about this?
Quoting Raju P L S S S N (2019-01-10 08:58:41) > > > On 1/9/2019 11:16 PM, Stephen Boyd wrote: > > That could be done by making a PDC clockevent that has some DT binding > > of a property pointing to an MMIO timer frame and then reimplementing > > the MMIO timer code in the PDC driver on top of the frame/register > > region it pulls out of there. Or it could be written in reverse by > > having the generic MMIO timer driver point to the PDC somehow and > > implement some platform specific API to pass that information to the > > real wakeup programming part in PDC. > > > > I'm leaning toward the first approach where PDC is the clockevent and > > that uses the MMIO timer on the backend to do what it needs to program a > > wakeup. That way you can mandate the usage of the physical timer and > > keep this quirk away from the ARM timer driver. It also makes the idea > > of a qcom,pdc-timer-mode sort of useless because the PDC will have a > > property that points to the timer frame and that will mean "use this for > > broadcast wakeup". I'm not sure how the ARM folks feel about this > > though. It would probably require some sort of ARM timer API that lets > > us program the MMIO timer frame from the PDC driver. So exporting > > arch_timer_reg_write() and making that always inlined to optimize hot > > paths would be required. > > > > Regarding the first approach - > If PDC clk_evt_dev is created and registered to broadcast framework, it > means it will replace MMIO timer event device in broadcast framework (as > only one broadcast event timer can exist). So in registration phase, the > tick broadcast framework would configure broadcat event handler for > PDC(clk_evt_dev). Yes all the MMIO timer frames would need to be marked as "disabled" I suppose, or somehow we would need to tell the MMIO timer driver to not register the MMIO timer frame from there because it's used by the PDC now. Maybe that would require changing the MMIO timer's compatible string to be something different. > > Even if we have some way of exporting arch_timer_reg_write() to program > the wakeup for MMIO timer, the irq handler for MMIO timer is responsible > to invoke event_handler. However, since MMIO timer is replaced by PDC > timer event device, event handler will not be able to inform broadcast > framework. What can be done about this? > It sounds like you imagine that the MMIO timer frame will still be active and in use by the ARM architected timer driver? I wasn't thinking that would be the case. I was thinking that the PDC code would basically reimplement all of the ARM timer code again by calling some shared timer code so that we avoid code duplication. Including the irq handler that would be registered for by the PDC driver and flow from that piece of code instead of the ARM timer code.
diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt index 9b86d1eff219..f24afb45d0d9 100644 --- a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt @@ -30,6 +30,12 @@ will be an aggregate of the sleep votes from each of those subsystems. Clients may request a sleep value for their shared resources in addition to the active mode requests. +When the processor enters deepest low power mode, the next wake-up timer value +needs to be programmed to PDC (Power Domain Controller) through RSC registers +dedicated for this purpose. PDC timer is specified as child node of RSC with +register offets to program timer match value. + + Properties: - compatible: @@ -86,6 +92,20 @@ Properties: Drivers that want to use the RSC to communicate with RPMH must specify their bindings as child nodes of the RSC controllers they wish to communicate with. +If an RSC needs to program next wake-up in the PDC timer, it must specify the +binding as child node with the following properties: + +Properties: +- compatible: + Usage: required + Value type: <string> + Definition: must be "qcom,pdc-timer". + +- reg: + Usage: required + Value type: <prop-encoded-array> + Definition: Specifies the offset of the control register. + Example 1: For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the @@ -103,6 +123,9 @@ TCS-OFFSET: 0xD00 <0x179d0000 0x10000>, <0x179e0000 0x10000>; reg-names = "drv-0", "drv-1", "drv-2"; + #address-cells = <1>; + #size-cells = <1>; + ranges; interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; @@ -112,6 +135,12 @@ TCS-OFFSET: 0xD00 <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + + pdc_timer@38 { + compatible = "qcom,pdc-timer"; + reg = <0x38 0x1>, + <0x40 0x1>; + } }; Example 2:
Add device binding documentation for Qualcomm Technology Inc's PDC timer. The driver is used for programming next wake-up timer value when processor enters SoC level deepest low power state. Cc: devicetree@vger.kernel.org Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org> --- .../devicetree/bindings/soc/qcom/rpmh-rsc.txt | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)