Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Radhey Shyam Pandey <radheys@xilinx.com>
To:     Andrea Merello <andrea.merello@gmail.com>,
        "vkoul@kernel.org" <vkoul@kernel.org>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        Michal Simek <michals@xilinx.com>,
        Appana Durga Kedareswara Rao <appanad@xilinx.com>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>
CC:     "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/6] dmaengine: xilinx_dma: fix completion callback is not
 invoked for each DMA operation
Thread-Topic: [PATCH 2/6] dmaengine: xilinx_dma: fix completion callback is
 not invoked for each DMA operation
Thread-Index: AQHUCHIQrPud7LE+Xk2xdT54xDVLUqRpD8yQ
Date:   Wed, 20 Jun 2018 12:36:07 +0000
Message-ID: <DM6PR02MB43615ED0221189B6B8D9017EC7770@DM6PR02MB4361.namprd02.prod.outlook.com>
References: <20180620083653.17010-1-andrea.merello@gmail.com>
 <20180620083653.17010-2-andrea.merello@gmail.com>
In-Reply-To: <20180620083653.17010-2-andrea.merello@gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: xilinx.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b6bf4bc-3d57-43fe-70cc-08d5d6aa659b
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jun 2018 12:36:07.9482
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR02MB4361
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: dmaengine-owner@vger.kernel.org [mailto:dmaengine-
> owner@vger.kernel.org] On Behalf Of Andrea Merello
> Sent: Wednesday, June 20, 2018 2:07 PM
> To: vkoul@kernel.org; dan.j.williams@intel.com; Michal Simek
> <michals@xilinx.com>; Appana Durga Kedareswara Rao
> <appanad@xilinx.com>; dmaengine@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> Andrea Merello <andrea.merello@gmail.com>
> Subject: [PATCH 2/6] dmaengine: xilinx_dma: fix completion callback is no=
t
> invoked for each DMA operation
>=20
> API specification says: "On completion of each DMA operation, the next in
> queue is started and a tasklet triggered. The tasklet will then call the
> client driver completion callback routine for notification, if set."
>=20
> Currently the driver keeps a "desc_pendingcount" counter of the total
> descriptor pending, and it uses as IRQ coalesce threshold, as result it
> only calls the CBs after ALL pending operations are completed, which is
> wrong.
I think IRQ coalescing enable/disable should be configurable.=20
Performance related usecases will need this support.

>=20
> This patch uses disable IRQ coalesce and checks for the completion flag
> for the descriptors (which is further divided in segments).
>=20
> Possibly a better optimization could be using proper IRQ coalesce
> threshold to get an IRQ after all segments of the descriptors are done.
> But we don't do that yet..
>=20
> NOTE: for now we do this only for AXI DMA, other DMA flavors are
> untested/untouched.
> This is loosely based on
> commit 65df81a6dc74 ("xilinx_dma: IrqThreshold set incorrectly, unreliabl=
e.")
> in my linux-4.6-zynq tree
NOTE description doesn't help much. =20

>=20
> From: Jeremy Trimble [original patch]
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> ---
>  drivers/dma/xilinx/xilinx_dma.c | 39 +++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 14 deletions(-)
>=20
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_=
dma.c
> index a516e7ffef21..cf12f7147f07 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -164,6 +164,7 @@
>  #define XILINX_DMA_CR_COALESCE_SHIFT	16
>  #define XILINX_DMA_BD_SOP		BIT(27)
>  #define XILINX_DMA_BD_EOP		BIT(26)
> +#define XILINX_DMA_BD_CMPLT		BIT(31)
>  #define XILINX_DMA_COALESCE_MAX		255
>  #define XILINX_DMA_NUM_DESCS		255
>  #define XILINX_DMA_NUM_APP_WORDS	5
> @@ -1274,12 +1275,9 @@ static void xilinx_dma_start_transfer(struct
> xilinx_dma_chan *chan)
>=20
>  	reg =3D dma_ctrl_read(chan, XILINX_DMA_REG_DMACR);
>=20
> -	if (chan->desc_pendingcount <=3D XILINX_DMA_COALESCE_MAX) {
> -		reg &=3D ~XILINX_DMA_CR_COALESCE_MAX;
> -		reg |=3D chan->desc_pendingcount <<
> -				  XILINX_DMA_CR_COALESCE_SHIFT;
> -		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
> -	}
> +	reg &=3D ~XILINX_DMA_CR_COALESCE_MAX;
> +	reg |=3D 1 << XILINX_DMA_CR_COALESCE_SHIFT;
> +	dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>=20
>  	if (chan->has_sg && !chan->xdev->mcdma)
>  		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
> @@ -1378,6 +1376,20 @@ static void xilinx_dma_complete_descriptor(struct
> xilinx_dma_chan *chan)
>  		return;
>=20
>  	list_for_each_entry_safe(desc, next, &chan->active_list, node) {
> +		if (chan->xdev->dma_config->dmatype =3D=3D
> XDMA_TYPE_AXIDMA) {
> +			/*
> +			 * Check whether the last segment in this descriptor
> +			 * has been completed.
> +			 */
> +			const struct xilinx_axidma_tx_segment *const tail_seg
> =3D
> +				list_last_entry(&desc->segments,
> +						struct
> xilinx_axidma_tx_segment,
> +						node);
> +
> +			/* we've processed all the completed descriptors */
> +			if (!(tail_seg->hw.status & XILINX_DMA_BD_CMPLT))
> +				break;
> +		}
>  		list_del(&desc->node);
>  		if (!desc->cyclic)
>  			dma_cookie_complete(&desc->async_tx);
> @@ -1826,14 +1838,13 @@ static struct dma_async_tx_descriptor
> *xilinx_dma_prep_slave_sg(
>  				   struct xilinx_axidma_tx_segment, node);
>  	desc->async_tx.phys =3D segment->phys;
>=20
> -	/* For the last DMA_MEM_TO_DEV transfer, set EOP */
> -	if (chan->direction =3D=3D DMA_MEM_TO_DEV) {
> -		segment->hw.control |=3D XILINX_DMA_BD_SOP;
> -		segment =3D list_last_entry(&desc->segments,
> -					  struct xilinx_axidma_tx_segment,
> -					  node);
> -		segment->hw.control |=3D XILINX_DMA_BD_EOP;
> -	}
> +	/* For the first transfer, set SOP */
> +	segment->hw.control |=3D XILINX_DMA_BD_SOP;
> +	/* For the last transfer, set EOP */
> +	segment =3D list_last_entry(&desc->segments,
> +				  struct xilinx_axidma_tx_segment,
> +				  node);
> +	segment->hw.control |=3D XILINX_DMA_BD_EOP;
>=20
>  	return &desc->async_tx;
>=20
> --
> 2.17.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html