Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753090AbdHIOYj (ORCPT ); Wed, 9 Aug 2017 10:24:39 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:38214 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752693AbdHIOYh (ORCPT ); Wed, 9 Aug 2017 10:24:37 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 201D3601D2 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sricharan@codeaurora.org Subject: Re: [PATCH v2] dmaengine: qcom-bam: Process multiple pending descriptors To: Abhishek Sahu Cc: vinod.koul@intel.com, andy.gross@linaro.org, david.brown@linaro.org, dan.j.williams@intel.com, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, dmaengine-owner@vger.kernel.org References: <1501766513-21349-1-git-send-email-sricharan@codeaurora.org> <43a9e692d8498b0e4469e4225d428e84@codeaurora.org> From: Sricharan R Message-ID: <1b266640-8503-91bb-a467-6bdab62b21f5@codeaurora.org> Date: Wed, 9 Aug 2017 19:54:30 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <43a9e692d8498b0e4469e4225d428e84@codeaurora.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Antivirus: Avast (VPS 170808-2, 08/08/2017), Outbound message X-Antivirus-Status: Clean Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4696 Lines: 143 Hi Abhishek, On 8/9/2017 7:48 PM, Abhishek Sahu wrote: > On 2017-08-03 18:51, Sricharan R wrote: >> The bam dmaengine has a circular FIFO to which we >> add hw descriptors that describes the transaction. >> The FIFO has space for about 4096 hw descriptors. >> >> Currently we add one descriptor and wait for it to >> complete with interrupt and then add the next pending >> descriptor. In this way, the FIFO is underutilized >> since only one descriptor is processed at a time, although >> there is space in FIFO for the BAM to process more. >> >> Instead keep adding descriptors to FIFO till its full, >> that allows BAM to continue to work on the next descriptor >> immediately after signalling completion interrupt for the >> previous descriptor. >> >> Also when the client has not set the DMA_PREP_INTERRUPT for >> a descriptor, then do not configure BAM to trigger a interrupt >> upon completion of that descriptor. This way we get a interrupt >> only for the descriptor for which DMA_PREP_INTERRUPT was >> requested and there signal completion of all the previous completed >> descriptors. So we still do callbacks for all requested descriptors, >> but just that the number of interrupts are reduced. >> >> CURRENT: >> >> ------ ------- --------------- >> |DES 0| |DESC 1| |DESC 2 + INT | >> ------ ------- --------------- >> | | | >> | | | >> INTERRUPT: (INT) (INT) (INT) >> CALLBACK: (CB) (CB) (CB) >> >> MTD_SPEEDTEST READ PAGE: 3560 KiB/s >> MTD_SPEEDTEST WRITE PAGE: 2664 KiB/s >> IOZONE READ: 2456 KB/s >> IOZONE WRITE: 1230 KB/s >> >> bam dma interrupts (after tests): 96508 >> >> CHANGE: >> >> ------ ------- ------------- >> |DES 0| |DESC 1 |DESC 2 + INT | >> ------ ------- -------------- >> | >> | >> (INT) >> (CB for 0, 1, 2) >> >> MTD_SPEEDTEST READ PAGE: 3860 KiB/s >> MTD_SPEEDTEST WRITE PAGE: 2837 KiB/s >> IOZONE READ: 2677 KB/s >> IOZONE WRITE: 1308 KB/s >> >> bam dma interrupts (after tests): 58806 >> >> Signed-off-by: Sricharan R > > Thanks Sricharan for your patch to do the descriptor > clubbing in BAM DMA driver. > > Verified this patch with my NAND QPIC patches > https://www.spinics.net/lists/kernel/msg2573736.html > > I run the MTD test overnight and no failure > observed. Also, achieved significant improvement in > NAND speed. Following are the numbers for IPQ4019 > DK04 board. > > Test Speed in KiB/s > Before After > > eraseblock write speed 4716 5483 > eraseblock read speed 6855 8294 > page write speed 4678 5436 > page read speed 6784 8217 > > Tested-by: Abhishek Sahu > Thanks for the testing. > Also, I reviewed the patch and following are > minor comments. ok. >> &bchan->vc.desc_issued); >> - bchan->curr_txd = NULL; >> - } >> + list_for_each_entry(async_desc, &bchan->desc_list, desc_node) >> + list_add(&async_desc->vd.node, &bchan->vc.desc_issued); > > should we free the list also since we are adding these descriptor > back to issued and vchan_dma_desc_free_list will free all theses > descriptors > > When the IRQ will be triggered then it will traverse this list > and fetch the async descriptor for which already we freed the > memory. ha ok, should add list_del here. >> 1); >> + >> + list_for_each_entry_safe(async_desc, tmp, >> + &bchan->desc_list, desc_node) { >> + if (async_desc) { > > Do we need this check since async_desc will be always not NULL. not needed, will remove. >> /* if work pending and idle, start a transaction */ >> - if (vchan_issue_pending(&bchan->vc) && !bchan->curr_txd) >> + if (vchan_issue_pending(&bchan->vc) && !bchan->is_busy) >> bam_start_dma(bchan); > > can we get rid of these bchan->is_busy since it is being used > whether we have space in actual hardware FIFO and same we can > check with CIRC_SPACE(bchan->tail, bchan->head, MAX_DESCRIPTORS + 1) > yeah, this variable can be removed and simplified. Will do in V3. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus