Re: [PATCH v4 02/11] PCI/bwctrl: Add support to scale bandwidth before & after link re-training

[Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxx>
> > ---
> >   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