[PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration

Krishna Chaitanya Chundru posted 5 patches 5 months, 2 weeks ago
There is a newer version of this series
arch/arm64/boot/dts/qcom/sc7280.dtsi              |  14 +--
drivers/pci/controller/dwc/Kconfig                |   1 +
drivers/pci/controller/dwc/pcie-designware-host.c | 145 +++++++++++++++++++---
drivers/pci/controller/dwc/pcie-designware.c      |  11 +-
drivers/pci/controller/dwc/pcie-designware.h      |   6 +
drivers/pci/controller/dwc/pcie-qcom.c            |  86 +++++++++++--
6 files changed, 232 insertions(+), 31 deletions(-)
[PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Krishna Chaitanya Chundru 5 months, 2 weeks ago
The current implementation requires iATU for every configuration
space access which increases latency & cpu utilization.

Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
would be matched against the Base and Limit addresses) of the incoming
CfgRd0/CfgWr0 down to bits[27:12]of the translated address.

Configuring iATU in config shift mode enables ECAM feature to access the
config space, which avoids iATU configuration for every config access.

Add cfg_shft_mode into struct dw_pcie_ob_atu_cfg to enable config shift mode.

As DBI comes under config space, this avoids remapping of DBI space
separately. Instead, it uses the mapped config space address returned from
ECAM initialization. Change the order of dw_pcie_get_resources() execution
to acheive this.

Enable the ECAM feature if the config space size is equal to size required
to represent number of buses in the bus range property.

ELBI registers are optional registers which are part of dwc. So move
ELBI resource mapping to dwc. Also change the dtbinding and devicetree
to make the elbi registers as optional one. Having ELBI as the required
one is making the ecam feature complicated.

The ELBI registers falls after the DBI space, PARF_SLV_DBI_ELBI register
gives us the offset from which ELBI starts. so use this offset and cfg
win to map these regions instead of doing the ioremap again.

On root bus, we have only the root port. Any access other than that
should not go out of the link and should return all F's. Since the iATU
is configured for the buses which starts after root bus, block the
transactions starting from function 1 of the root bus to the end of
the root bus (i.e from dbi_base + 4kb to dbi_base + 1MB) from going
outside the link through ECAM blocker through PARF registers.

Increase the configuration size to 256MB as required by the ECAM feature
and also move config space, DBI, iATU to upper space and use lower space
entirely for BAR region.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v8:
- Added 256MB alignment check for config space address (Mayank).
- Link to v7: https://lore.kernel.org/r/20250822-ecam_v4-v7-0-098fb4ca77c1@oss.qualcomm.com

Changes in v7:
- Rebased with the latest kernel.
- change ecam_mode to ecam_enabled (Konrad)
- change dw_pcie_ecam_supported to dw_pcie_ecam_enabled
- use FIELD_GET & GENMASK for reading elbi offset (Konrad)
- Link to v6: https://lore.kernel.org/r/20250712-ecam_v4-v6-0-d820f912e354@qti.qualcomm.com

Changes in v6:
- Remove the dtbinding and dt changes which make elbi optional
- Use non overlap region in the devicetree and in the driver ELBI
  registers will be overridden using offset of elbi from dbi start using
  parf registers (mani).
- Link to v5: https://lore.kernel.org/r/20250309-ecam_v4-v5-0-8eff4b59790d@oss.qualcomm.com

Changes in v5:
- Make elbi as optional and move resource mapping to the dwc (Mani)
- Make the changes in the code as we made elbi as optional.
- Link to v4: https://lore.kernel.org/r/20250207-ecam_v4-v4-0-94b5d5ec5017@oss.qualcomm.com

Changes in v4:
- Update the commit messgaes and do minor code changes like adding
  export for the api, adding error message( mani)
- Link to v3: https://lore.kernel.org/all/20250121-enable_ecam-v3-0-cd84d3b2a7ba@oss.qualcomm.com/
Changes in v3:
- if bus range is less than 2 return with out configuring iATU for next
  bus & update the logic of ecam_supported() as suggested ( Konrad)
- updated commit text and update S-o-b (Bjorn Andresson)
- Link to v2: https://lore.kernel.org/r/20241224-enable_ecam-v2-0-43daef68a901@oss.qualcomm.com

changes in v2:
- rename enable_ecam to ecam_mode as suggested by mani.
- refactor changes as suggested by bjorn
- remove ecam_init() function op as we have removed ELBI virtual address
update from the ecam_init and moved to host init as we need the clocks
to be enabled to read the ELBI offset from the PARF registers.
- Update comments and commit message as suggested by bjorn.
- Allocate host bridge in the DWC glue drivers as suggested by bjorn
- move qcom_pcie_ecam_supported to dwc as suggested by mani.
Link to v1: https://lore.kernel.org/r/linux-devicetree/20241117-ecam-v1-1-6059faf38d07@quicinc.com/T/

---
Krishna Chaitanya Chundru (5):
      arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
      PCI: dwc: Add support for ELBI resource mapping
      PCI: dwc: qcom: Switch to dwc ELBI resource mapping
      PCI: dwc: Add ECAM support with iATU configuration
      PCI: qcom: Add support for ECAM feature

 arch/arm64/boot/dts/qcom/sc7280.dtsi              |  14 +--
 drivers/pci/controller/dwc/Kconfig                |   1 +
 drivers/pci/controller/dwc/pcie-designware-host.c | 145 +++++++++++++++++++---
 drivers/pci/controller/dwc/pcie-designware.c      |  11 +-
 drivers/pci/controller/dwc/pcie-designware.h      |   6 +
 drivers/pci/controller/dwc/pcie-qcom.c            |  86 +++++++++++--
 6 files changed, 232 insertions(+), 31 deletions(-)
---
base-commit: 07d9df80082b8d1f37e05658371b087cb6738770
change-id: 20250207-ecam_v4-f4eb9b893eeb

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Manivannan Sadhasivam 5 months, 1 week ago
On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
> The current implementation requires iATU for every configuration
> space access which increases latency & cpu utilization.
> 
> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> would be matched against the Base and Limit addresses) of the incoming
> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
> 
> [...]

Applied, thanks!

[2/5] PCI: dwc: Add support for ELBI resource mapping
      commit: d39e0103e38f9889271a77a837b6179b42d6730d
[3/5] PCI: dwc: qcom: Switch to dwc ELBI resource mapping
      commit: 6955a13b1349376a00ebca80d1a8e2960de3fb0f
[4/5] PCI: dwc: Add ECAM support with iATU configuration
      commit: 0c959f675ce0338f3bee2636437bce4d5713ea03
[5/5] PCI: qcom: Add support for ECAM feature
      commit: 0bba488d552dccda4b803fa4b5d4d5d3029d601d

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Bjorn Andersson 3 months, 1 week ago
On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
> The current implementation requires iATU for every configuration
> space access which increases latency & cpu utilization.
> 
> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> would be matched against the Base and Limit addresses) of the incoming
> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
> 
> [...]

Applied, thanks!

[1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
      commit: 03e928442d469f7d8dafc549638730647202d9ce

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Krishna Chaitanya Chundru 3 months, 1 week ago
On 10/28/2025 4:07 AM, Bjorn Andersson wrote:
> On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
>> The current implementation requires iATU for every configuration
>> space access which increases latency & cpu utilization.
>>
>> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
>> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
>> would be matched against the Base and Limit addresses) of the incoming
>> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>>
>> [...]
> Applied, thanks!
>
> [1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
>        commit: 03e928442d469f7d8dafc549638730647202d9ce

Hi Bjorn,

Can you revert this change, this is regression due to this series due to 
that we have change the logic,
we need to update the dtsi accordingly, I will send a separate for all 
controllers to enable this ECAM feature.

- Krishna Chaitanya.


> Best regards,
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Bjorn Andersson 2 months, 4 weeks ago
On Tue, Oct 28, 2025 at 11:12:23PM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 10/28/2025 4:07 AM, Bjorn Andersson wrote:
> > On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
> > > The current implementation requires iATU for every configuration
> > > space access which increases latency & cpu utilization.
> > > 
> > > Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> > > which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> > > would be matched against the Base and Limit addresses) of the incoming
> > > CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
> > > 
> > > [...]
> > Applied, thanks!
> > 
> > [1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
> >        commit: 03e928442d469f7d8dafc549638730647202d9ce
> 
> Hi Bjorn,
> 
> Can you revert this change, this is regression due to this series due to
> that we have change the logic,

How is that possible? This is patch 1 in the series, by definition it
doesn't have any outstanding dependencies.


I've reverted the change.

Regards,
Bjorn

> we need to update the dtsi accordingly, I will send a separate for all
> controllers to enable this ECAM feature.
> 
> - Krishna Chaitanya.
> 
> 
> > Best regards,
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Krishna Chaitanya Chundru 2 months, 3 weeks ago
On 11/10/2025 11:51 PM, Bjorn Andersson wrote:
> On Tue, Oct 28, 2025 at 11:12:23PM +0530, Krishna Chaitanya Chundru wrote:
>> On 10/28/2025 4:07 AM, Bjorn Andersson wrote:
>>> On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
>>>> The current implementation requires iATU for every configuration
>>>> space access which increases latency & cpu utilization.
>>>>
>>>> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
>>>> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
>>>> would be matched against the Base and Limit addresses) of the incoming
>>>> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>>>>
>>>> [...]
>>> Applied, thanks!
>>>
>>> [1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
>>>         commit: 03e928442d469f7d8dafc549638730647202d9ce
>> Hi Bjorn,
>>
>> Can you revert this change, this is regression due to this series due to
>> that we have change the logic,
> How is that possible? This is patch 1 in the series, by definition it
> doesn't have any outstanding dependencies.
The regression is due to the drivers changes on non qcom platforms.

- Krishna Chaitanya.
> I've reverted the change.
>
> Regards,
> Bjorn
>
>> we need to update the dtsi accordingly, I will send a separate for all
>> controllers to enable this ECAM feature.
>>
>> - Krishna Chaitanya.
>>
>>
>>> Best regards,
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Bjorn Andersson 2 months, 3 weeks ago
On Thu, Nov 13, 2025 at 09:27:36AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 11/10/2025 11:51 PM, Bjorn Andersson wrote:
> > On Tue, Oct 28, 2025 at 11:12:23PM +0530, Krishna Chaitanya Chundru wrote:
> > > On 10/28/2025 4:07 AM, Bjorn Andersson wrote:
> > > > On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
> > > > > The current implementation requires iATU for every configuration
> > > > > space access which increases latency & cpu utilization.
> > > > > 
> > > > > Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
> > > > > which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
> > > > > would be matched against the Base and Limit addresses) of the incoming
> > > > > CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
> > > > > 
> > > > > [...]
> > > > Applied, thanks!
> > > > 
> > > > [1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
> > > >         commit: 03e928442d469f7d8dafc549638730647202d9ce
> > > Hi Bjorn,
> > > 
> > > Can you revert this change, this is regression due to this series due to
> > > that we have change the logic,
> > How is that possible? This is patch 1 in the series, by definition it
> > doesn't have any outstanding dependencies.
> The regression is due to the drivers changes on non qcom platforms.
> 

Please be specific when you're answering, this way of saying "go fish"
isn't helpful.

By investing a little bit more time and writing a single sentence to
share what you know, you could have enlightened me and other readers of
this email list.

Regards,
Bjorn

> - Krishna Chaitanya.
> > I've reverted the change.
> > 
> > Regards,
> > Bjorn
> > 
> > > we need to update the dtsi accordingly, I will send a separate for all
> > > controllers to enable this ECAM feature.
> > > 
> > > - Krishna Chaitanya.
> > > 
> > > 
> > > > Best regards,
Re: (subset) [PATCH v8 0/5] PCI: dwc: Add ECAM support with iATU configuration
Posted by Krishna Chaitanya Chundru 2 months, 3 weeks ago

On 11/13/2025 11:03 PM, Bjorn Andersson wrote:
> On Thu, Nov 13, 2025 at 09:27:36AM +0530, Krishna Chaitanya Chundru wrote:
>> On 11/10/2025 11:51 PM, Bjorn Andersson wrote:
>>> On Tue, Oct 28, 2025 at 11:12:23PM +0530, Krishna Chaitanya Chundru wrote:
>>>> On 10/28/2025 4:07 AM, Bjorn Andersson wrote:
>>>>> On Thu, 28 Aug 2025 13:04:21 +0530, Krishna Chaitanya Chundru wrote:
>>>>>> The current implementation requires iATU for every configuration
>>>>>> space access which increases latency & cpu utilization.
>>>>>>
>>>>>> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
>>>>>> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
>>>>>> would be matched against the Base and Limit addresses) of the incoming
>>>>>> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>>>>>>
>>>>>> [...]
>>>>> Applied, thanks!
>>>>>
>>>>> [1/5] arm64: dts: qcom: sc7280: Increase config size to 256MB for ECAM feature
>>>>>          commit: 03e928442d469f7d8dafc549638730647202d9ce
>>>> Hi Bjorn,
>>>>
>>>> Can you revert this change, this is regression due to this series due to
>>>> that we have change the logic,
>>> How is that possible? This is patch 1 in the series, by definition it
>>> doesn't have any outstanding dependencies.
>> The regression is due to the drivers changes on non qcom platforms.
>>
> Please be specific when you're answering, this way of saying "go fish"
> isn't helpful.
>
> By investing a little bit more time and writing a single sentence to
> share what you know, you could have enlightened me and other readers of
> this email list.
Sorry, for not being clear in the previous reply.
The driver is implemented in a assumption that config space start 
address will
be the dbi address. The driver is using config space start address as 
dbi address
and ignoring the dbi address provided in the device tree.  This 
assumption is
breaking other vendor driver i.e SiFive driver. To fix this regression 
we have removed
this assumption keeping dbi and config space as separate entity.

Due to these driver changes, we have to move DBI to different location 
(controller driver
has ability to move the dbi space) for this reason we need to revert this.

- Krishna Chaitanya.
> Regards,
> Bjorn
>
>> - Krishna Chaitanya.
>>> I've reverted the change.
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> we need to update the dtsi accordingly, I will send a separate for all
>>>> controllers to enable this ECAM feature.
>>>>
>>>> - Krishna Chaitanya.
>>>>
>>>>
>>>>> Best regards,