Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-20 Thread Lucas Stach
Am Dienstag, dem 20.04.2021 um 13:47 + schrieb Robin Gong:
> On 2021/04/19 17:46 Lucas Stach  wrote:
> > Am Montag, dem 19.04.2021 um 07:17 + schrieb Robin Gong:
> > > Hi Lucas,
> > > 
> > > On 2021/04/14 Lucas Stach  wrote:
> > > > Hi Robin,
> > > > 
> > > > Am Mittwoch, dem 14.04.2021 um 14:33 + schrieb Robin Gong:
> > > > > On 2020/05/20 17:43 Lucas Stach  wrote:
> > > > > > Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > > > > > > Hi
> > > > > > > 
> > > > > > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach
> > > > > > > 
> > > > > > wrote:
> > > > > > > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > > > > > > There are two requirements that we need to move the
> > > > > > > > > request of dma channel from probe to open.
> > > > > > > > 
> > > > > > > > How do you handle -EPROBE_DEFER return code from the channel
> > > > > > > > request if you don't do it in probe?
> > > > > > > 
> > > > > > > I use the dma_request_slave_channel or dma_request_channel
> > > > > > > instead of dmaengine_pcm_request_chan_of. so there should be
> > > > > > > not -EPROBE_DEFER return code.
> > > > > > 
> > > > > > This is a pretty weak argument. The dmaengine device might probe
> > > > > > after you try to get the channel. Using a function to request
> > > > > > the channel that doesn't allow you to handle probe deferral is
> > > > > > IMHO a bug and should be fixed, instead of building even more
> > > > > > assumptions on top
> > > > of it.
> > > > > > 
> > > > > > > > > - When dma device binds with power-domains, the power will
> > > > > > > > > be enabled when we request dma channel. If the request of
> > > > > > > > > dma channel happen on probe, then the power-domains will
> > > > > > > > > be always enabled after kernel boot up,  which is not good
> > > > > > > > > for power saving,  so we need to move the request of dma
> > > > > > > > > channel to .open();
> > > > > > > > 
> > > > > > > > This is certainly something which could be fixed in the
> > > > > > > > dmaengine driver.
> > > > > > > 
> > > > > > > Dma driver always call the pm_runtime_get_sync in
> > > > > > > device_alloc_chan_resources, the device_alloc_chan_resources
> > > > > > > is called when channel is requested. so power is enabled on
> > > > > > > channel
> > > > request.
> > > > > > 
> > > > > > So why can't you fix the dmaengine driver to do that RPM call at
> > > > > > a later time when the channel is actually going to be used? This
> > > > > > will allow further power savings with other slave devices than the 
> > > > > > audio
> > PCM.
> > > > > Hi Lucas,
> > > > >   Thanks for your suggestion. I have tried to implement runtime
> > > > > autosuspend in fsl-edma driver on i.mx8qm/qxp with delay time (2
> > > > > sec) for this feature as below (or you can refer to
> > > > > drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
> > > > > pm_runtime_put_autosuspend in all dmaengine driver interface like
> > > > > device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_c
> > > > > ycli
> > > > > c/
> > > > > device_tx_status...
> > > > > 
> > > > > 
> > > > > pm_runtime_use_autosuspend(fsl_chan->dev);
> > > > > pm_runtime_set_autosuspend_delay(fsl_chan->
> > dev,
> > > > 2000);
> > > > > 
> > > > > That could resolve this audio case since the autosuspend could
> > > > > suspend runtime after
> > > > > 2 seconds if there is no further dma transfer but only channel
> > > > request(device_alloc_chan_resources).
> > > > > But unfortunately, it cause another issue. As you know, on our
> > > > > i.mx8qm/qxp, power domain done by scfw
> > > > > (drivers/firmware/imx/scu-pd.c)
> > > > over mailbox:
> > > > >  imx_sc_pd_power()->imx_scu_call_rpc()->
> > > > > imx_scu_ipc_write()->mbox_send_message()
> > > > > which means have to 'waits for completion', meanwhile, some driver
> > > > > like tty will call dmaengine interfaces in non-atomic case as
> > > > > below,
> > > > > 
> > > > > static int uart_write(struct tty_struct *tty, const unsigned char
> > > > > *buf, int count) {
> > > > >    ...
> > > > >   port = uart_port_lock(state, flags);
> > > > >    ..
> > > > > __uart_start(tty);  //call
> > start_tx()->dmaengine_prep_slave_sg...
> > > > > uart_port_unlock(port, flags);
> > > > > return ret;
> > > > > }
> > > > > 
> > > > > Thus dma runtime resume may happen in that timing window and cause
> > > > kernel alarm.
> > > > > I'm not sure whether there are similar limitations on other driver
> > > > > subsystem. But for me, It looks like the only way to resolve the
> > > > > contradiction between tty and scu-pd (hardware limitation on
> > > > > i.mx8qm/qxp) is to give up autosuspend and keep
> > > > > pm_runtime_get_sync
> > > > only in device_alloc_chan_resources because request channel is a
> > > > safe non-atomic phase.
> > > > > Do you have any idea? Thanks in advance.
> > > > 
> > > > If you look closely at the driver 

RE: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-20 Thread Robin Gong
On 2021/04/19 17:46 Lucas Stach  wrote:
> Am Montag, dem 19.04.2021 um 07:17 + schrieb Robin Gong:
> > Hi Lucas,
> >
> > On 2021/04/14 Lucas Stach  wrote:
> > > Hi Robin,
> > >
> > > Am Mittwoch, dem 14.04.2021 um 14:33 + schrieb Robin Gong:
> > > > On 2020/05/20 17:43 Lucas Stach  wrote:
> > > > > Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > > > > > Hi
> > > > > >
> > > > > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach
> > > > > > 
> > > > > wrote:
> > > > > > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > > > > > There are two requirements that we need to move the
> > > > > > > > request of dma channel from probe to open.
> > > > > > >
> > > > > > > How do you handle -EPROBE_DEFER return code from the channel
> > > > > > > request if you don't do it in probe?
> > > > > >
> > > > > > I use the dma_request_slave_channel or dma_request_channel
> > > > > > instead of dmaengine_pcm_request_chan_of. so there should be
> > > > > > not -EPROBE_DEFER return code.
> > > > >
> > > > > This is a pretty weak argument. The dmaengine device might probe
> > > > > after you try to get the channel. Using a function to request
> > > > > the channel that doesn't allow you to handle probe deferral is
> > > > > IMHO a bug and should be fixed, instead of building even more
> > > > > assumptions on top
> > > of it.
> > > > >
> > > > > > > > - When dma device binds with power-domains, the power will
> > > > > > > > be enabled when we request dma channel. If the request of
> > > > > > > > dma channel happen on probe, then the power-domains will
> > > > > > > > be always enabled after kernel boot up,  which is not good
> > > > > > > > for power saving,  so we need to move the request of dma
> > > > > > > > channel to .open();
> > > > > > >
> > > > > > > This is certainly something which could be fixed in the
> > > > > > > dmaengine driver.
> > > > > >
> > > > > > Dma driver always call the pm_runtime_get_sync in
> > > > > > device_alloc_chan_resources, the device_alloc_chan_resources
> > > > > > is called when channel is requested. so power is enabled on
> > > > > > channel
> > > request.
> > > > >
> > > > > So why can't you fix the dmaengine driver to do that RPM call at
> > > > > a later time when the channel is actually going to be used? This
> > > > > will allow further power savings with other slave devices than the 
> > > > > audio
> PCM.
> > > > Hi Lucas,
> > > >   Thanks for your suggestion. I have tried to implement runtime
> > > > autosuspend in fsl-edma driver on i.mx8qm/qxp with delay time (2
> > > > sec) for this feature as below (or you can refer to
> > > > drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
> > > > pm_runtime_put_autosuspend in all dmaengine driver interface like
> > > > device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_c
> > > > ycli
> > > > c/
> > > > device_tx_status...
> > > >
> > > >
> > > > pm_runtime_use_autosuspend(fsl_chan->dev);
> > > > pm_runtime_set_autosuspend_delay(fsl_chan->
> dev,
> > > 2000);
> > > >
> > > > That could resolve this audio case since the autosuspend could
> > > > suspend runtime after
> > > > 2 seconds if there is no further dma transfer but only channel
> > > request(device_alloc_chan_resources).
> > > > But unfortunately, it cause another issue. As you know, on our
> > > > i.mx8qm/qxp, power domain done by scfw
> > > > (drivers/firmware/imx/scu-pd.c)
> > > over mailbox:
> > > >  imx_sc_pd_power()->imx_scu_call_rpc()->
> > > > imx_scu_ipc_write()->mbox_send_message()
> > > > which means have to 'waits for completion', meanwhile, some driver
> > > > like tty will call dmaengine interfaces in non-atomic case as
> > > > below,
> > > >
> > > > static int uart_write(struct tty_struct *tty, const unsigned char
> > > > *buf, int count) {
> > > >    ...
> > > > port = uart_port_lock(state, flags);
> > > >    ..
> > > > __uart_start(tty);  //call
> start_tx()->dmaengine_prep_slave_sg...
> > > > uart_port_unlock(port, flags);
> > > > return ret;
> > > > }
> > > >
> > > > Thus dma runtime resume may happen in that timing window and cause
> > > kernel alarm.
> > > > I'm not sure whether there are similar limitations on other driver
> > > > subsystem. But for me, It looks like the only way to resolve the
> > > > contradiction between tty and scu-pd (hardware limitation on
> > > > i.mx8qm/qxp) is to give up autosuspend and keep
> > > > pm_runtime_get_sync
> > > only in device_alloc_chan_resources because request channel is a
> > > safe non-atomic phase.
> > > > Do you have any idea? Thanks in advance.
> > >
> > > If you look closely at the driver you used as an example (hidma.c)
> > > it looks like there is already something in there, which looks very
> > > much like what you need
> > > here:
> > >
> > > In hidma_issue_pending() the driver tries to get the device to runtime
> resume.
> > > If this doesn't work, maybe due to the power 

Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-19 Thread Lucas Stach
Am Montag, dem 19.04.2021 um 07:17 + schrieb Robin Gong:
> Hi Lucas,
> 
> On 2021/04/14 Lucas Stach  wrote:
> > Hi Robin,
> > 
> > Am Mittwoch, dem 14.04.2021 um 14:33 + schrieb Robin Gong:
> > > On 2020/05/20 17:43 Lucas Stach  wrote:
> > > > Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > > > > Hi
> > > > > 
> > > > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach
> > > > > 
> > > > wrote:
> > > > > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > > > > There are two requirements that we need to move the request of
> > > > > > > dma channel from probe to open.
> > > > > > 
> > > > > > How do you handle -EPROBE_DEFER return code from the channel
> > > > > > request if you don't do it in probe?
> > > > > 
> > > > > I use the dma_request_slave_channel or dma_request_channel instead
> > > > > of dmaengine_pcm_request_chan_of. so there should be not
> > > > > -EPROBE_DEFER return code.
> > > > 
> > > > This is a pretty weak argument. The dmaengine device might probe
> > > > after you try to get the channel. Using a function to request the
> > > > channel that doesn't allow you to handle probe deferral is IMHO a
> > > > bug and should be fixed, instead of building even more assumptions on 
> > > > top
> > of it.
> > > > 
> > > > > > > - When dma device binds with power-domains, the power will be
> > > > > > > enabled when we request dma channel. If the request of dma
> > > > > > > channel happen on probe, then the power-domains will be always
> > > > > > > enabled after kernel boot up,  which is not good for power
> > > > > > > saving,  so we need to move the request of dma channel to
> > > > > > > .open();
> > > > > > 
> > > > > > This is certainly something which could be fixed in the
> > > > > > dmaengine driver.
> > > > > 
> > > > > Dma driver always call the pm_runtime_get_sync in
> > > > > device_alloc_chan_resources, the device_alloc_chan_resources is
> > > > > called when channel is requested. so power is enabled on channel
> > request.
> > > > 
> > > > So why can't you fix the dmaengine driver to do that RPM call at a
> > > > later time when the channel is actually going to be used? This will
> > > > allow further power savings with other slave devices than the audio PCM.
> > > Hi Lucas,
> > >   Thanks for your suggestion. I have tried to implement runtime
> > > autosuspend in fsl-edma driver on i.mx8qm/qxp with delay time (2 sec)
> > > for this feature as below (or you can refer to
> > > drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
> > > pm_runtime_put_autosuspend in all dmaengine driver interface like
> > > device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_cycli
> > > c/
> > > device_tx_status...
> > > 
> > > 
> > > pm_runtime_use_autosuspend(fsl_chan->dev);
> > > pm_runtime_set_autosuspend_delay(fsl_chan->dev,
> > 2000);
> > > 
> > > That could resolve this audio case since the autosuspend could suspend
> > > runtime after
> > > 2 seconds if there is no further dma transfer but only channel
> > request(device_alloc_chan_resources).
> > > But unfortunately, it cause another issue. As you know, on our
> > > i.mx8qm/qxp, power domain done by scfw (drivers/firmware/imx/scu-pd.c)
> > over mailbox:
> > >  imx_sc_pd_power()->imx_scu_call_rpc()->
> > > imx_scu_ipc_write()->mbox_send_message()
> > > which means have to 'waits for completion', meanwhile, some driver
> > > like tty will call dmaengine interfaces in non-atomic case as below,
> > > 
> > > static int uart_write(struct tty_struct *tty, const unsigned char
> > > *buf, int count) {
> > >    ...
> > >   port = uart_port_lock(state, flags);
> > >    ..
> > > __uart_start(tty);  //call start_tx()->dmaengine_prep_slave_sg...
> > > uart_port_unlock(port, flags);
> > > return ret;
> > > }
> > > 
> > > Thus dma runtime resume may happen in that timing window and cause
> > kernel alarm.
> > > I'm not sure whether there are similar limitations on other driver
> > > subsystem. But for me, It looks like the only way to resolve the
> > > contradiction between tty and scu-pd (hardware limitation on
> > > i.mx8qm/qxp) is to give up autosuspend and keep pm_runtime_get_sync
> > only in device_alloc_chan_resources because request channel is a safe
> > non-atomic phase.
> > > Do you have any idea? Thanks in advance.
> > 
> > If you look closely at the driver you used as an example (hidma.c) it looks 
> > like
> > there is already something in there, which looks very much like what you 
> > need
> > here:
> > 
> > In hidma_issue_pending() the driver tries to get the device to runtime 
> > resume.
> > If this doesn't work, maybe due to the power domain code not being able to
> > be called in atomic context, the actual work of waking up the dma hardware
> > and issuing the descriptor is shunted to a tasklet.
> > 
> > If I'm reading this right, this is exactly what you need here to be able to 
> > call the
> > dmaengine code from at

RE: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-19 Thread Robin Gong
Hi Lucas,

On 2021/04/14 Lucas Stach  wrote:
> Hi Robin,
> 
> Am Mittwoch, dem 14.04.2021 um 14:33 + schrieb Robin Gong:
> > On 2020/05/20 17:43 Lucas Stach  wrote:
> > > Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > > > Hi
> > > >
> > > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach
> > > > 
> > > wrote:
> > > > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > > > There are two requirements that we need to move the request of
> > > > > > dma channel from probe to open.
> > > > >
> > > > > How do you handle -EPROBE_DEFER return code from the channel
> > > > > request if you don't do it in probe?
> > > >
> > > > I use the dma_request_slave_channel or dma_request_channel instead
> > > > of dmaengine_pcm_request_chan_of. so there should be not
> > > > -EPROBE_DEFER return code.
> > >
> > > This is a pretty weak argument. The dmaengine device might probe
> > > after you try to get the channel. Using a function to request the
> > > channel that doesn't allow you to handle probe deferral is IMHO a
> > > bug and should be fixed, instead of building even more assumptions on top
> of it.
> > >
> > > > > > - When dma device binds with power-domains, the power will be
> > > > > > enabled when we request dma channel. If the request of dma
> > > > > > channel happen on probe, then the power-domains will be always
> > > > > > enabled after kernel boot up,  which is not good for power
> > > > > > saving,  so we need to move the request of dma channel to
> > > > > > .open();
> > > > >
> > > > > This is certainly something which could be fixed in the
> > > > > dmaengine driver.
> > > >
> > > > Dma driver always call the pm_runtime_get_sync in
> > > > device_alloc_chan_resources, the device_alloc_chan_resources is
> > > > called when channel is requested. so power is enabled on channel
> request.
> > >
> > > So why can't you fix the dmaengine driver to do that RPM call at a
> > > later time when the channel is actually going to be used? This will
> > > allow further power savings with other slave devices than the audio PCM.
> > Hi Lucas,
> >   Thanks for your suggestion. I have tried to implement runtime
> > autosuspend in fsl-edma driver on i.mx8qm/qxp with delay time (2 sec)
> > for this feature as below (or you can refer to
> > drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
> > pm_runtime_put_autosuspend in all dmaengine driver interface like
> > device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_cycli
> > c/
> > device_tx_status...
> >
> >
> > pm_runtime_use_autosuspend(fsl_chan->dev);
> > pm_runtime_set_autosuspend_delay(fsl_chan->dev,
> 2000);
> >
> > That could resolve this audio case since the autosuspend could suspend
> > runtime after
> > 2 seconds if there is no further dma transfer but only channel
> request(device_alloc_chan_resources).
> > But unfortunately, it cause another issue. As you know, on our
> > i.mx8qm/qxp, power domain done by scfw (drivers/firmware/imx/scu-pd.c)
> over mailbox:
> >  imx_sc_pd_power()->imx_scu_call_rpc()->
> > imx_scu_ipc_write()->mbox_send_message()
> > which means have to 'waits for completion', meanwhile, some driver
> > like tty will call dmaengine interfaces in non-atomic case as below,
> >
> > static int uart_write(struct tty_struct *tty, const unsigned char
> > *buf, int count) {
> >    ...
> > port = uart_port_lock(state, flags);
> >    ..
> > __uart_start(tty);  //call start_tx()->dmaengine_prep_slave_sg...
> > uart_port_unlock(port, flags);
> > return ret;
> > }
> >
> > Thus dma runtime resume may happen in that timing window and cause
> kernel alarm.
> > I'm not sure whether there are similar limitations on other driver
> > subsystem. But for me, It looks like the only way to resolve the
> > contradiction between tty and scu-pd (hardware limitation on
> > i.mx8qm/qxp) is to give up autosuspend and keep pm_runtime_get_sync
> only in device_alloc_chan_resources because request channel is a safe
> non-atomic phase.
> > Do you have any idea? Thanks in advance.
> 
> If you look closely at the driver you used as an example (hidma.c) it looks 
> like
> there is already something in there, which looks very much like what you need
> here:
> 
> In hidma_issue_pending() the driver tries to get the device to runtime resume.
> If this doesn't work, maybe due to the power domain code not being able to
> be called in atomic context, the actual work of waking up the dma hardware
> and issuing the descriptor is shunted to a tasklet.
> 
> If I'm reading this right, this is exactly what you need here to be able to 
> call the
> dmaengine code from atomic context: try the rpm get and issue immediately
> when possible, otherwise shunt the work to a non- atomic context where you
> can deal with the requirements of scu-pd.
Yes, I can schedule_work to worker to runtime resume edma channel by calling 
scu-pd.
But that means all dmaengine interfaces should b

Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-14 Thread Lucas Stach
Hi Robin,

Am Mittwoch, dem 14.04.2021 um 14:33 + schrieb Robin Gong:
> On 2020/05/20 17:43 Lucas Stach  wrote:
> > Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > > Hi
> > > 
> > > On Tue, May 19, 2020 at 6:04 PM Lucas Stach 
> > wrote:
> > > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > > There are two requirements that we need to move the request of dma
> > > > > channel from probe to open.
> > > > 
> > > > How do you handle -EPROBE_DEFER return code from the channel request
> > > > if you don't do it in probe?
> > > 
> > > I use the dma_request_slave_channel or dma_request_channel instead of
> > > dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER
> > > return code.
> > 
> > This is a pretty weak argument. The dmaengine device might probe after you
> > try to get the channel. Using a function to request the channel that doesn't
> > allow you to handle probe deferral is IMHO a bug and should be fixed, 
> > instead
> > of building even more assumptions on top of it.
> > 
> > > > > - When dma device binds with power-domains, the power will be
> > > > > enabled when we request dma channel. If the request of dma channel
> > > > > happen on probe, then the power-domains will be always enabled
> > > > > after kernel boot up,  which is not good for power saving,  so we
> > > > > need to move the request of dma channel to .open();
> > > > 
> > > > This is certainly something which could be fixed in the dmaengine
> > > > driver.
> > > 
> > > Dma driver always call the pm_runtime_get_sync in
> > > device_alloc_chan_resources, the device_alloc_chan_resources is called
> > > when channel is requested. so power is enabled on channel request.
> > 
> > So why can't you fix the dmaengine driver to do that RPM call at a later 
> > time
> > when the channel is actually going to be used? This will allow further power
> > savings with other slave devices than the audio PCM.
> Hi Lucas,
>   Thanks for your suggestion. I have tried to implement runtime autosuspend in
> fsl-edma driver on i.mx8qm/qxp with delay time (2 sec) for this feature as 
> below
> (or you can refer to drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
> pm_runtime_put_autosuspend in all dmaengine driver interface like
> device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_cyclic/
> device_tx_status...
> 
> 
> pm_runtime_use_autosuspend(fsl_chan->dev);
> pm_runtime_set_autosuspend_delay(fsl_chan->dev, 2000);
> 
> That could resolve this audio case since the autosuspend could suspend 
> runtime after
> 2 seconds if there is no further dma transfer but only channel 
> request(device_alloc_chan_resources).
> But unfortunately, it cause another issue. As you know, on our i.mx8qm/qxp, 
> power domain done by scfw (drivers/firmware/imx/scu-pd.c) over mailbox:
>  imx_sc_pd_power()->imx_scu_call_rpc()-> 
> imx_scu_ipc_write()->mbox_send_message()
> which means have to 'waits for completion', meanwhile, some driver like tty 
> will call dmaengine
> interfaces in non-atomic case as below, 
> 
> static int uart_write(struct tty_struct *tty, const unsigned char *buf, int 
> count)
> {
>    ...
>   port = uart_port_lock(state, flags);
>    ..
> __uart_start(tty);  //call start_tx()->dmaengine_prep_slave_sg...
> uart_port_unlock(port, flags);
> return ret;
> }
> 
> Thus dma runtime resume may happen in that timing window and cause kernel 
> alarm. 
> I'm not sure whether there are similar limitations on other driver subsystem. 
> But for me,
> It looks like the only way to resolve the contradiction between tty and 
> scu-pd (hardware
> limitation on i.mx8qm/qxp) is to give up autosuspend and keep 
> pm_runtime_get_sync
> only in device_alloc_chan_resources because request channel is a safe 
> non-atomic phase. 
> Do you have any idea? Thanks in advance. 

If you look closely at the driver you used as an example (hidma.c) it
looks like there is already something in there, which looks very much
like what you need here:

In hidma_issue_pending() the driver tries to get the device to runtime
resume. If this doesn't work, maybe due to the power domain code not
being able to be called in atomic context, the actual work of waking up
the dma hardware and issuing the descriptor is shunted to a tasklet.

If I'm reading this right, this is exactly what you need here to be
able to call the dmaengine code from atomic context: try the rpm get
and issue immediately when possible, otherwise shunt the work to a non-
atomic context where you can deal with the requirements of scu-pd.

Also you don't need the runtime resume in all of the functions you
mentioned. From a quick look into the edma driver it looks like for
example the prep_slave_dma() function only touches data structures in
memory, so you don't actually need the device to be awake at that
point. Only later in the flow when you write registers in the dma
hardware and actual

RE: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2021-04-14 Thread Robin Gong
On 2020/05/20 17:43 Lucas Stach  wrote:
> Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > Hi
> >
> > On Tue, May 19, 2020 at 6:04 PM Lucas Stach 
> wrote:
> > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > There are two requirements that we need to move the request of dma
> > > > channel from probe to open.
> > >
> > > How do you handle -EPROBE_DEFER return code from the channel request
> > > if you don't do it in probe?
> >
> > I use the dma_request_slave_channel or dma_request_channel instead of
> > dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER
> > return code.
> 
> This is a pretty weak argument. The dmaengine device might probe after you
> try to get the channel. Using a function to request the channel that doesn't
> allow you to handle probe deferral is IMHO a bug and should be fixed, instead
> of building even more assumptions on top of it.
> 
> > > > - When dma device binds with power-domains, the power will be
> > > > enabled when we request dma channel. If the request of dma channel
> > > > happen on probe, then the power-domains will be always enabled
> > > > after kernel boot up,  which is not good for power saving,  so we
> > > > need to move the request of dma channel to .open();
> > >
> > > This is certainly something which could be fixed in the dmaengine
> > > driver.
> >
> > Dma driver always call the pm_runtime_get_sync in
> > device_alloc_chan_resources, the device_alloc_chan_resources is called
> > when channel is requested. so power is enabled on channel request.
> 
> So why can't you fix the dmaengine driver to do that RPM call at a later time
> when the channel is actually going to be used? This will allow further power
> savings with other slave devices than the audio PCM.
Hi Lucas,
  Thanks for your suggestion. I have tried to implement runtime autosuspend in
fsl-edma driver on i.mx8qm/qxp with delay time (2 sec) for this feature as below
(or you can refer to drivers/dma/qcom/hidma.c), and pm_runtime_get_sync/
pm_runtime_put_autosuspend in all dmaengine driver interface like
device_alloc_chan_resources/device_prep_slave_sg/device_prep_dma_cyclic/
device_tx_status...


pm_runtime_use_autosuspend(fsl_chan->dev);
pm_runtime_set_autosuspend_delay(fsl_chan->dev, 2000);

That could resolve this audio case since the autosuspend could suspend runtime 
after
2 seconds if there is no further dma transfer but only channel 
request(device_alloc_chan_resources).
But unfortunately, it cause another issue. As you know, on our i.mx8qm/qxp, 
power domain done by scfw (drivers/firmware/imx/scu-pd.c) over mailbox:
 imx_sc_pd_power()->imx_scu_call_rpc()-> 
imx_scu_ipc_write()->mbox_send_message()
which means have to 'waits for completion', meanwhile, some driver like tty 
will call dmaengine
interfaces in non-atomic case as below, 

static int uart_write(struct tty_struct *tty, const unsigned char *buf, int 
count)
{
   ...
port = uart_port_lock(state, flags);
   ..
__uart_start(tty);  //call start_tx()->dmaengine_prep_slave_sg...
uart_port_unlock(port, flags);
return ret;
}

Thus dma runtime resume may happen in that timing window and cause kernel 
alarm. 
I'm not sure whether there are similar limitations on other driver subsystem. 
But for me,
It looks like the only way to resolve the contradiction between tty and scu-pd 
(hardware
limitation on i.mx8qm/qxp) is to give up autosuspend and keep 
pm_runtime_get_sync
only in device_alloc_chan_resources because request channel is a safe 
non-atomic phase. 
Do you have any idea? Thanks in advance. 
  

> 
> > > > - With FE-BE case, if the dma channel is requested in probe, then
> > > > there will be below issue, which is caused by that the dma channel
> > > > will be requested duplicately
> > >
> > > Why is this requested a second time? Is this just some missing
> > > cleanup on a deferred probe path?
> >
> > Not relate with deferred probe.  With DMA1->ASRC->DMA2->ESAI case, the
> > DMA1->ASRC->DMA2 is in FE,  ESAI is in BE.  When ESAI drvier probe,
> > DMA3 channel is created with ESAI's "dma:tx" (DMA3 channel
> > is not used in this FE-BE case).When FE-BE startup, DMA2
> > channel is created, it needs the ESAI's "dma:tx", so below warning
> > comes out.
> >
> > > Regards,
> > > Lucas
> > >
> > > > [  638.906268] sysfs: cannot create duplicate filename
> '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx'
> > > > [  638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted
> > > > 5.7.0-rc1-12956-gfc64b2585593 #287 [  638.927113] Hardware name:
> > > > Freescale i.MX6 Quad/DualLite (Device Tree) [  638.933690]
> > > > [] (unwind_backtrace) from []
> > > > (show_stack+0x10/0x14) [  638.941464] [] (show_stack)
> > > > from [] (dump_stack+0xe4/0x118) [  638.948808]
> > > > [] (dump_stack) from []
> > > > (sysfs_warn_dup+0x50/0x64) [  638.956406] []
> > > > (sysfs_warn_dup) from []
> > > > (sysfs_do_c

Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-22 Thread Mark Brown
On Thu, May 21, 2020 at 07:30:04PM +0800, Shengjiu Wang wrote:
> On Wed, May 20, 2020 at 8:38 PM Mark Brown  wrote:

> > Other drivers having problems means those drivers should be fixed, not
> > that we should copy the problems.  In the case of the PXA driver that's
> > very old code which predates deferred probe by I'd guess a decade.

> Thanks.

> For the FE-BE case, do you have any suggestion for how fix it?

> With DMA1->ASRC->DMA2->ESAI case, the DMA1->ASRC->DMA2
> is in FE,  ESAI is in BE.  When ESAI drvier probe,  DMA3 channel is
> created with ESAI's "dma:tx" (DMA3 channel
> is not used in this FE-BE case).When FE-BE startup, DMA2
> channel is created, it needs the ESAI's "dma:tx", so the warning
> comes out.

Not really TBH, this seems like another one of those csaes where DPCM is
creaking at the seams :/


signature.asc
Description: PGP signature


Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-21 Thread Shengjiu Wang
On Wed, May 20, 2020 at 8:38 PM Mark Brown  wrote:
>
> On Wed, May 20, 2020 at 07:22:19PM +0800, Shengjiu Wang wrote:
>
> > I see some driver also request dma channel in open() or hw_params().
> > how can they avoid the defer probe issue?
> > for example:
> > sound/arm/pxa2xx-pcm-lib.c
> > sound/soc/sprd/sprd-pcm-dma.c
>
> Other drivers having problems means those drivers should be fixed, not
> that we should copy the problems.  In the case of the PXA driver that's
> very old code which predates deferred probe by I'd guess a decade.

Thanks.

For the FE-BE case, do you have any suggestion for how fix it?

With DMA1->ASRC->DMA2->ESAI case, the DMA1->ASRC->DMA2
is in FE,  ESAI is in BE.  When ESAI drvier probe,  DMA3 channel is
created with ESAI's "dma:tx" (DMA3 channel
is not used in this FE-BE case).When FE-BE startup, DMA2
channel is created, it needs the ESAI's "dma:tx", so the warning
comes out.

best regards
wang shengjiu


Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-20 Thread Mark Brown
On Wed, May 20, 2020 at 07:22:19PM +0800, Shengjiu Wang wrote:

> I see some driver also request dma channel in open() or hw_params().
> how can they avoid the defer probe issue?
> for example:
> sound/arm/pxa2xx-pcm-lib.c
> sound/soc/sprd/sprd-pcm-dma.c

Other drivers having problems means those drivers should be fixed, not
that we should copy the problems.  In the case of the PXA driver that's
very old code which predates deferred probe by I'd guess a decade.


signature.asc
Description: PGP signature


Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-20 Thread Shengjiu Wang
Hi

On Wed, May 20, 2020 at 5:42 PM Lucas Stach  wrote:
>
> Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> > Hi
> >
> > On Tue, May 19, 2020 at 6:04 PM Lucas Stach  wrote:
> > > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > > There are two requirements that we need to move the request
> > > > of dma channel from probe to open.
> > >
> > > How do you handle -EPROBE_DEFER return code from the channel request if
> > > you don't do it in probe?
> >
> > I use the dma_request_slave_channel or dma_request_channel instead
> > of dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER
> > return code.
>
> This is a pretty weak argument. The dmaengine device might probe after
> you try to get the channel. Using a function to request the channel
> that doesn't allow you to handle probe deferral is IMHO a bug and
> should be fixed, instead of building even more assumptions on top of
> it.
>

I see some driver also request dma channel in open() or hw_params().
how can they avoid the defer probe issue?
for example:
sound/arm/pxa2xx-pcm-lib.c
sound/soc/sprd/sprd-pcm-dma.c

> > > > - When dma device binds with power-domains, the power will
> > > > be enabled when we request dma channel. If the request of dma
> > > > channel happen on probe, then the power-domains will be always
> > > > enabled after kernel boot up,  which is not good for power
> > > > saving,  so we need to move the request of dma channel to .open();
> > >
> > > This is certainly something which could be fixed in the dmaengine
> > > driver.
> >
> > Dma driver always call the pm_runtime_get_sync in
> > device_alloc_chan_resources, the device_alloc_chan_resources is
> > called when channel is requested. so power is enabled on channel
> > request.
>
> So why can't you fix the dmaengine driver to do that RPM call at a
> later time when the channel is actually going to be used? This will
> allow further power savings with other slave devices than the audio
> PCM.
>
> Regards,
> Lucas
>

It seems the best place for calling pm_runtime_get_sync is the
device_alloc_chan_resources, and calling pm_runtime_put_sync
in the .device_free_chan_resources

For the slave_sg mode, the .device_prep_slave_sg and
.device_issue_pending  will be called many times after
.device_alloc_chan_resources. so it is not good to call
pm_runtime_get_sync in .device_prep_slave_sg or
.device_issue_pending

best regards
wang shengjiu


Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-20 Thread Lucas Stach
Am Mittwoch, den 20.05.2020, 16:20 +0800 schrieb Shengjiu Wang:
> Hi
> 
> On Tue, May 19, 2020 at 6:04 PM Lucas Stach  wrote:
> > Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > > There are two requirements that we need to move the request
> > > of dma channel from probe to open.
> > 
> > How do you handle -EPROBE_DEFER return code from the channel request if
> > you don't do it in probe?
> 
> I use the dma_request_slave_channel or dma_request_channel instead
> of dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER
> return code.

This is a pretty weak argument. The dmaengine device might probe after
you try to get the channel. Using a function to request the channel
that doesn't allow you to handle probe deferral is IMHO a bug and
should be fixed, instead of building even more assumptions on top of
it.

> > > - When dma device binds with power-domains, the power will
> > > be enabled when we request dma channel. If the request of dma
> > > channel happen on probe, then the power-domains will be always
> > > enabled after kernel boot up,  which is not good for power
> > > saving,  so we need to move the request of dma channel to .open();
> > 
> > This is certainly something which could be fixed in the dmaengine
> > driver.
> 
> Dma driver always call the pm_runtime_get_sync in
> device_alloc_chan_resources, the device_alloc_chan_resources is
> called when channel is requested. so power is enabled on channel
> request.

So why can't you fix the dmaengine driver to do that RPM call at a
later time when the channel is actually going to be used? This will
allow further power savings with other slave devices than the audio
PCM.

Regards,
Lucas

> > > - With FE-BE case, if the dma channel is requested in probe,
> > > then there will be below issue, which is caused by that the
> > > dma channel will be requested duplicately
> > 
> > Why is this requested a second time? Is this just some missing cleanup
> > on a deferred probe path?
> 
> Not relate with deferred probe.  With DMA1->ASRC->DMA2->ESAI case,
> the DMA1->ASRC->DMA2 is in FE,  ESAI is in BE.  When ESAI drvier
> probe,  DMA3 channel is created with ESAI's "dma:tx" (DMA3 channel
> is not used in this FE-BE case).When FE-BE startup, DMA2
> channel is created, it needs the ESAI's "dma:tx", so below warning
> comes out.
> 
> > Regards,
> > Lucas
> > 
> > > [  638.906268] sysfs: cannot create duplicate filename 
> > > '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx'
> > > [  638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted 
> > > 5.7.0-rc1-12956-gfc64b2585593 #287
> > > [  638.927113] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > > [  638.933690] [] (unwind_backtrace) from [] 
> > > (show_stack+0x10/0x14)
> > > [  638.941464] [] (show_stack) from [] 
> > > (dump_stack+0xe4/0x118)
> > > [  638.948808] [] (dump_stack) from [] 
> > > (sysfs_warn_dup+0x50/0x64)
> > > [  638.956406] [] (sysfs_warn_dup) from [] 
> > > (sysfs_do_create_link_sd+0xc8/0xd4)
> > > [  638.965134] [] (sysfs_do_create_link_sd) from [] 
> > > (dma_request_chan+0xb0/0x210)
> > > [  638.974120] [] (dma_request_chan) from [] 
> > > (dma_request_slave_channel+0x8/0x14)
> > > [  638.983111] [] (dma_request_slave_channel) from [] 
> > > (fsl_asrc_dma_hw_params+0x1e0/0x438)
> > > [  638.992881] [] (fsl_asrc_dma_hw_params) from [] 
> > > (soc_pcm_hw_params+0x4a0/0x6a8)
> > > [  639.001952] [] (soc_pcm_hw_params) from [] 
> > > (dpcm_fe_dai_hw_params+0x70/0xe4)
> > > [  639.010765] [] (dpcm_fe_dai_hw_params) from [] 
> > > (snd_pcm_hw_params+0x158/0x418)
> > > [  639.019750] [] (snd_pcm_hw_params) from [] 
> > > (snd_pcm_ioctl+0x734/0x183c)
> > > [  639.028129] [] (snd_pcm_ioctl) from [] 
> > > (ksys_ioctl+0x2ac/0xb98)
> > > [  639.035812] [] (ksys_ioctl) from [] 
> > > (ret_fast_syscall+0x0/0x28)
> > > [  639.043490] Exception stack(0xec529fa8 to 0xec529ff0)
> > > [  639.048565] 9fa0:   bee84650 01321870 0004 
> > > c25c4111 bee84650 0002000f
> > > [  639.056766] 9fc0: bee84650 01321870 01321820 0036 1f40 
> > >  0002c2f8 0003
> > > [  639.064964] 9fe0: b6f483fc bee8451c b6ee2655 b6e1dcf8
> > > [  639.070339] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink
> > > 
> > > Signed-off-by: Shengjiu Wang 
> > > ---
> > >  sound/soc/fsl/imx-pcm-dma.c | 173 +---
> > >  1 file changed, 159 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
> > > index 04a9bc749016..dae53b384df4 100644
> > > --- a/sound/soc/fsl/imx-pcm-dma.c
> > > +++ b/sound/soc/fsl/imx-pcm-dma.c
> > > @@ -11,6 +11,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > 
> > >  #include 
> > >  #include 
> > > @@ -29,24 +30,168 @@ static bool filter(struct dma_chan *chan, void 
> > > *param)
> > >   return true;
> > >  }
> > > 
> > > -static const struct snd_dmaengine_pcm_config imx_dmaengine

Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-20 Thread Shengjiu Wang
Hi

On Tue, May 19, 2020 at 6:04 PM Lucas Stach  wrote:
>
> Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> > There are two requirements that we need to move the request
> > of dma channel from probe to open.
>
> How do you handle -EPROBE_DEFER return code from the channel request if
> you don't do it in probe?

I use the dma_request_slave_channel or dma_request_channel instead
of dmaengine_pcm_request_chan_of. so there should be not -EPROBE_DEFER
return code.

>
> > - When dma device binds with power-domains, the power will
> > be enabled when we request dma channel. If the request of dma
> > channel happen on probe, then the power-domains will be always
> > enabled after kernel boot up,  which is not good for power
> > saving,  so we need to move the request of dma channel to .open();
>
> This is certainly something which could be fixed in the dmaengine
> driver.

Dma driver always call the pm_runtime_get_sync in
device_alloc_chan_resources, the device_alloc_chan_resources is
called when channel is requested. so power is enabled on channel
request.

>
> > - With FE-BE case, if the dma channel is requested in probe,
> > then there will be below issue, which is caused by that the
> > dma channel will be requested duplicately
>
> Why is this requested a second time? Is this just some missing cleanup
> on a deferred probe path?

Not relate with deferred probe.  With DMA1->ASRC->DMA2->ESAI case,
the DMA1->ASRC->DMA2 is in FE,  ESAI is in BE.  When ESAI drvier
probe,  DMA3 channel is created with ESAI's "dma:tx" (DMA3 channel
is not used in this FE-BE case).When FE-BE startup, DMA2
channel is created, it needs the ESAI's "dma:tx", so below warning
comes out.

>
> Regards,
> Lucas
>
> > [  638.906268] sysfs: cannot create duplicate filename 
> > '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx'
> > [  638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted 
> > 5.7.0-rc1-12956-gfc64b2585593 #287
> > [  638.927113] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > [  638.933690] [] (unwind_backtrace) from [] 
> > (show_stack+0x10/0x14)
> > [  638.941464] [] (show_stack) from [] 
> > (dump_stack+0xe4/0x118)
> > [  638.948808] [] (dump_stack) from [] 
> > (sysfs_warn_dup+0x50/0x64)
> > [  638.956406] [] (sysfs_warn_dup) from [] 
> > (sysfs_do_create_link_sd+0xc8/0xd4)
> > [  638.965134] [] (sysfs_do_create_link_sd) from [] 
> > (dma_request_chan+0xb0/0x210)
> > [  638.974120] [] (dma_request_chan) from [] 
> > (dma_request_slave_channel+0x8/0x14)
> > [  638.983111] [] (dma_request_slave_channel) from [] 
> > (fsl_asrc_dma_hw_params+0x1e0/0x438)
> > [  638.992881] [] (fsl_asrc_dma_hw_params) from [] 
> > (soc_pcm_hw_params+0x4a0/0x6a8)
> > [  639.001952] [] (soc_pcm_hw_params) from [] 
> > (dpcm_fe_dai_hw_params+0x70/0xe4)
> > [  639.010765] [] (dpcm_fe_dai_hw_params) from [] 
> > (snd_pcm_hw_params+0x158/0x418)
> > [  639.019750] [] (snd_pcm_hw_params) from [] 
> > (snd_pcm_ioctl+0x734/0x183c)
> > [  639.028129] [] (snd_pcm_ioctl) from [] 
> > (ksys_ioctl+0x2ac/0xb98)
> > [  639.035812] [] (ksys_ioctl) from [] 
> > (ret_fast_syscall+0x0/0x28)
> > [  639.043490] Exception stack(0xec529fa8 to 0xec529ff0)
> > [  639.048565] 9fa0:   bee84650 01321870 0004 c25c4111 
> > bee84650 0002000f
> > [  639.056766] 9fc0: bee84650 01321870 01321820 0036 1f40  
> > 0002c2f8 0003
> > [  639.064964] 9fe0: b6f483fc bee8451c b6ee2655 b6e1dcf8
> > [  639.070339] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink
> >
> > Signed-off-by: Shengjiu Wang 
> > ---
> >  sound/soc/fsl/imx-pcm-dma.c | 173 +---
> >  1 file changed, 159 insertions(+), 14 deletions(-)
> >
> > diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
> > index 04a9bc749016..dae53b384df4 100644
> > --- a/sound/soc/fsl/imx-pcm-dma.c
> > +++ b/sound/soc/fsl/imx-pcm-dma.c
> > @@ -11,6 +11,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >  #include 
> > @@ -29,24 +30,168 @@ static bool filter(struct dma_chan *chan, void *param)
> >   return true;
> >  }
> >
> > -static const struct snd_dmaengine_pcm_config imx_dmaengine_pcm_config = {
> > - .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> > - .compat_filter_fn = filter,
> > -};
> > +static int imx_pcm_hw_params(struct snd_soc_component *component,
> > +  struct snd_pcm_substream *substream,
> > +  struct snd_pcm_hw_params *params)
> > +{
> > + struct snd_pcm_runtime *runtime = substream->runtime;
> > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> > + struct snd_dmaengine_dai_dma_data *dma_data;
> > + struct dma_slave_config config;
> > + struct dma_chan *chan;
> > + int ret = 0;
> >
> > -int imx_pcm_dma_init(struct platform_device *pdev, size_t size)
> > + sn

Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe

2020-05-19 Thread Lucas Stach
Am Dienstag, den 19.05.2020, 17:41 +0800 schrieb Shengjiu Wang:
> There are two requirements that we need to move the request
> of dma channel from probe to open.

How do you handle -EPROBE_DEFER return code from the channel request if
you don't do it in probe?

> - When dma device binds with power-domains, the power will
> be enabled when we request dma channel. If the request of dma
> channel happen on probe, then the power-domains will be always
> enabled after kernel boot up,  which is not good for power
> saving,  so we need to move the request of dma channel to .open();

This is certainly something which could be fixed in the dmaengine
driver.

> - With FE-BE case, if the dma channel is requested in probe,
> then there will be below issue, which is caused by that the
> dma channel will be requested duplicately

Why is this requested a second time? Is this just some missing cleanup
on a deferred probe path?

Regards,
Lucas

> [  638.906268] sysfs: cannot create duplicate filename 
> '/devices/soc0/soc/200.bus/200.spba-bus/2024000.esai/dma:tx'
> [  638.919061] CPU: 1 PID: 673 Comm: aplay Not tainted 
> 5.7.0-rc1-12956-gfc64b2585593 #287
> [  638.927113] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [  638.933690] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [  638.941464] [] (show_stack) from [] 
> (dump_stack+0xe4/0x118)
> [  638.948808] [] (dump_stack) from [] 
> (sysfs_warn_dup+0x50/0x64)
> [  638.956406] [] (sysfs_warn_dup) from [] 
> (sysfs_do_create_link_sd+0xc8/0xd4)
> [  638.965134] [] (sysfs_do_create_link_sd) from [] 
> (dma_request_chan+0xb0/0x210)
> [  638.974120] [] (dma_request_chan) from [] 
> (dma_request_slave_channel+0x8/0x14)
> [  638.983111] [] (dma_request_slave_channel) from [] 
> (fsl_asrc_dma_hw_params+0x1e0/0x438)
> [  638.992881] [] (fsl_asrc_dma_hw_params) from [] 
> (soc_pcm_hw_params+0x4a0/0x6a8)
> [  639.001952] [] (soc_pcm_hw_params) from [] 
> (dpcm_fe_dai_hw_params+0x70/0xe4)
> [  639.010765] [] (dpcm_fe_dai_hw_params) from [] 
> (snd_pcm_hw_params+0x158/0x418)
> [  639.019750] [] (snd_pcm_hw_params) from [] 
> (snd_pcm_ioctl+0x734/0x183c)
> [  639.028129] [] (snd_pcm_ioctl) from [] 
> (ksys_ioctl+0x2ac/0xb98)
> [  639.035812] [] (ksys_ioctl) from [] 
> (ret_fast_syscall+0x0/0x28)
> [  639.043490] Exception stack(0xec529fa8 to 0xec529ff0)
> [  639.048565] 9fa0:   bee84650 01321870 0004 c25c4111 
> bee84650 0002000f
> [  639.056766] 9fc0: bee84650 01321870 01321820 0036 1f40  
> 0002c2f8 0003
> [  639.064964] 9fe0: b6f483fc bee8451c b6ee2655 b6e1dcf8
> [  639.070339] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  sound/soc/fsl/imx-pcm-dma.c | 173 +---
>  1 file changed, 159 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
> index 04a9bc749016..dae53b384df4 100644
> --- a/sound/soc/fsl/imx-pcm-dma.c
> +++ b/sound/soc/fsl/imx-pcm-dma.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -29,24 +30,168 @@ static bool filter(struct dma_chan *chan, void *param)
>   return true;
>  }
>  
> -static const struct snd_dmaengine_pcm_config imx_dmaengine_pcm_config = {
> - .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> - .compat_filter_fn = filter,
> -};
> +static int imx_pcm_hw_params(struct snd_soc_component *component,
> +  struct snd_pcm_substream *substream,
> +  struct snd_pcm_hw_params *params)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> + struct snd_dmaengine_dai_dma_data *dma_data;
> + struct dma_slave_config config;
> + struct dma_chan *chan;
> + int ret = 0;
>  
> -int imx_pcm_dma_init(struct platform_device *pdev, size_t size)
> + snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
> + runtime->dma_bytes = params_buffer_bytes(params);
> +
> + chan = snd_dmaengine_pcm_get_chan(substream);
> + if (!chan)
> + return -EINVAL;
> +
> + ret = snd_hwparams_to_dma_slave_config(substream, params, &config);
> + if (ret)
> + return ret;
> +
> + dma_data = snd_soc_dai_get_dma_data(cpu_dai, substream);
> + if (!dma_data)
> + return -EINVAL;
> +
> + snd_dmaengine_pcm_set_config_from_dai_data(substream,
> +dma_data,
> +&config);
> + return dmaengine_slave_config(chan, &config);
> +}
> +
> +static int imx_pcm_hw_free(struct snd_soc_component *component,
> +struct snd_pcm_substream *substream)
>  {
> - struct snd_dmaengine_pcm_config *config;
> + snd_pcm_set_r