On 5/8/24 03:45, Mike Tipton wrote:
Hi Konrad,
On Tue, Mar 26, 2024 at 08:42:35PM +0100, Konrad Dybcio wrote:
Currently, the QoS settings are stored in the node data, even though
they're a property of the bus/provider instead. Moreover, they are only
needed during the probe step, so they can be easily moved into struct
qcom_icc_desc.
The QoS settings *are* fundamentally a property of the node. The nodes
are 1:1 with the NOC ports. And the QoS settings tune the priority of
the data coming out of those ports. So, logically speaking, the QoS data
does belong in the node structs along with the rest of the data for that
node and port.
Only a subset of NOC ports support configurable QoS, but for those ports
that do it's a property of the port itself. Those settings impact just
that specific port and nothing else.
The current method of directly embedding the qcom_icc_qos_data struct
into qcom_icc_node isn't optimal, since that data is irrelevant for
ports that don't support it. So, the size could be optimized by
converting qcom_icc_node::qos into a pointer instead. But I don't think
we should separate the QoS settings from node struct entirely. It makes
it very difficult to understand which QoS settings are impacting which
port.
I think that would make more sense indeed
[...]
+static const struct qcom_icc_qos_data a0noc_qos_data[] = {
+ {
+ .qos_port = 0,
+ .qos_mode = NOC_QOS_MODE_FIXED,
+ .areq_prio = 1,
+ .prio_level = 1,
+ .urg_fwd_en = false,
+ .limit_commands = false,
+ }, {
How can I tell that these a0noc_qos_data[0] settings are for the
mas_pcie_0 port? It's not possible from the code anymore. *We* could
figure it out internally by looking at the NOC SWI to determine the
qos_port index. But this should be obvious from the code itself.
Right
Konrad
