Re: [PATCH v4 00/12] The great interconnecification fixation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14/02/2023 17:27, Konrad Dybcio wrote:
+CC Georgi, linux-pm, linux-kernel (thanks git send-email for not including these)

Time to switch to b4 prep ;-)


On 14.02.2023 15:37, Konrad Dybcio wrote:
Hi!

v3 -> v4 changelog:
- Drop "Always set QoS params on QNoC", it only causes issues.. this
   can be investigated another day, as it's not necessary for operation

- Drop "Add a way to always set QoS registers", same as /\

- Add a way (and use it) to have no bus_clocks (the ones we set rate on),
   as at least msm8996 has a bus (A0NoC) that doesn't have any and does
   all the scaling through RPM requests

- Promote 8996 icc to core_initcall

- Introduce keep_alive (see patch [11/12]) (important!, will be used by at least 6375)

- Allow negative QoS offsets in preparation for introducing 8998 icc [12/12]

Link to v3: https://lore.kernel.org/linux-arm-msm/20230116132152.405535-1-konrad.dybcio@xxxxxxxxxx/

v2 -> v3 changelog:
- Drop "Don't set QoS params before non-zero bw is requested"

- Rebase on next

- [1/9] ("..make QoS INVALID default.."): remove unused define for
   MODE_INVALID_VAL

- Pick up tags

v1 -> v2 changelog:
- reorder "make QoS INVALID default", makes more sense to have it
   before "Always set QoS params on QNoC"

- Limit ap_owned-independent QoS setting to QNoC only

- Add new patches for handling the 8996-and-friends clocks situation
   and optional BIMC regardless-of-ap_owned QoS programming


[1] https://lore.kernel.org/linux-arm-msm/14e06574-f95e-8960-0243-8c95a1c294e9@xxxxxxxxxx/T/#m056692bea71d4c272968d5e07afbd9eb07a88123
[2] https://lore.kernel.org/linux-arm-msm/20230110132202.956619-1-konrad.dybcio@xxxxxxxxxx/

This series grew quite a bit bigger than the previous [1] attempt, so
I decided to also add a cover letter.

Link to v2: [2]

It addresses a few things that were not quite right:

- Setting QoS params before a "real" (non-zero) bandwidth request
   makes little sense (since there's no data supposed to flow through
   the bus, why would the QoS matter) and (at least newer) downstream
   prevents that from happening. Do the same in Patch 1.

- QNoC type buses expect to always have their QoS registers set as long
   as there's a non-INVALID QoS mode set; ap_owned is not really a thing
   on these anymore, Patch 3 handles that.

- The recent MSM8996 boot fix was done quickly and not quite properly,
   leading to possibly setting the aggregate bus rate on "normal"
   hardware interface clocks; this series handles that by limiting the
   number of bus_clocks to 2 (which is the maximum that makes sense,
   anyway) and handling the rest as "intf_clocks", which are required
   to access the   hardware at the other end. Patches 5-8 take care of
   that and Patch 10 reverts the _optional moniker in clk_get_ to make
   sure we always have the bus scaling clocks, as they're well, kind
   of important ;)

- Similarly to QNoC, BIMC on "newer" (which can be loosely approximated
   by "new enough" == "has only BIMC and QNoC hosts") SoCs expects to
   always receive QoS programming, whereas BIMC on "older" SoCs cries
   like a wild boar and crashes the platform when trying to do so
   unconditionally. Patch 9 adds a way to take care of that for newer
   SoCs (like SM6375)

- QoS mode INVALID was assumed by developers before to be the default
   ("I didn't specify any QoS settings, so the driver can't assume I
   did.. right? right!?" - wrong, partial struct initialization led to
   0 being set and 0 corresponded to QoS mode FIXED). Make it so, as
   that's the logical choice. This allows the "Always set QoS params
   on QNoC" patch to work without setting tons of what-should-
   -obviously-be-the-default values everywhere, as well as fixes older
   drivers that set ap_owned = true but left the QoS mode field unset.
   Patch 2 cleans that up.

- Some nodes are physically connected over more than one channel
   (usually DDR or other high-throughput paths). Patch 4 allows that
   to be reflected in calculations. This will be required for at least
   MSM8998 and SM6375 (which will be submitted soon after this lands)


Konrad Dybcio (12):
   interconnect: qcom: rpm: make QoS INVALID default, separate out driver
     data
   interconnect: qcom: rpm: Add support for specifying channel num
   interconnect: qcom: Sort kerneldoc entries
   interconnect: qcom: rpm: Rename icc desc clocks to bus_blocks
   interconnect: qcom: rpm: Rename icc provider num_clocks to
     num_bus_clocks
   interconnect: qcom: rpm: Handle interface clocks
   interconnect: qcom: icc-rpm: Allow negative num_bus_clocks
   interconnect: qcom: msm8996: Specify no bus clock scaling on A0NoC
   interconnect: qcom: rpm: Don't use clk_get_optional for bus clocks
     anymore
   interconnect: qcom: msm8996: Promote to core_initcall
   interconnect: qcom: icc-rpm: Introduce keep_alive
   interconnect: qcom: icc-rpm: Allow negative QoS offset

  drivers/interconnect/qcom/icc-rpm.c | 101 ++++++++++++++++++++--------
  drivers/interconnect/qcom/icc-rpm.h |  41 +++++++----
  drivers/interconnect/qcom/msm8996.c |  35 ++++++----
  drivers/interconnect/qcom/sdm660.c  |  16 ++---
  4 files changed, 126 insertions(+), 67 deletions(-)





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux