[PATCH v3 02/11] PCI/bwctrl: Add support to scale bandwidth before & after link re-training
Krishna Chaitanya Chundru
krishna.chundru at oss.qualcomm.com
Mon May 19 21:06:56 PDT 2025
On 5/19/2025 7:11 PM, Ilpo Järvinen wrote:
> On Mon, 19 May 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. There is no explicit locking mechanisms as these are
>> called by a single client endpoint driver.
>>
>> 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 at oss.qualcomm.com>
>> ---
>> drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++
>> include/linux/pci.h | 14 ++++++++++++++
>> 2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
>> index d8d2aa85a22928b99c5bba1d2bcc5647c0edeeb6..3525bc0cd10f1dd7794abbe84ccb10e2c53a10af 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_rootbus = 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 host bridge driver may need to be scaled for targeted speed
>> + * otherwise link might not come up at requested speed.
>> + */
>> + if (is_rootbus && host->pre_scale_bus_bw) {
>> + ret = host->pre_scale_bus_bw(host, port, 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 (bus && is_rootbus && host->post_scale_bus_bw)
>> + host->post_scale_bus_bw(host, port, pci_bus_speed2lnkctl2(bus->cur_bus_speed));
>> +
>> return ret;
>> }
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 51e2bd6405cda5acc33d268bbe1d491b145e083f..7eb0856ba0ed20bd1336683b68add124c7483902 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -601,6 +601,20 @@ struct pci_host_bridge {
>> void (*release_fn)(struct pci_host_bridge *);
>> int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
>> void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
>> + /*
>> + * Callback to the host bridge drivers to update ICC bw votes, clock frequencies etc
>
> BW
>
>> + * for the link re-train to come up in targeted speed. These are intended to be
>> + * called by devices directly attached to the root port. These are called by a single
>
> Root Port
>
>> + * client endpoint driver, so there is no need for explicit locking mechanisms.
>
> Endpoint
>
>> + */
>> + int (*pre_scale_bus_bw)(struct pci_host_bridge *bridge, struct pci_dev *dev, int speed);
>> + /*
>> + * Callback to the host bridge drivers to adjust ICC bw votes, clock frequencies etc
>> + * to the updated speed after link re-train. These are intended to be called by
>> + * devices directly attached to the root port. These are called by a single client
>> + * endpoint driver, so there is no need for explicit locking mechanisms.
>> + */
>
> Please fold comments to 80 characters.
>
>> + void (*post_scale_bus_bw)(struct pci_host_bridge *bridge, struct pci_dev *dev, int speed);
>
> I still don't like the names. Maybe simply pre/post_link_speed_change
> would sound more generic.
>
Ack, I will change the name in the next patch.
- Krishna Chaitanya.
> Not a show-stopper but the current name sounds pretty esoteric.
> >> void *release_data;
>> unsigned int ignore_reset_delay:1; /* For entire hierarchy */
>> unsigned int no_ext_tags:1; /* No Extended Tags */
>>
>>
>
More information about the ath11k
mailing list