On 2/22/2024 6:00 PM, Konrad Dybcio wrote: > > > On 2/21/24 18:57, Odelu Kukatla wrote: >> >> >> On 2/5/2024 5:51 PM, Konrad Dybcio wrote: >>> On 5.02.2024 12:57, Odelu Kukatla (QUIC) wrote: >>>> -----Original Message----- >>>> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >>>> Sent: Monday, January 22, 2024 8:55 PM >>>> To: Odelu Kukatla (QUIC) <quic_okukatla@xxxxxxxxxxx>; georgi.djakov@xxxxxxxxxx; Bjorn Andersson <andersson@xxxxxxxxxx>; Georgi Djakov <djakov@xxxxxxxxxx>; linux-arm-msm@xxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx >>>> Subject: Re: [1/4] interconnect: qcom: icc-rpmh: Add QoS config support > > Why is this email taken private? > > [...] > I am adding previous list back for now, will add "to list" from "./scripts/get_maintainer.pl" in V3. >>> >> >> Yeah, you are right. !synced_state holds all BCMs enabled. But clock voting is still required. >> But qcom_icc_rpmh_configure_qos() can be invoked from out side of qcom_icc_rpmh_probe() in QuickBoot mode where *_probe does not happen. >> so irrespective of !synced_state votes, better to keep it separate from sync_state() and place votes on required BCMs during QoS time and remove them after that. > > What on earth is quickboot mode? > Grepping for it on linux-next, there's precisely 0 results. > > If it's some downstream sw invention, it has no right to affect > the code submitted upstream - "if it's not on the list, it doesn't > exist". > Quickboot is not there yet in upstream. I will clean the code related to BCM voting and move the required clock voting to qcom_icc_rpmh_configure_qos(). >> >>>>> + * @prio: priority value assigned to requests on the node >>>>> + * @urg_fwd: if set, master priority is used for requests. >>>> >>>> "master priority" meaning "this req goes before anyone else", or "use the icc provider [master]'s priority value"? >>>> >>>>> + * @prio_fwd_disable: if set, master priority is ignored and NOCs default priority is used. >>>> >>>> NoC's >>>> >>>> This sounds like !(prio || urg_fwd)? Surely it must do something more useful? >>> >>> This remains unanswered >>> >>> Konrad >> >> @prio_fwd_disable: >> 1 -> FIXED MODE; upstream tnx-inband-priority is ignored; value @pio is used for transaction. >> 0 -> BYPASS MODE; max of (tnx-inband-priority, @prio). > > So, this could be basically called "force_priority_val" > > >> @urg_fwd: >> 1 -> Urgency promotion messages issued by the connected master IP remain unchanged as they transit the NIU and the QNoC QoS generator. >> 0 -> IP-generated messages are discarded, and messages generated by QoS generators are used instead. > > So the interconnect endpoints can effectively modify the priority if > urg_fwd is enabled.. Interesting.. "ignore_ep_urgency_req"? > Names were chosen closer to bit fields in QoS registers. I will keep the names same and update the documentation with clear explanation. > Both of these should also be better documented, the kerneldoc you > submitted explains nothing unless this what I believe to be excerpt > from the documentation is present to support it. > > Konrad Thanks, Odelu