Re: [PATCH v7 06/10] PCI: dwc: Implement endpoint DMA channel delegation
Koichiro Den <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.dmaengine,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <w223tumnzi3zircm4tnv4b7cug2g23z52xris7fcrhxyr6knvr@cw6meanwd5e3> |
On Tue, Aug 25, 2026 at 11:36:54AM +0900, Koichiro Den wrote: > On Thu, Aug 13, 2026 at 02:09:26PM -0500, Frank Li wrote: > > On Thu, Aug 13, 2026 at 03:37:53PM +0900, Koichiro Den wrote: > > > Implement the EPC DMA channel delegation operations for DesignWare > > > endpoint controllers. Verify that the DMA engine channel reserved by the > > > endpoint function belongs to the integrated DesignWare eDMA instance, > > > then hand its interrupt routing to the host. > > > > > > The DWC eDMA/HDMA engine generates DMA requests with a programmable > > > requester function number. For delegated channels, the host-side > > > dw-edma-pcie instance bound to the exposed DMA function programs its own > > > PCI_FUNC() into the per-channel requester field; the endpoint-side chip > > > func_no does > > > not participate in that handoff. > > > > > > Reject VF requests because the RC-programmable DWC eDMA/HDMA register > > > window is assigned to a PF BAR only. > > > > > > Reclaim returns interrupt routing to endpoint ownership. Propagate the > > > EPC quiesce request so bind failure paths can reclaim unexposed channels > > > without quiescing the DMA engine. > > > > > > Signed-off-by: Koichiro Den <[email protected]> > > > --- > > > Changes in v7: > > > - Delegate the DMA engine channel already reserved by the endpoint > > > function, matching the API and helper changes in patches 4 and 5. > > > > > > .../pci/controller/dwc/pcie-designware-ep.c | 42 +++++++++++++++++++ > > > 1 file changed, 42 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > index 0b915824963a..a74d3896e436 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > @@ -858,6 +858,18 @@ dw_pcie_ep_find_bar_rsvd_region(struct dw_pcie_ep *ep, > > > return NULL; > > > } > > > > > > +static int dw_pcie_ep_check_edma_vfunc(u8 vfunc_no) > > > +{ > > > + /* > > > + * The DWC endpoint databook says it is not possible to assign the > > > + * DMA/HDMA registers to any Virtual Function. > > > + */ > > > + if (vfunc_no) > > > + return -EOPNOTSUPP; > > > + > > > + return 0; > > > +} > > > + > > > static int > > > dw_pcie_ep_get_aux_resources_count(struct pci_epc *epc, u8 func_no, > > > u8 vfunc_no) > > > @@ -933,6 +945,34 @@ dw_pcie_ep_get_aux_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > > > return 0; > > > } > > > > > > +static int dw_pcie_ep_delegate_dma_chan(struct pci_epc *epc, u8 func_no, > > > + u8 vfunc_no, struct dma_chan *chan) > > > +{ > > > + struct dw_pcie_ep *ep = epc_get_drvdata(epc); > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > > + struct dw_edma_chip *edma = &pci->edma; > > > + int ret; > > > + > > > + ret = dw_pcie_ep_check_edma_vfunc(vfunc_no); > > > + if (ret) > > > + return ret; > > > + > > > + if (!edma->dw) > > > + return -ENODEV; > > > + > > > + if (!chan || chan->device->dev != edma->dev) > > > + return -EINVAL; > > > + > > > + return dw_edma_delegate_chan(chan); > > > +} > > > + > > > +static void dw_pcie_ep_reclaim_dma_chan(struct pci_epc *epc, u8 func_no, > > > + u8 vfunc_no, struct dma_chan *chan, > > > + bool quiesce) > > > +{ > > > + dw_edma_reclaim_chan(chan, quiesce); > > > +} > > > + > > > > After use fixed chan_ids, needn't these functins. EPF driver already > > find expected dma channel and config it remote only by standard API. > > (I've been somewhat holding this thread, sorry. I'm still unsure whether the > separate PCI DMA EPF idea will survive, or whether making it much smaller and > initially limiting it to a test endpoint for dw-edma-pcie would make it > acceptable. AFAICT, Mani has not answered that point yet. Still, I think I > should reply here in case the idea survives in some form and v8 becomes much > simpler.) > > I agree fixed chan_id removes the private channel lookup. In v7, the EPF already > reserves the channels to delegate through dma_request_channel(), so Patch 4 may > look like a leftover. Its remaining benefit is to hide dw-edma-specific > interrupt-routing semantics from the generic EPF. Let me correct myself. On second thought, that "benefit" is really superficial. If we ever want a generic abstraction for that purpose, it should belong in dmaengine, not PCI EPC. Now that the EPF reserves channels through standard dma_request_channel(), it owns them until dma_release_channel(). Going thought PCI EPC just to change their interrupt routing (in the dw-edma case) is odd. > > The EPF could instead do something like this in pci_epf_dma_claim_channel() > after extending dw-edma to accept the setting: > > config.peripheral_config = ... DW_EDMA_CH_IRQ_REMOTE ...; > dmaengine_slave_config(chan, &config); > > That would make the EPF depend on dw-edma-specific interrupt routing. However, > if the goal is to make the PCI DMA EPF as small as possible and accept that it > initially only serves the dw-edma-pcie path, I think that is fine. I think > that's your point, but if not, please correct me. So even in the vNTB-embedded design, your suggestion to use prepheral_config fits well. I've now implemented and tested it there, and it works. I'm preparing v2 on that basis and hope to send it soon. Best regards, Koichiro > > Thanks for reviewing. > Koichiro > > > > > Frank > > > > > static const struct pci_epc_ops epc_ops = { > > > .write_header = dw_pcie_ep_write_header, > > > .set_bar = dw_pcie_ep_set_bar, > > > @@ -950,6 +990,8 @@ static const struct pci_epc_ops epc_ops = { > > > .get_features = dw_pcie_ep_get_features, > > > .get_aux_resources_count = dw_pcie_ep_get_aux_resources_count, > > > .get_aux_resources = dw_pcie_ep_get_aux_resources, > > > + .delegate_dma_chan = dw_pcie_ep_delegate_dma_chan, > > > + .reclaim_dma_chan = dw_pcie_ep_reclaim_dma_chan, > > > }; > > > > > > /** > > > -- > > > 2.51.0 > > > >