Re: [PATCH 00/53] icc-rpmh multi-RSC voting groundwork

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 7.08.2023 23:57, Mike Tipton wrote:
> On Thu, Aug 03, 2023 at 07:48:08PM +0300, Georgi Djakov wrote:
>> Hi Konrad,
>>
>> On 11.07.23 15:17, Konrad Dybcio wrote:
>>> Many parts of Qualcomm SoCs are entirely independent of each other and can
>>> run when the other parts are off. The RPMh system architecture embraces
>>> this by giving each (loosely defined) subsystem its own connection (as in,
>>> physical wires) to the AOSS, terminated by per-subsystem RSCs (Resource
>>> State Coordinators) that barter for power, bandwidth etc.
>>>
>>> This series introduces the groundwork necessary for voting for resources
>>> through non-APPS RSCs. It should allow for lower-latency vote adjustments
>>> (e.g. for very high bandwidth / multiple displays) and could potentially
>>> allow for full APSS collapse while keeping e.g. MDSS operating (say
>>> refreshing an image from a RAM buffer).
>>
>> This is good stuff. Thanks for working on it! Actually the path tagging,
>> that have been introduced some time ago could be used for supporting the
>> multiple RSCs. Today we can get the tags from DT, and tag the path with
>> some DISP_RSC flag (for example) and avoid the qcom,bcm-voter-idx property.
>>
>> Mike has been also looking into this, so maybe he can share his thoughts.
>>
> 
> Yeah, the current way we've been supporting multiple voters (e.g. RSCs)
> doesn't scale. We currently duplicate the topology for any path that
> requires a secondary, non-APSS voter. Which means we have duplicates
> nodes and bindings for each hop in those paths, even though there's only
> a single logical path.
> 
> For example, in qcom/sm8550.c, each node and BCM ending with _disp,
> _ife_0, _ife_1, or _ife_2 is a duplicate. The only reason they exist is
> to allow clients to target their votes to the non-APPS voters. And to
> provide separate, voter-specific buckets of aggregation. But everything
> else about them is 100% identical to their default APPS counterparts.
> For sm8550, this amounts to roughly 643 extra lines of code.
> 
> Initially there was only the one secondary display voter, so the scaling
> problem wasn't a huge issue. But sm8550 has four voters. And future SOCs
> will have even more.
> 
> We should only define the logical topology once. The ratio of NOC ports
> to interconnect nodes should be 1:1, rather than 1:N where N is the
> number of voters that care about them.
> 
> The general idea is that we could use tags for this. So, instead of...
> 
>   path = icc_get(dev, MASTER_MDP_DISP, SLAVE_EBI1_DISP);
> 
> it would be...
> 
>   path = icc_get(dev, MASTER_MDP, SLAVE_EBI1);
>   icc_set_tag(path, QCOM_ICC_TAG_VOTER_DISP);
> 
> I have an early prototype with basic testing already. I can hopefully
> clean it up and post for review in the next couple of weeks.
I was initially not very happy with this approach (overloading tags
with additional information), but it grew on me over time.

My only concern is that if we reserve say bits 16-31 for path tags
(remember, dt-bindings are ABI), we may eventually run out of them.

Konrad