From: Appana Durga Kedareswara Rao <appana.durga.rao@xilinx.com> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "vinod.koul@intel.com" <vinod.koul@intel.com>, "michal.simek@xilinx.com" <michal.simek@xilinx.com>, Soren Brinkmann <sorenb@xilinx.com>, "moritz.fischer@ettus.com" <moritz.fischer@ettus.com>, "luis@debethencourt.com" <luis@debethencourt.com>, "Jose.Abreu@synopsys.com" <Jose.Abreu@synopsys.com>, "dmaengine@vger.kernel.org" <dmaengine@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> Subject: RE: [PATCH 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor Date: Mon, 19 Dec 2016 15:39:43 +0000 [thread overview] Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A65D58E@XAP-PVEXMBX01.xlnx.xilinx.com> (raw) In-Reply-To: <5248247.n0rV8xBPrZ@avalon> Hi Laurent Pinchart, Thanks for the review... > > > > + if (!chan->idle) > > + return; > > Don't you need to perform the same check for the DMA and CDMA channels ? If > so, shouldn't this be moved to common code ? Will fix it in v2... > > There's another problem (not strictly introduced by this patch) I wanted to > mention. The append_desc_queue() function, called from your tx_submit > handler, appends descriptors to the pending_list. The DMA engine API states > that a transfer submitted by tx_submit will not be executed until .issue_pending() > is called. However, if a transfer is in progress at tx_submit time, I believe that > the IRQ handler, at transfer completion, will start the next transfer from the > pending_list even if .issue_pending() hasn't been called for it. > > > if (list_empty(&chan->pending_list)) > > return; If user submits more than h/w limit then that case only driver will process The descriptors from the pending_list for other cases the pending_list will be Empty so driver just returns from there. > > Now that you catch busy channels with a software flag, do you still need the > xilinx_dma_is_running() and xilinx_dma_is_idle() checks ? Three different checks > for the same or very similar conditions are confusing, if you really need them > you should document clearly how they differ. Will remove the xilinx_dma_is_running and xilinx_dmais_idle() checks and will use Chan->idle check across all the IP's. Will fix it v2... > > > @@ -1110,6 +1116,7 @@ static void xilinx_vdma_start_transfer(struct > > xilinx_dma_chan *chan) vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, > > last->hw.vsize); > > } > > > > + chan->idle = false; > > (and this too) Will fix in v2... Regards, Kedar.
WARNING: multiple messages have this Message-ID (diff)
From: appana.durga.rao@xilinx.com (Appana Durga Kedareswara Rao) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor Date: Mon, 19 Dec 2016 15:39:43 +0000 [thread overview] Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A65D58E@XAP-PVEXMBX01.xlnx.xilinx.com> (raw) In-Reply-To: <5248247.n0rV8xBPrZ@avalon> Hi Laurent Pinchart, Thanks for the review... > > > > + if (!chan->idle) > > + return; > > Don't you need to perform the same check for the DMA and CDMA channels ? If > so, shouldn't this be moved to common code ? Will fix it in v2... > > There's another problem (not strictly introduced by this patch) I wanted to > mention. The append_desc_queue() function, called from your tx_submit > handler, appends descriptors to the pending_list. The DMA engine API states > that a transfer submitted by tx_submit will not be executed until .issue_pending() > is called. However, if a transfer is in progress at tx_submit time, I believe that > the IRQ handler, at transfer completion, will start the next transfer from the > pending_list even if .issue_pending() hasn't been called for it. > > > if (list_empty(&chan->pending_list)) > > return; If user submits more than h/w limit then that case only driver will process The descriptors from the pending_list for other cases the pending_list will be Empty so driver just returns from there. > > Now that you catch busy channels with a software flag, do you still need the > xilinx_dma_is_running() and xilinx_dma_is_idle() checks ? Three different checks > for the same or very similar conditions are confusing, if you really need them > you should document clearly how they differ. Will remove the xilinx_dma_is_running and xilinx_dmais_idle() checks and will use Chan->idle check across all the IP's. Will fix it v2... > > > @@ -1110,6 +1116,7 @@ static void xilinx_vdma_start_transfer(struct > > xilinx_dma_chan *chan) vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, > > last->hw.vsize); > > } > > > > + chan->idle = false; > > (and this too) Will fix in v2... Regards, Kedar.
next prev parent reply other threads:[~2016-12-19 15:54 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-12-15 15:11 [PATCH 0/3] dmaengine: xilinx_dma: Bug fixes Kedareswara rao Appana 2016-12-15 15:11 ` Kedareswara rao Appana 2016-12-15 15:11 ` [PATCH 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor Kedareswara rao Appana 2016-12-15 15:11 ` Kedareswara rao Appana 2016-12-15 15:45 ` Jose Abreu 2016-12-15 15:45 ` Jose Abreu 2016-12-15 18:49 ` Appana Durga Kedareswara Rao 2016-12-15 18:49 ` Appana Durga Kedareswara Rao 2016-12-16 15:35 ` Laurent Pinchart 2016-12-16 15:35 ` Laurent Pinchart 2016-12-19 15:39 ` Appana Durga Kedareswara Rao [this message] 2016-12-19 15:39 ` Appana Durga Kedareswara Rao 2016-12-19 17:18 ` Laurent Pinchart 2016-12-19 17:18 ` Laurent Pinchart 2016-12-23 8:49 ` Appana Durga Kedareswara Rao 2016-12-23 8:49 ` Appana Durga Kedareswara Rao 2016-12-15 15:11 ` [PATCH 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma Kedareswara rao Appana 2016-12-15 15:11 ` Kedareswara rao Appana 2016-12-15 16:10 ` Jose Abreu 2016-12-15 16:10 ` Jose Abreu 2016-12-15 19:09 ` Appana Durga Kedareswara Rao 2016-12-15 19:09 ` Appana Durga Kedareswara Rao 2016-12-16 10:11 ` Jose Abreu 2016-12-16 10:11 ` Jose Abreu 2016-12-19 15:40 ` Appana Durga Kedareswara Rao 2016-12-19 15:40 ` Appana Durga Kedareswara Rao 2016-12-16 15:54 ` Laurent Pinchart 2016-12-16 15:54 ` Laurent Pinchart 2016-12-19 15:41 ` Appana Durga Kedareswara Rao 2016-12-19 15:41 ` Appana Durga Kedareswara Rao 2016-12-15 15:11 ` [PATCH 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario Kedareswara rao Appana 2016-12-15 15:11 ` Kedareswara rao Appana
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=C246CAC1457055469EF09E3A7AC4E11A4A65D58E@XAP-PVEXMBX01.xlnx.xilinx.com \ --to=appana.durga.rao@xilinx.com \ --cc=Jose.Abreu@synopsys.com \ --cc=dan.j.williams@intel.com \ --cc=dmaengine@vger.kernel.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luis@debethencourt.com \ --cc=michal.simek@xilinx.com \ --cc=moritz.fischer@ettus.com \ --cc=sorenb@xilinx.com \ --cc=vinod.koul@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.