[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a5b8bbcf-f4ea-45bb-8a81-efc834eba233@oss.qualcomm.com>
Date: Wed, 9 Jul 2025 17:38:32 +0530
From: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Jingoo Han <jingoohan1@...il.com>,
Lorenzo Pieralisi
<lpieralisi@...nel.org>,
Rob Herring <robh@...nel.org>, Jeff Johnson <jjohnson@...nel.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, mhi@...ts.linux.dev,
linux-wireless@...r.kernel.org, ath11k@...ts.infradead.org,
qiang.yu@....qualcomm.com, quic_vbadigan@...cinc.com,
quic_vpernami@...cinc.com, quic_mrana@...cinc.com,
Jeff Johnson <jeff.johnson@....qualcomm.com>
Subject: Re: [PATCH v4 02/11] PCI/bwctrl: Add support to scale bandwidth
before & after link re-training
On 7/8/2025 9:55 PM, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 04:21:23PM GMT, Krishna Chaitanya Chundru wrote:
>> If the driver wants to move to higher data rate/speed than the current data
>
> s/driver/PCI client driver
>
>> 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_link_speed_change() & post_link_speed_change() 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.
>>
>
> What if client drivers of multiple endpoints connected to different RP of the
> same Host Bridge call this API? Won't you need locking?
>
Yeah you are right, I will add locking in next patch.
- Krishna Chaitanya.
> - Mani
>
>> 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@....qualcomm.com>
>> ---
>> drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++
>> include/linux/pci.h | 18 ++++++++++++++++++
>> 2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
>> index 36f939f23d34e8a3b25a2d1b9059e015f298ca94..dafb8d4f1cfba987e1ff08edfc7caba527f0c76b 100644
>> --- a/drivers/pci/pcie/bwctrl.c
>> +++ b/drivers/pci/pcie/bwctrl.c
>> @@ -140,6 +140,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;
>> @@ -152,6 +154,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_link_speed_change) {
>> + ret = host->pre_link_speed_change(host, port, target_speed);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> scoped_guard(rwsem_read, &pcie_bwctrl_setspeed_rwsem) {
>> struct pcie_bwctrl_data *data = port->link_bwctrl;
>>
>> @@ -176,6 +188,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_link_speed_change)
>> + host->post_link_speed_change(host, port, pci_bus_speed2lnkctl2(bus->cur_bus_speed));
>> +
>> return ret;
>> }
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 05e68f35f39238f8b9ce08df97b384d1c1e89bbe..1740bab514b0a9a61c027463a1fb154843312a22 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -599,6 +599,24 @@ 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.. 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 client Endpoint driver, so
>> + * there is no need for explicit locking mechanisms.
>> + */
>> + int (*pre_link_speed_change)(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.
>> + */
>> + void (*post_link_speed_change)(struct pci_host_bridge *bridge,
>> + struct pci_dev *dev, int speed);
>> void *release_data;
>> unsigned int ignore_reset_delay:1; /* For entire hierarchy */
>> unsigned int no_ext_tags:1; /* No Extended Tags */
>>
>> --
>> 2.34.1
>>
>
Powered by blists - more mailing lists