| Message ID | 20250217-mhi_bw_up-v1-2-9bad1e42bdb1@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | bus: mhi: host: Add support for mhi bus bw | expand |
On Mon, 17 Feb 2025, Krishna Chaitanya Chundru wrote: > If the driver wants to move to higher data rate/speed than the current data > rate then the controller driver may need to change certain votes so that > link may come up at requested data rate/speed like QCOM PCIe controllers > need to change their RPMh (Resource Power Manager-hardened) state. Once > link retraining is done controller drivers needs to adjust their votes > based on the final data rate. > > Some controllers also may need to update their bandwidth voting like > ICC bw votings etc. > > So, add pre_scale_bus_bw() & post_scale_bus_bw() op to call before & after > the link re-train. > > In case of PCIe switch, if there is a request to change target speed for a > downstream port then no need to call these function ops as these are > outside the scope of the controller drivers. > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> > --- > drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++ > include/linux/pci.h | 2 ++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c > index 0a5e7efbce2c..e3faa4d1f935 100644 > --- a/drivers/pci/pcie/bwctrl.c > +++ b/drivers/pci/pcie/bwctrl.c > @@ -161,6 +161,8 @@ static int pcie_bwctrl_change_speed(struct pci_dev *port, u16 target_speed, bool > int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, > bool use_lt) > { > + struct pci_host_bridge *host = pci_find_host_bridge(port->bus); > + bool is_root = pci_is_root_bus(port->bus); is_rootport ? > struct pci_bus *bus = port->subordinate; > u16 target_speed; > int ret; > @@ -173,6 +175,16 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, > > target_speed = pcie_bwctrl_select_speed(port, speed_req); > > + /* > + * The controller driver may need to be scaled for targeted speed > + * otherwise link might not come up at requested speed. > + */ > + if (is_root && host->ops->pre_scale_bus_bw) { > + ret = host->ops->pre_scale_bus_bw(host->bus, target_speed); > + if (ret) > + return ret; > + } > + > scoped_guard(rwsem_read, &pcie_bwctrl_setspeed_rwsem) { > struct pcie_bwctrl_data *data = port->link_bwctrl; > > @@ -197,6 +209,9 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, > !list_empty(&bus->devices)) > ret = -EAGAIN; > > + if (is_root && host->ops->post_scale_bus_bw) > + host->ops->post_scale_bus_bw(host->bus, pci_bus_speed2lnkctl2(bus->cur_bus_speed)); Is the naming of these callbacks too specific for your use case? Does PCIe spec actually call changing the Target Speed "scaling bus bandwidth" or something along those line? > return ret; > } > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 47b31ad724fa..58f1de626c37 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -804,6 +804,8 @@ struct pci_ops { > void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); > int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); > int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); > + int (*pre_scale_bus_bw)(struct pci_bus *bus, int target_speed); > + void (*post_scale_bus_bw)(struct pci_bus *bus, int current_speed); Please document these, including the locking requirements.
On 2/17/2025 2:58 PM, Ilpo Järvinen wrote: > On Mon, 17 Feb 2025, Krishna Chaitanya Chundru wrote: > >> If the driver wants to move to higher data rate/speed than the current data >> rate then the controller driver may need to change certain votes so that >> link may come up at requested data rate/speed like QCOM PCIe controllers >> need to change their RPMh (Resource Power Manager-hardened) state. Once >> link retraining is done controller drivers needs to adjust their votes >> based on the final data rate. >> >> Some controllers also may need to update their bandwidth voting like >> ICC bw votings etc. >> >> So, add pre_scale_bus_bw() & post_scale_bus_bw() op to call before & after >> the link re-train. >> >> In case of PCIe switch, if there is a request to change target speed for a >> downstream port then no need to call these function ops as these are >> outside the scope of the controller drivers. >> >> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> >> --- >> drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++ >> include/linux/pci.h | 2 ++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c >> index 0a5e7efbce2c..e3faa4d1f935 100644 >> --- a/drivers/pci/pcie/bwctrl.c >> +++ b/drivers/pci/pcie/bwctrl.c >> @@ -161,6 +161,8 @@ static int pcie_bwctrl_change_speed(struct pci_dev *port, u16 target_speed, bool >> int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, >> bool use_lt) >> { >> + struct pci_host_bridge *host = pci_find_host_bridge(port->bus); >> + bool is_root = pci_is_root_bus(port->bus); > > is_rootport ? > ack. >> struct pci_bus *bus = port->subordinate; >> u16 target_speed; >> int ret; >> @@ -173,6 +175,16 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, >> >> target_speed = pcie_bwctrl_select_speed(port, speed_req); >> >> + /* >> + * The controller driver may need to be scaled for targeted speed >> + * otherwise link might not come up at requested speed. >> + */ >> + if (is_root && host->ops->pre_scale_bus_bw) { >> + ret = host->ops->pre_scale_bus_bw(host->bus, target_speed); >> + if (ret) >> + return ret; >> + } >> + >> scoped_guard(rwsem_read, &pcie_bwctrl_setspeed_rwsem) { >> struct pcie_bwctrl_data *data = port->link_bwctrl; >> >> @@ -197,6 +209,9 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, >> !list_empty(&bus->devices)) >> ret = -EAGAIN; >> >> + if (is_root && host->ops->post_scale_bus_bw) >> + host->ops->post_scale_bus_bw(host->bus, pci_bus_speed2lnkctl2(bus->cur_bus_speed)); > > Is the naming of these callbacks too specific for your use case? Does PCIe > spec actually call changing the Target Speed "scaling bus bandwidth" or > something along those line? > it is not the PCIe spec, I named based on current driver only. If there is better name I can modify it. >> return ret; >> } >> >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 47b31ad724fa..58f1de626c37 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -804,6 +804,8 @@ struct pci_ops { >> void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); >> int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); >> int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); >> + int (*pre_scale_bus_bw)(struct pci_bus *bus, int target_speed); >> + void (*post_scale_bus_bw)(struct pci_bus *bus, int current_speed); > > Please document these, including the locking requirements. ack. - Krishna Chaitanya. >
diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c index 0a5e7efbce2c..e3faa4d1f935 100644 --- a/drivers/pci/pcie/bwctrl.c +++ b/drivers/pci/pcie/bwctrl.c @@ -161,6 +161,8 @@ static int pcie_bwctrl_change_speed(struct pci_dev *port, u16 target_speed, bool int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, bool use_lt) { + struct pci_host_bridge *host = pci_find_host_bridge(port->bus); + bool is_root = pci_is_root_bus(port->bus); struct pci_bus *bus = port->subordinate; u16 target_speed; int ret; @@ -173,6 +175,16 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, target_speed = pcie_bwctrl_select_speed(port, speed_req); + /* + * The controller driver may need to be scaled for targeted speed + * otherwise link might not come up at requested speed. + */ + if (is_root && host->ops->pre_scale_bus_bw) { + ret = host->ops->pre_scale_bus_bw(host->bus, target_speed); + if (ret) + return ret; + } + scoped_guard(rwsem_read, &pcie_bwctrl_setspeed_rwsem) { struct pcie_bwctrl_data *data = port->link_bwctrl; @@ -197,6 +209,9 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req, !list_empty(&bus->devices)) ret = -EAGAIN; + if (is_root && host->ops->post_scale_bus_bw) + host->ops->post_scale_bus_bw(host->bus, pci_bus_speed2lnkctl2(bus->cur_bus_speed)); + return ret; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 47b31ad724fa..58f1de626c37 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -804,6 +804,8 @@ struct pci_ops { void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); + int (*pre_scale_bus_bw)(struct pci_bus *bus, int target_speed); + void (*post_scale_bus_bw)(struct pci_bus *bus, int current_speed); }; /*
If the driver wants to move to higher data rate/speed than the current data rate then the controller driver may need to change certain votes so that link may come up at requested data rate/speed like QCOM PCIe controllers need to change their RPMh (Resource Power Manager-hardened) state. Once link retraining is done controller drivers needs to adjust their votes based on the final data rate. Some controllers also may need to update their bandwidth voting like ICC bw votings etc. So, add pre_scale_bus_bw() & post_scale_bus_bw() op to call before & after the link re-train. In case of PCIe switch, if there is a request to change target speed for a downstream port then no need to call these function ops as these are outside the scope of the controller drivers. Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> --- drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++ include/linux/pci.h | 2 ++ 2 files changed, 17 insertions(+)