Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists

[Serge Semin <fancer.lancer@gmail.com>]

On Mon, Dec 12, 2022 at 10:41:02PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Dec 12, 2022 at 07:56:00PM +0300, Serge Semin wrote:
> > On Mon, Dec 12, 2022 at 06:26:58PM +0530, Manivannan Sadhasivam wrote:
> > > Hi Serge,
> > > 
> > > On Sun, Dec 11, 2022 at 06:28:49PM +0300, Serge Semin wrote:
> > > > Hi Frank
> > > > 
> > > > On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> > > > > Hi Serge,
> > > > > 
> > > > > > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> > > > > >
> > > > > > Cc += Frank Li
> > > > > >
> > > > > > @Frank could you have a look at the thread and check the content of
> > > > > > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > > > > > devices?
> > > > > 
> > > > 
> > > > > [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001
> > > > 
> > > > Thanks for the reply. So it's 4.80a with the legacy viewport-based
> > > > access. Alas it isn't what we need in this thread. We'll need
> > > > @Mani's respond in order to decide how to fix the auto-detection
> > > > procedure.
> > > > 
> > > 
> > 
> > > Sorry for the late reply!
> > > 
> > > With below diff on the EP:
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 6f3805228a18..0eb4d3218738 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -665,6 +665,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >         if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > >  
> > > +               dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > +                       dw_pcie_readl_dbi(pci, 0x8f8),
> > > +                       dw_pcie_readl_dbi(pci, 0x978));
> > > +
> > >                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > >         } else if (val != 0xFFFFFFFF) {
> > >                 pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > > 
> > > 
> > > The output was:
> > > 
> > > qcom-pcie-ep 1c08000.pcie-ep: dw_pcie_edma_find_chip: +0x8f8 = 3533302a, +0x978 = ffffffff
> > > 
> > > Hope this helps!
> > 
> > Great! Thanks. This indeed helps. So it's 5.30a IP-core. Just one
> > quick question. Does that device have eDMA embedded into the DW PCIe
> > controller?
> > 
> 

> Yes it is and it is the test platform I use for eDMA/PCI_EP work.

So the procedure works well for IP-core 5.30a and AFAICS it doesn't
for 5.40a (eDMA viewport-based CSRs are missing in the HW-manual) and
for an unexpected reason in IP-core 5.20a synthesized for Renesas
R-Car Gen4 PCIe. Thus this seems more like a vendor-specific problem,
than a version-specific one since the HW-manual in both 5.20a and
5.30a cases state that the dbi+0x978 register must have FFs if the CSR
doesn't exist. It doesn't exist if the next statement is false:
!CX_PL_REG_DISABLE && CC_DMA_ENABLE && !CC_UNROLL_ENABLE && CC_DEVICE_TYPE!=3
So seeing the R-Car Gen4 PCIe has the unrolled eDMA mapping the
dbi+0x978 registers must contain FFs.

The best solution in this case would be to have a special
capability flag which would force the unrolled eDMA mapping for the
problematic devices. Like this:
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -840,8 +840,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 	 * Indirect eDMA CSRs access has been completely removed since v5.40a
 	 * thus no space is now reserved for the eDMA channels viewport and
 	 * former DMA CTRL register is no longer fixed to FFs.
+	 *
+	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
+	 * register even though the HW-manual explicitly states there must FFs
+	 * if the unrolled mapping is enabled. For such cases the low-level
+	 * drivers are supposed to manually activate the unrolled mapping to
+	 * bypass the auto-detection procedure.
 	 */
-	if (dw_pcie_ver_is_ge(pci, 540A)) {
+	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL)) {
 		val = 0xFFFFFFFF;
 	else
 		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -52,7 +52,8 @@
 /* DWC PCIe controller capabilities */
 #define DW_PCIE_CAP_REQ_RES		0
 #define DW_PCIE_CAP_IATU_UNROLL		1
-#define DW_PCIE_CAP_CDM_CHECK		2
+#define DW_PCIE_CAP_EDMA_UNROLL		2
+#define DW_PCIE_CAP_CDM_CHECK		3
 
 #define dw_pcie_cap_is(_pci, _cap) \
 	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)

The patch above is based on the updated version of my patchset, which
I'll resubmit for review tomorrow. I'll add @Yoshihiro in Cc-list of
the series.

-Serge(y)

> 
> Thanks,
> Mani
> 
> > -Serge(y)
> > 
> > > 
> > > Thanks,
> > > Mani
> > > 
> > > > -Serge(y)
> > > > 
> > > > > 
> > > > > Frank Li
> > > > > 
> > > > > 
> > > 
> > > -- 
> > > மணிவண்ணன் சதாசிவம்
> 
> -- 
> மணிவண்ணன் சதாசிவம்