| Message ID | 20190506123456.6777-1-peter.ujfalusi@ti.com |
|---|---|
| Headers | show |
| Series | dmaengine/soc/firmware: Add Texas Instruments UDMA support | expand |
On 06/05/2019 15.34, Peter Ujfalusi wrote: > dma_get_any_slave_channel() would skip using the filter function, which > in some cases needed to be executed before the alloc_chan_resources > callback to make sure that all parameters are provided for the slave > channel. This can be dropped in favor of https://patchwork.kernel.org/patch/10932299/ from Baolin Wangm and using __dma_request_channel() in the k3-udma driver. - Péter > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/dmaengine.c | 7 ++++--- > include/linux/dmaengine.h | 5 ++++- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > index 8eed5ff0fc01..7ec93be12088 100644 > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -617,7 +617,8 @@ struct dma_chan *dma_get_slave_channel(struct dma_chan *chan) > } > EXPORT_SYMBOL_GPL(dma_get_slave_channel); > > -struct dma_chan *dma_get_any_slave_channel(struct dma_device *device) > +struct dma_chan *dmadev_get_slave_channel(struct dma_device *device, > + dma_filter_fn fn, void *fn_param) > { > dma_cap_mask_t mask; > struct dma_chan *chan; > @@ -628,13 +629,13 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device) > /* lock against __dma_request_channel */ > mutex_lock(&dma_list_mutex); > > - chan = find_candidate(device, &mask, NULL, NULL); > + chan = find_candidate(device, &mask, fn, fn_param); > > mutex_unlock(&dma_list_mutex); > > return IS_ERR(chan) ? NULL : chan; > } > -EXPORT_SYMBOL_GPL(dma_get_any_slave_channel); > +EXPORT_SYMBOL_GPL(dmadev_get_slave_channel); > > /** > * __dma_request_channel - try to allocate an exclusive channel > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index c1486564a314..4774b66f2064 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -1541,7 +1541,10 @@ int dmaenginem_async_device_register(struct dma_device *device); > void dma_async_device_unregister(struct dma_device *device); > void dma_run_dependencies(struct dma_async_tx_descriptor *tx); > struct dma_chan *dma_get_slave_channel(struct dma_chan *chan); > -struct dma_chan *dma_get_any_slave_channel(struct dma_device *device); > +struct dma_chan *dmadev_get_slave_channel(struct dma_device *device, > + dma_filter_fn fn, void *fn_param); > +#define dma_get_any_slave_channel(device) \ > + dmadev_get_slave_channel(device, NULL, NULL) > #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y) > #define dma_request_slave_channel_compat(mask, x, y, dev, name) \ > __dma_request_slave_channel_compat(&(mask), x, y, dev, name) > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi Peter, On 06/05/19 6:04 PM, Peter Ujfalusi wrote: > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Patch has the following checkpatch warnings and checks which can be fixed: WARNING: Missing commit description - Add an appropriate one CHECK: Lines should not end with a '(' #262: FILE: drivers/firmware/ti_sci.c:2286: +static int ti_sci_cmd_rm_udmap_tx_ch_cfg( CHECK: Lines should not end with a '(' #323: FILE: drivers/firmware/ti_sci.c:2347: +static int ti_sci_cmd_rm_udmap_rx_ch_cfg( CHECK: Lines should not end with a '(' #383: FILE: drivers/firmware/ti_sci.c:2407: +static int ti_sci_cmd_rm_udmap_rx_flow_cfg1( CHECK: Lines should not end with a '(' #1414: FILE: include/linux/soc/ti/ti_sci_protocol.h:455: + int (*rx_flow_cfg)( total: 0 errors, 2 warnings, 4 checks, 1399 lines checked > --- > drivers/firmware/ti_sci.c | 439 +++++++++++++++ > drivers/firmware/ti_sci.h | 704 +++++++++++++++++++++++++ > include/linux/soc/ti/ti_sci_protocol.h | 216 ++++++++ > 3 files changed, 1359 insertions(+) > > diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c > index 64d895b80bc3..af3ebcdeab18 100644 > --- a/drivers/firmware/ti_sci.c > +++ b/drivers/firmware/ti_sci.c [..snip.] > +} > + > +static int ti_sci_cmd_rm_psil_pair(const struct ti_sci_handle *handle, > + u32 nav_id, u32 src_thread, u32 dst_thread) > +{ All the psil ops doesn't have the kernel-doc function comments. Just be consistent with other functions :) > + struct ti_sci_msg_hdr *resp; > + struct ti_sci_msg_psil_pair *req; > + struct ti_sci_xfer *xfer; > + struct ti_sci_info *info; > + struct device *dev; > + int ret = 0; > + > + if (IS_ERR(handle)) > + return PTR_ERR(handle); > + if (!handle) > + return -EINVAL; > + > + info = handle_to_ti_sci_info(handle); > + dev = info->dev; > + > + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_RM_PSIL_PAIR, > + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, > + sizeof(*req), sizeof(*resp)); > + if (IS_ERR(xfer)) { > + ret = PTR_ERR(xfer); > + dev_err(dev, "RM_PSIL:Message reconfig failed(%d)\n", ret); > + return ret; > + } > + req = (struct ti_sci_msg_psil_pair *)xfer->xfer_buf; > + req->nav_id = nav_id; > + req->src_thread = src_thread; > + req->dst_thread = dst_thread; > + > + ret = ti_sci_do_xfer(info, xfer); > + if (ret) { > + dev_err(dev, "RM_PSIL:Mbox send fail %d\n", ret); > + goto fail; > + } > + > + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; > + ret = ti_sci_is_response_ack(resp) ? 0 : -EINVAL; > + > +fail: > + ti_sci_put_one_xfer(&info->minfo, xfer); > + > + return ret; > +} > + [..snip..] > + */ > +struct ti_sci_msg_rm_ring_cfg_req { > + struct ti_sci_msg_hdr hdr; > + u32 valid_params; > + u16 nav_id; > + u16 index; > + u32 addr_lo; > + u32 addr_hi; > + u32 count; > + u8 mode; > + u8 size; > + u8 order_id; > +} __packed; > + > +/** > + * struct ti_sci_msg_rm_ring_cfg_resp - Response to configuring a ring. > + * > + * @hdr: Generic Header > + */ > +struct ti_sci_msg_rm_ring_cfg_resp { > + struct ti_sci_msg_hdr hdr; > +} __packed; If it is a generic ACK, NACK response, just use the header directly. [..snip..] > + */ > +struct ti_sci_msg_rm_udmap_rx_ch_cfg_req { > + struct ti_sci_msg_hdr hdr; > + u32 valid_params; > + u16 nav_id; > + u16 index; > + u16 rx_fetch_size; > + u16 rxcq_qnum; > + u8 rx_priority; > + u8 rx_qos; > + u8 rx_orderid; > + u8 rx_sched_priority; > + u16 flowid_start; > + u16 flowid_cnt; > + u8 rx_pause_on_err; > + u8 rx_atype; > + u8 rx_chan_type; > + u8 rx_ignore_short; > + u8 rx_ignore_long; > + u8 rx_burst_size; > + extra line? > +} __packed; > + > +/** Thanks and regards, Lokesh
Hi Lokesh, On 06/06/2019 9.00, Lokesh Vutla wrote: > Hi Peter, > > On 06/05/19 6:04 PM, Peter Ujfalusi wrote: >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > Patch has the following checkpatch warnings and checks which can be fixed: > > WARNING: Missing commit description - Add an appropriate one How did I missed it? > CHECK: Lines should not end with a '(' > #262: FILE: drivers/firmware/ti_sci.c:2286: > +static int ti_sci_cmd_rm_udmap_tx_ch_cfg( > > CHECK: Lines should not end with a '(' > #323: FILE: drivers/firmware/ti_sci.c:2347: > +static int ti_sci_cmd_rm_udmap_rx_ch_cfg( > > CHECK: Lines should not end with a '(' > #383: FILE: drivers/firmware/ti_sci.c:2407: > +static int ti_sci_cmd_rm_udmap_rx_flow_cfg1( > > CHECK: Lines should not end with a '(' > #1414: FILE: include/linux/soc/ti/ti_sci_protocol.h:455: > + int (*rx_flow_cfg)( > > total: 0 errors, 2 warnings, 4 checks, 1399 lines checked There must be a reason why these left, but I will take another look. >> --- >> drivers/firmware/ti_sci.c | 439 +++++++++++++++ >> drivers/firmware/ti_sci.h | 704 +++++++++++++++++++++++++ >> include/linux/soc/ti/ti_sci_protocol.h | 216 ++++++++ >> 3 files changed, 1359 insertions(+) >> >> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c >> index 64d895b80bc3..af3ebcdeab18 100644 >> --- a/drivers/firmware/ti_sci.c >> +++ b/drivers/firmware/ti_sci.c > > [..snip.] > >> +} >> + >> +static int ti_sci_cmd_rm_psil_pair(const struct ti_sci_handle *handle, >> + u32 nav_id, u32 src_thread, u32 dst_thread) >> +{ > > All the psil ops doesn't have the kernel-doc function comments. Just be > consistent with other functions :) OK. >> + struct ti_sci_msg_hdr *resp; >> + struct ti_sci_msg_psil_pair *req; >> + struct ti_sci_xfer *xfer; >> + struct ti_sci_info *info; >> + struct device *dev; >> + int ret = 0; >> + >> + if (IS_ERR(handle)) >> + return PTR_ERR(handle); >> + if (!handle) >> + return -EINVAL; >> + >> + info = handle_to_ti_sci_info(handle); >> + dev = info->dev; >> + >> + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_RM_PSIL_PAIR, >> + TI_SCI_FLAG_REQ_ACK_ON_PROCESSED, >> + sizeof(*req), sizeof(*resp)); >> + if (IS_ERR(xfer)) { >> + ret = PTR_ERR(xfer); >> + dev_err(dev, "RM_PSIL:Message reconfig failed(%d)\n", ret); >> + return ret; >> + } >> + req = (struct ti_sci_msg_psil_pair *)xfer->xfer_buf; >> + req->nav_id = nav_id; >> + req->src_thread = src_thread; >> + req->dst_thread = dst_thread; >> + >> + ret = ti_sci_do_xfer(info, xfer); >> + if (ret) { >> + dev_err(dev, "RM_PSIL:Mbox send fail %d\n", ret); >> + goto fail; >> + } >> + >> + resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf; >> + ret = ti_sci_is_response_ack(resp) ? 0 : -EINVAL; >> + >> +fail: >> + ti_sci_put_one_xfer(&info->minfo, xfer); >> + >> + return ret; >> +} >> + > > [..snip..] > >> + */ >> +struct ti_sci_msg_rm_ring_cfg_req { >> + struct ti_sci_msg_hdr hdr; >> + u32 valid_params; >> + u16 nav_id; >> + u16 index; >> + u32 addr_lo; >> + u32 addr_hi; >> + u32 count; >> + u8 mode; >> + u8 size; >> + u8 order_id; >> +} __packed; >> + >> +/** >> + * struct ti_sci_msg_rm_ring_cfg_resp - Response to configuring a ring. >> + * >> + * @hdr: Generic Header >> + */ >> +struct ti_sci_msg_rm_ring_cfg_resp { >> + struct ti_sci_msg_hdr hdr; >> +} __packed; > > If it is a generic ACK, NACK response, just use the header directly. Sure, I'll fix it and other places if any. > > [..snip..] > >> + */ >> +struct ti_sci_msg_rm_udmap_rx_ch_cfg_req { >> + struct ti_sci_msg_hdr hdr; >> + u32 valid_params; >> + u16 nav_id; >> + u16 index; >> + u16 rx_fetch_size; >> + u16 rxcq_qnum; >> + u8 rx_priority; >> + u8 rx_qos; >> + u8 rx_orderid; >> + u8 rx_sched_priority; >> + u16 flowid_start; >> + u16 flowid_cnt; >> + u8 rx_pause_on_err; >> + u8 rx_atype; >> + u8 rx_chan_type; >> + u8 rx_ignore_short; >> + u8 rx_ignore_long; >> + u8 rx_burst_size; >> + > > extra line? Will remove it. > >> +} __packed; >> + >> +/** > > > Thanks and regards, > Lokesh > Thanks, - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Changes since RFC (https://patchwork.kernel.org/cover/10612465/): - Based on linux-next (20190506) which now have the ti_sci interrupt support - The series can be applied and the UDMA via DMAengine API will be functional - Included in the series: ti_sci Resource management API, cppi5 header and driver for the ring accelerator. - The DMAengine core patches have been updated as per the review comments for earlier submittion. - The DMAengine driver patch is artificially split up to 6 smaller patches The AM65x TRM (http://www.ti.com/lit/pdf/spruid7) describes the Data Movement Architecture which is implmented by the k3-udma driver. This DMA architecture is a big departure from 'traditional' architecture where we had either EDMA or sDMA as system DMA. Packet DMAs were used as dedicated DMAs to service only networking (Kesytone2) or USB (am335x) while other peripherals were serviced by EDMA. In AM65x the UDMA (Unified DMA) is used for all data movment within the SoC, tasked to service all peripherals (UART, McSPI, McASP, networking, etc). The NAVSS/UDMA is built around CPPI5 (Communications Port Programming Interface) and it supports Packet mode (similar to CPPI4.1 in Keystone2 for networking) and TR mode (similar to EDMA descriptor). The data movement is done within a PSI-L fabric, all peripherals (including the UDMA-P) are not addressed by their I/O register as with traditional DMAs but with their PSI-L thread ID. In AM65x we have two main type of peripherals: Legacy: McASP, McSPI, UART, etc. to provide connectivity they are serviced by PDMA (Peripheral DMA) PDMA threads are locked to service a given peripheral, for example PSI-L thread 0x4400/0xc400 is to service McASP0 rx/tx. The PDMa configuration can be done via the UDMA Real Time Peer registers. Native: Networking, security accelerator these peripherals have native support for PSI-L. To be able to use the DMA the following generic steps need to be taken: - configure a DMA channel (tchan for TX, rchan for RX) - channel mode: Packet or TR mode - for memcpy a tchan and rchan pair is used. - for packet mode RX we also need to configure a receive flow to configure the packet receiption - the source and destination threads must be paired - at minimum one pair of rings need to be configured: - tx: transfer ring and transfer completion ring - rx: free descriptor ring and receive ring - two interrupts: UDMA-P channel interrupt and ring interrupt for tc_ring/r_ring - If the channel is in packet mode or configured to memcpy then we only need one interrupt from the ring, events from UDMAP is not used. When the channel setup is completed we only interract with the rings: - TX: push a descriptor to t_ring and wait for it to be pushed to the tc_ring by the UDMA-P - RX: push a descriptor to the fd_ring and waith for UDMA-P to push it back to the r_ring. Since we have FIFOs in the DMA fabric (UDMA-P, PSI-L and PDMA) which was not the case in previous DMAs we need to report the amount of data held in these FIFOs to clients (delay calculation for ALSA, UART FIFO flush support). - dmadev_get_slave_channel() I needed a way to request a channel from a specific dma_device which would invoke the filter function to get the needed parameters prior needed for the alloc_chan_resources. Note on the last patch: In Keystone2 the networking had dedicated DMA (packet DMA) which is not the case anymore and the DMAengine API currently missing support for the features we would need to support networking, things like - support for receive descriptor 'classification' - we need to support several receive queues for a channel. - the queues are used for packet priority handling for example, but they can be used to have pools of descriptors for different sizes. - out of order completion of descriptors on a channel - when we have several queues to handle different priority packets the descriptors will be completed 'out-of-order' - NAPI type of operation (polling instead of interrupt driven transfer) - without this we can not sustain gigabit speeds and we need to support NAPI - not to limit this to networking, but other high performance operations It is my intention to work on these to be able to remove the 'glue' layer and switch to DMAengine API - or have an API aside of DMAengine to have generic way to support networking, but given how controversial and not trivial these changes are we need something to support networking. Regards, Peter --- Grygorii Strashko (3): bindings: soc: ti: add documentation for k3 ringacc soc: ti: k3: add navss ringacc driver dmaengine: ti: k3-udma: Add glue layer for non DMAengine users Peter Ujfalusi (13): firmware: ti_sci: Add resource management APIs for ringacc, psi-l and udma dmaengine: doc: Add sections for per descriptor metadata support dmaengine: Add metadata_ops for dma_async_tx_descriptor dmaengine: Add support for reporting DMA cached data amount dmaengine: Add function to request slave channel from a dma_device dmaengine: ti: Add cppi5 header for UDMA dt-bindings: dma: ti: Add document for K3 UDMA dmaengine: ti: New driver for K3 UDMA - split#1: defines, structs, io func dmaengine: ti: New driver for K3 UDMA - split#2: probe/remove, xlate and filter_fn dmaengine: ti: New driver for K3 UDMA - split#3: alloc/free chan_resources dmaengine: ti: New driver for K3 UDMA - split#4: dma_device callbacks 1 dmaengine: ti: New driver for K3 UDMA - split#5: dma_device callbacks 2 dmaengine: ti: New driver for K3 UDMA - split#6: Kconfig and Makefile .../devicetree/bindings/dma/ti/k3-udma.txt | 134 + .../devicetree/bindings/soc/ti/k3-ringacc.txt | 59 + Documentation/driver-api/dmaengine/client.rst | 75 + .../driver-api/dmaengine/provider.rst | 46 + drivers/dma/dmaengine.c | 80 +- drivers/dma/dmaengine.h | 8 + drivers/dma/ti/Kconfig | 22 + drivers/dma/ti/Makefile | 2 + drivers/dma/ti/k3-udma-glue.c | 1039 +++++ drivers/dma/ti/k3-udma-private.c | 124 + drivers/dma/ti/k3-udma.c | 3329 +++++++++++++++++ drivers/dma/ti/k3-udma.h | 159 + drivers/firmware/ti_sci.c | 439 +++ drivers/firmware/ti_sci.h | 704 ++++ drivers/soc/ti/Kconfig | 17 + drivers/soc/ti/Makefile | 1 + drivers/soc/ti/k3-ringacc.c | 1190 ++++++ include/dt-bindings/dma/k3-udma.h | 26 + include/linux/dma/k3-udma-glue.h | 123 + include/linux/dma/ti-cppi5.h | 994 +++++ include/linux/dmaengine.h | 115 +- include/linux/soc/ti/k3-ringacc.h | 258 ++ include/linux/soc/ti/ti_sci_protocol.h | 216 ++ 23 files changed, 9156 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-udma.txt create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt create mode 100644 drivers/dma/ti/k3-udma-glue.c create mode 100644 drivers/dma/ti/k3-udma-private.c create mode 100644 drivers/dma/ti/k3-udma.c create mode 100644 drivers/dma/ti/k3-udma.h create mode 100644 drivers/soc/ti/k3-ringacc.c create mode 100644 include/dt-bindings/dma/k3-udma.h create mode 100644 include/linux/dma/k3-udma-glue.h create mode 100644 include/linux/dma/ti-cppi5.h create mode 100644 include/linux/soc/ti/k3-ringacc.h