Re: [PATCH] dmaengine: cppi41: Fix cppi41_dma_prep_slave_sg() when idle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 23/10/2019 23:18, Tony Lindgren wrote:
* Peter Ujfalusi <peter.ujfalusi@xxxxxx> [191023 19:55]:
On 10/23/19 10:18 PM, Tony Lindgren wrote:
We'd have to allow dma consumer driver call pm_runtime_get_sync()
on the dma device. Something similar maybe to what we have
for phy_pm_runtime_get_sync(). Or just get the device handle for
dma so the consumer can call pm_runtime_get_sync() on it.

How much a pm_runtime_get_sync(dmadev) is different when it is issued by
the client driver compared to when the dma driver issues it for it's own
device?

Well the consumer device could call pm_runtime_get_sync(dmadev)
when the USB cable is connected for example, and then call
pm_runtime_pu(dmadev) when let's say the USB cable is disconnected.

Without using pm_runtime_irq_safe() we currently don't have a
clear path for doing this where the pm_runtime_get_sync(dmadev)
may sleep.

But I still fail to see the difference between the events before this
patch and with the case when there is a 100ms delay between prep_sg and
issue_pending.

Before this patch:

prep_sg()
issue_pending() <- runtime_get() /  put_autosuspend()
		   _not_ starting transfer
runtime_resume() <- starts the transfer

With this patch and than 100ms delay between prep_sg and issue_pending:

prep_sg() <- runtime_get() /  put_autosuspend()
runtime_resume() <- not starting transfer
issue_pending() <- runtime_get() /  put_autosuspend()
		   starts the transfer

With this patch, but more than 100ms delay in between:

prep_sg() <- runtime_get() /  put_autosuspend()
runtime_resume() <- not starting transfer
100ms delay
runtime_suspend()
issue_pending() <- runtime_get() /  put_autosuspend()
		   _not_ starting transfer
runtime_resume() <- starts the transfer

pm_runtime_get_sync() in issue_pending would be the solution to avoid
delayed execution, but the usb driver should not assume that DMA is
completed as soon as issue_pending returned.

Oh I see. Yes the consumer driver would need to check for
the completed dma transfer in all cases. The delay issues
should not currently happen in the musb_ep_program() problem
case as it gets called from IRQ context.

And no, adding pm_runtime_get_sync() to issue_pending is not
a solution. There may be clocks and regulators that need to
be powered up, and we don't want to use pm_runtime_irq_safe()
because of the permanent use count on the parent.

5 cents.

I think the right thing might be to get rid of pm_runtime_xxx()
in cppi41_dma_issue_pending(). So overall approach will be:

- new job -> cppi41_dma_prep_slave_sg() -> pm_runtime_get()
- issue_pending: fill backlog if suspended or run_queue if active (pm_runtime_active())
- job done: dmaengine_desc_get_callback_invoke() ->

	dmaengine_desc_get_callback_invoke();
	pm_runtime_mark_last_busy(cdd->ddev.dev);
	pm_runtime_put_autosuspend(cdd->ddev.dev);
  in all places.

It even might allow to get rid of cdd->lock.


--
Best regards,
grygorii



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux