Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

From: Lina Iyer
Date: Wed Apr 18 2018 - 15:31:24 EST

On Mon, Apr 16 2018 at 00:01 -0600, Stephen Boyd wrote:
Quoting Lina Iyer (2018-04-16 09:08:18)
On Fri, Apr 13 2018 at 16:40 -0600, Stephen Boyd wrote:
>Well it seems like an RSC contains many DRVs and those DRVs contain many
>TCSes. This is what I get after talking with Bjorn on IRC.
> +--------------------------------------------------+ (0x00000)
> | |
> | DRV #0 |
> | |
> |---------- --------------| (tcs-offset (0xd00))
> | DRV0_TCS0 |
> | common space |
> | cmd sequencer | 0xd00 + 0x14
> | |
> | DRV0_TCS1 |
> | common space | 0xd00 + 0x2a0
> | cmd sequencer | 0xd00 + 0x2a0 + 0x14
> | |
> | DRV0_TCS2 |
> | |
> | |
> +--------------------------------------------------+ (0x10000)
> | |
> | DRV #1 |
> | |
> |---------- --------------| (tcs-offset)
> | DRV1_TCS0 |
> | DRV1_TCS1 |
> | DRV1_TCS2 |
> +--------------------------------------------------+ (0x20000)
> | |
> | DRV #2 |
> | |
> |---------- --------------|
> | DRV2_TCS0 |
> | DRV2_TCS1 |
> | DRV2_TCS2 |
> | DRV2_TCS3 |
> | DRV2_TCS4 |
> | DRV2_TCS5 |
> +--------------------------------------------------+
>I think I understand it now. There aren't any "RSC common" registers
>that are common to the entire RSC. Instead, everything goes into a DRV,
>or into a common TCS space, or into a TCS "queue".
>> >Put another way, even if the "apps" RSC is complicated, we should be
>> >describing it to the best of our abilities in the binding so that when
>> >it is used by non-linux OSes things still work by simply tweaking the
>> >drv-id that we use to pick the right things out of the node.
>> >
>> >Or we're describing the RSC but it's really a container node that
>> >doesn't do much besides hold DRVs? So this is described at the wrong
>> >level?
>> What we are describing is a DRV, but a standalone DRV alone is useless
>> without the necessary RSC registers. So its a unique RSC+DRV combination
>> that is represented here.
>If my understanding is correct up there then the binding could either
>describe a single RSC DRV, or it could describe all the RSC DRV
>instances and interrupts going into the RSC "block" and then we can use
>drv-id to pick the offset we jump to.
Your understanding is correct.

>I imagine we don't have any practical use-case for the entire RSC space
>because there aren't any common RSC registers to deal with.
Not true.

So then my understanding is not correct! :/

>So we've
>boiled this all down to describing one DRV and then I wonder why we care
>about having drv-id at all? It looks to be used to check for a max
>number of TCS, but that's already described by DT so it doesn't seem
>very useful to double check what the hardware can tells us.
There is also a number of commands per TCS (NCPT), that may way vary
between different RSCs. The RSC of the application processor has 16
commands in each TCS, but that is variable. I am not saying it cannot be
described in DT, but it is something I read from the common RSC
registers, currently.
Also, I will using common/DRV0 registers to write wakeup time value,
when the processor subsystem goes into power down. This is not DRV2
register, but is a DRV0 register that we will have special access to.
The patches for those I intend to publish, when we have support for
sleep/suspend with this new architecture. So the address of the start of
the RSC (=DRV0) is necessary.

>Long story short, we can remove drv-id and just describe drvs by
Yes, we may. As long as I have a way to describe the register addresss
of the start of the DRV (0x20000 for DRV#2) and the tcs-offset (0xd00),
we can work with the RSC-DRV in the driver.

From this new information it seems like we need to know about all the
DRVs in the RSC then. So let's go back to my previous binding proposal
describing all of them and having the qcom,drv-id property describe
which one to use most of the time and hardcode the assumption to use
DRV0 in the driver when we need to do things for sleep/suspend? Then
we're back to describing the whole RSC and configuring the picker to
pick the right DRV depending on firmware configuration.

Hmm.. I am okay with that, even though it might be bit confusing to
define all that and not use them.

-- Lina