From:   "Gaddam, Sarath Babu Naidu" <sarath.babu.naidu.gaddam@amd.com>
To:     Vinod Koul <vkoul@kernel.org>
CC:     "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "lars@metafoo.de" <lars@metafoo.de>,
        "adrianml@alumnos.upm.es" <adrianml@alumnos.upm.es>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Simek, Michal" <michal.simek@amd.com>,
        "Pandey, Radhey Shyam" <radhey.shyam.pandey@amd.com>,
        "Sarangi, Anirudha" <anirudha.sarangi@amd.com>,
        "Katakam, Harini" <harini.katakam@amd.com>,
        "git (AMD-Xilinx)" <git@amd.com>
Subject: RE: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt delay
 timeout
Thread-Topic: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt delay
 timeout
Thread-Index: AQHY/++1voKTnTLde0axfE0Yfexfaq6DV2WAgDhcHzA=
Date:   Thu, 2 Feb 2023 07:52:00 +0000
Message-ID: <MW5PR12MB55981EAC28AC5A32BF76CF7C87D69@MW5PR12MB5598.namprd12.prod.outlook.com>
References: <20221124102745.2620370-1-sarath.babu.naidu.gaddam@amd.com>
 <20221124102745.2620370-7-sarath.babu.naidu.gaddam@amd.com>
 <Y6wh3W0S/pzTI4vL@matsya>
In-Reply-To: <Y6wh3W0S/pzTI4vL@matsya>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?HEZVOwTut+mKENfwROVLWkkZvQ8VCFytAGHtYtMuWvmPEnfil1koIvEir5Pa?=
 =?us-ascii?Q?2YzXnKAqU9buBeX278xafPY9nhDiiOQHZGW67M2FrXDvxgZCkeZQSVSIvB73?=
 =?us-ascii?Q?+xcjPguYzXDwDbFrrJLF/ffJNFfQXGXNAMROo59A0BytJnXbuUgQpFdahXkX?=
 =?us-ascii?Q?yhDkJPZE//qsmeyfxSmHS+GVqyG7mwsFwebo6uopHoac9qNaU59/3DbceY4n?=
 =?us-ascii?Q?Z1inMS60X0k+fcpDPVogp6Kj6qcnURK1YDLT3oJC7hEvAM/xh8lD3X976v3y?=
 =?us-ascii?Q?CGOQJDSEIwOxEyJ/c/JUHHbZcskRaO2qYN0a3b0H4nKATIK/9GlDZL00UAUd?=
 =?us-ascii?Q?j+1qOs2EaaeB3EA5SskWyNZA7M8cRq56rle2Ze/yF6Du9aBHNwdgAtdfBzEs?=
 =?us-ascii?Q?3159b1Zi9MhsiMXY7KFOMdCcliCsp//gLRn0oUHpCaxYrh3Oyli/Mh9dKqbV?=
 =?us-ascii?Q?Vx2rDV5ckuOZXMaYB2xeTn0FzAnw8OSQ9Y2XUNY6eY1oyqUnCQrHPm81p5RN?=
 =?us-ascii?Q?PcukaxCsE/lgEJZBOm5YlzJVN77Rbl2NNt/tERjSsytOuU5OEgMO7hDT1PYD?=
 =?us-ascii?Q?ea9kr3+8TyXwMNCzQTGYNF9oXlZLGEpdUVvJoFPselxKFfBYJiQmxo/3kAk+?=
 =?us-ascii?Q?PjEK8Y3b1PFBrNPkjGNblQvh3TkPAknH1Ge6U5kZGZzGWnnAuUMwAZTkID01?=
 =?us-ascii?Q?vDEfP8ejhV/R/4uHZkFGCRQcVRwe2sfzfSFAFWnus9swjNH2cPA8xmUDgN3T?=
 =?us-ascii?Q?OgnoTZnLcBvK7rJ/cfBCIYc77n3pMd/+QDxgCgFKuK6+UXwJken5iv/2eTeh?=
 =?us-ascii?Q?cvJgzhGXPMsMEOxtxCCpaZrHTLdziPsnx0yX6gxoIhYfAqvSpDWKlHpJefU5?=
 =?us-ascii?Q?dCPXnGpaDWHgv9+s0bvwWOWfAZF4vuNonJfrwADSo+Nj024GFPgwffAJftvS?=
 =?us-ascii?Q?nMKbEEC7hFhmH1XR5ZE2b/S8NdStikxs9rQyO778u3tyxrNpGVBjuW8Xt4fA?=
 =?us-ascii?Q?eXSyWnOSGb7ggnzpPNq8Gn8pT+7TVNNoTQCkO3uyrG7zIKG6lT9GQ01Z27dx?=
 =?us-ascii?Q?OU4L6/Be+XnRZs+H9tq2v3hynK2HvPbBD6tuRIbANw21pc0KGmK1lfKjBVIp?=
 =?us-ascii?Q?zfz3kWxI78d0CD+aYMB9755OazIIjY1seF+cpQzdt+/AQT6qRmVLCIoMLryq?=
 =?us-ascii?Q?LeY04TI8/vJBvkSK7fF93E0cchXonG5wi2KeCoezbeQewWCyeV4IRZ5AVlYU?=
 =?us-ascii?Q?0LD0GIJS7uh71loZAQvwIdnMg/o78w92zo3qr2SAIvg0ekQHH7wwe0ZHeCSP?=
 =?us-ascii?Q?IhdkDlU7vgsXsww+KcFvVE3o+xapOqa7BfB0lAB+DUtjl7t4/mSRgV89t9bQ?=
 =?us-ascii?Q?6i+OCCJ3EM1cxSfZBVYrvWoG86TIJX3+hTIX+I4UK+HJSg3NZdr7Zqz/KtHz?=
 =?us-ascii?Q?K9m4tXBcK9CxnlTrHfpTHaKf8P/QdKdCGiCyqPz0z6VOcnkxFgjDFRT4qe2O?=
 =?us-ascii?Q?VFLCWR+7CDxnLfvofa10rov043I6yoPFAPnP5IFOWKhhte1KXsbxcTc/D/Xe?=
 =?us-ascii?Q?QOJtu0YCaI3tmET49cI=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MW5PR12MB5598.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b2166b4-ff4e-46ce-65af-08db04f25db7
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Feb 2023 07:52:00.3485
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: LVlNIuWsDva0C7ArHph30W3aPpNtweTVJrxxl0udUZzv/QkU8Bkw5UWVq3Ql7JXudX4qbqnQFWWT4FfCNi/Daw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5901
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Wednesday, December 28, 2022 4:31 PM
> To: Gaddam, Sarath Babu Naidu
> <sarath.babu.naidu.gaddam@amd.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> lars@metafoo.de; adrianml@alumnos.upm.es;
> dmaengine@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Simek, Michal
> <michal.simek@amd.com>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Sarangi, Anirudha
> <anirudha.sarangi@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH V2 6/6] dmaengine: xilinx_dma: Program interrupt
> delay timeout
>=20
> On 24-11-22, 15:57, Sarath Babu Naidu Gaddam wrote:
> > From: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> >
> > Program IRQDelay for AXI DMA. The interrupt timeout mechanism
> causes
> > the DMA engine to generate an interrupt after the delay time period
> > has expired. It enables dmaengine to respond in real-time even though
> > interrupt coalescing is configured. It also remove the placeholder for
> > delay interrupt and merge it with frame completion interrupt.
> > Since by default interrupt delay timeout is disabled this feature
> > addition has no functional impact on VDMA and CDMA IP's.
> >
> > Signed-off-by: Radhey Shyam Pandey
> <radhey.shyam.pandey@xilinx.com>
> > Signed-off-by: Sarath Babu Naidu Gaddam
> > <sarath.babu.naidu.gaddam@amd.com>
> > ---
> >  drivers/dma/xilinx/xilinx_dma.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> > b/drivers/dma/xilinx/xilinx_dma.c index ce0c151d8f61..333d68ee3559
> > 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -173,8 +173,10 @@
> >  #define XILINX_DMA_MAX_TRANS_LEN_MAX	23
> >  #define XILINX_DMA_V2_MAX_TRANS_LEN_MAX	26
> >  #define XILINX_DMA_CR_COALESCE_MAX	GENMASK(23, 16)
> > +#define XILINX_DMA_CR_DELAY_MAX		GENMASK(31, 24)
> >  #define XILINX_DMA_CR_CYCLIC_BD_EN_MASK	BIT(4)
> >  #define XILINX_DMA_CR_COALESCE_SHIFT	16
> > +#define XILINX_DMA_CR_DELAY_SHIFT	24
> >  #define XILINX_DMA_BD_SOP		BIT(27)
> >  #define XILINX_DMA_BD_EOP		BIT(26)
> >  #define XILINX_DMA_COALESCE_MAX		255
> > @@ -410,6 +412,7 @@ struct xilinx_dma_tx_descriptor {
> >   * @stop_transfer: Differentiate b/w DMA IP's quiesce
> >   * @tdest: TDEST value for mcdma
> >   * @has_vflip: S2MM vertical flip
> > + * @irq_delay: Interrupt delay timeout
> >   */
> >  struct xilinx_dma_chan {
> >  	struct xilinx_dma_device *xdev;
> > @@ -448,6 +451,7 @@ struct xilinx_dma_chan {
> >  	int (*stop_transfer)(struct xilinx_dma_chan *chan);
> >  	u16 tdest;
> >  	bool has_vflip;
> > +	u8 irq_delay;
> >  };
> >
> >  /**
> > @@ -1560,6 +1564,9 @@ static void xilinx_dma_start_transfer(struct
> xilinx_dma_chan *chan)
> >  	if (chan->has_sg)
> >  		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
> >  			     head_desc->async_tx.phys);
> > +	reg  &=3D ~XILINX_DMA_CR_DELAY_MAX;
> > +	reg  |=3D chan->irq_delay << XILINX_DMA_CR_DELAY_SHIFT;
> > +	dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
> >
> >  	xilinx_dma_start(chan);
> >
> > @@ -1887,15 +1894,8 @@ static irqreturn_t
> xilinx_dma_irq_handler(int irq, void *data)
> >  		}
> >  	}
> >
> > -	if (status & XILINX_DMA_DMASR_DLY_CNT_IRQ) {
> > -		/*
> > -		 * Device takes too long to do the transfer when user
> requires
> > -		 * responsiveness.
> > -		 */
> > -		dev_dbg(chan->dev, "Inter-packet latency too long\n");
> > -	}
> > -
> > -	if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
> > +	if (status & (XILINX_DMA_DMASR_FRM_CNT_IRQ |
> > +		      XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
> >  		spin_lock(&chan->lock);
> >  		xilinx_dma_complete_descriptor(chan);
> >  		chan->idle =3D true;
> > @@ -2822,6 +2822,8 @@ static int xilinx_dma_chan_probe(struct
> xilinx_dma_device *xdev,
> >  	/* Retrieve the channel properties from the device tree */
> >  	has_dre =3D of_property_read_bool(node, "xlnx,include-dre");
> >
> > +	of_property_read_u8(node, "xlnx,irq-delay", &chan->irq_delay);
>=20
> Same question here too

This is an optional property. If it is not there, then driver programs
 this value as zero and it does not break existing functionality. =20

>=20
> > +
> >  	chan->genlock =3D of_property_read_bool(node, "xlnx,genlock-
> mode");
> >
> >  	err =3D of_property_read_u32(node, "xlnx,datawidth", &value);
> > --
> > 2.25.1
>=20
> --
> ~Vinod