Re: of_dma_request_slave_channel() failed ?
Hi Morimoto-san,
On Fri, Sep 14, 2018 at 2:53 AM Kuninori Morimoto
wrote:
> > > Before 6c92d5a2744e2761 patch, driver will forcibly ignore
> > > non-SSI modules, and try to use PIO mode.
> > > I'm not sure it is "kindly support" or "overkill support".
> > >
> > > After this patch, it needs DMA, otherwise, probe will be failed.
> > > DT shouldn't have non-SSI modules if you want to use PIO mode.
> > >
> > > + /* use PIO mode */
> > > - playback = < >;
> > > + playback = <>;
> >
> > OK, so falling back to PIO was really a "best effort" fallback, and the user
> > should really enable DMA?
>
> Yeah, I'm thinking this is better.
> PIO fallback is of course "nice to have" if possible.
> But, because of this new iommu patch, keeping this feature
> needs "big complicated patch", and we can get "small effect" I think.
> Thus, I think this is the time to remove this feature.
> Can you agree ?
You're the rcar-sound expert ;-)
If you think there's not much value in keeping PIO fallback, it can be
removed.
We can change behavior once these dependency issues have been resolved
in a generic way.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Geert > > ...which is in fact the exact same problem that the IOMMU code has - > > might it make sense to give of_dma_request_slave_channel() similar > > (optional?) driver_deferred_probe_check_state() logic, i.e. "if my DMAC > > driver's not shown up by this point, assume it's not built-in and go on > > without it". Of course it is somewhat easier for IOMMU drivers as > > there's zero chance of those popping up as modules later on. > > It may solve the issue in some cases. But only if the dmac is reprobed > before the DMA slave driver, which is not guaranteed. > > BTW, it seems e.g. i2c and serial suffer from the same problem, and fall > back to PIO. However, these drivers try to obtain the DMA channel when > used, not during probe, so they start using DMA after the dmac has been > probed successfully. Actually sound driver get DMA channel when used. But checking DMA availability when probe timing (= try to get, and release it soon). sound driver side other approach is that it don't check it when probe. Best regards --- Kuninori Morimoto ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Geert > > Before 6c92d5a2744e2761 patch, driver will forcibly ignore > > non-SSI modules, and try to use PIO mode. > > I'm not sure it is "kindly support" or "overkill support". > > > > After this patch, it needs DMA, otherwise, probe will be failed. > > DT shouldn't have non-SSI modules if you want to use PIO mode. > > > > + /* use PIO mode */ > > - playback = < >; > > + playback = <>; > > > > OK, so falling back to PIO was really a "best effort" fallback, and the user > should really enable DMA? Yeah, I'm thinking this is better. PIO fallback is of course "nice to have" if possible. But, because of this new iommu patch, keeping this feature needs "big complicated patch", and we can get "small effect" I think. Thus, I think this is the time to remove this feature. Can you agree ? Best regards --- Kuninori Morimoto ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Robin,
On Thu, Sep 13, 2018 at 12:12 PM Robin Murphy wrote:
> On 13/09/18 10:00, Geert Uytterhoeven wrote:
> [...]
> > The main issue is that if of_dma_find_controller() fails, a DMA slave driver
> > cannot distinguish between dmac not yet probed successfully, and dmac
> > driver not present.
>
> ...which is in fact the exact same problem that the IOMMU code has -
> might it make sense to give of_dma_request_slave_channel() similar
> (optional?) driver_deferred_probe_check_state() logic, i.e. "if my DMAC
> driver's not shown up by this point, assume it's not built-in and go on
> without it". Of course it is somewhat easier for IOMMU drivers as
> there's zero chance of those popping up as modules later on.
It may solve the issue in some cases. But only if the dmac is reprobed
before the DMA slave driver, which is not guaranteed.
BTW, it seems e.g. i2c and serial suffer from the same problem, and fall
back to PIO. However, these drivers try to obtain the DMA channel when
used, not during probe, so they start using DMA after the dmac has been
probed successfully.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
On 13/09/18 10:00, Geert Uytterhoeven wrote: [...] The main issue is that if of_dma_find_controller() fails, a DMA slave driver cannot distinguish between dmac not yet probed successfully, and dmac driver not present. ...which is in fact the exact same problem that the IOMMU code has - might it make sense to give of_dma_request_slave_channel() similar (optional?) driver_deferred_probe_check_state() logic, i.e. "if my DMAC driver's not shown up by this point, assume it's not built-in and go on without it". Of course it is somewhat easier for IOMMU drivers as there's zero chance of those popping up as modules later on. However, as we rely more and more on probe deferral, this will cause more issues in the future. I guess the proper solution is to take into account dependencies described in DT before probing devices, i.e. devices with phandles should always be probed after the targets of these phandles? Complication: - Circular dependencies, - Optional phandle targets (dmacs, oops). Yeah, a proper dependency graph would be ideal, but it's certainly not trivial. There was an interesting proposal a couple of years ago of a half-way approach where various phandle-chasing routines could actively try to probe the target, but I think that got stymied on the fact that a single OF node may represent multiple different driver-level providers, so it's tricky to tell whether the *right* dependency has been satisfied without detailed knowledge of the individual bindings and driver implementations. That's probably still an issue for other approaches too, sadly. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Mark,
On Wed, Sep 12, 2018 at 5:51 PM Mark Brown wrote:
> On Tue, Sep 11, 2018 at 11:43:47AM +0200, Geert Uytterhoeven wrote:
> > So it seems the audio DMAC is deferred a second time, before the iommu
> > driver
> > probed.
>
> Shouldn't there be at least one more round of deferred probe triggers
> after the IOMMU probes?
My statement above was incorrect. Adding more debug info allowed to gain
more insight:
A. If CONFIG_IPMMU_VMSA=y, everything is fine:
1. ipmmu probes,
2. audio-dmac probes,
3. audio probes, using DMA
B. If CONFIG_IPMMU_VMSA=n, the sound driver falls back to PIO, which
Morimoto-san worked around by commit 6c92d5a2744e2761 ("ASoC: rsnd:
don't fallback to PIO mode when -EPROBE_DEFER")[1]
1. ipmmu is not probed: driver not enabled => OK
2. audio-dmac is not probed: iommu not found
3. audio fails to probe (3 times, hence just ignoring the first one is
not sufficient):
"of_dma_find_controller: can't find DMA controller
/soc/dma-controller@ec70"
=> fell back to PIO before [1]
=> -EPROBE_DEFER since [1]
4. audio-dmac probes
of_iommu_xlate() calls driver_deferred_probe_check_state(), leading
to ("ignoring dependency for device, assuming no driver")
If step 3 returned -EPROBE_DEFER, there's one additional step:
5. audio reprobes successfully, using DMA.
While step 2 (DMA-capable device not probed because iommu not found)
is correct for devices with a builtin dmac, it causes issues with external
dmacs, as the dmac probe may be postponed after the DMA slave driver
probe.
C. Before commit ac6bbf0cdf4206c5 ("iommu: Remove IOMMU_OF_DECLARE"),
with CONFIG_IPMMU_VMSA=n:
1. ipmmu is not probed: driver not enabled => OK
2. audio-dmac probes => OK
Missing IOMMU ignored, as per
!ops && !of_iommu_driver_present(iommu_spec->np)
in of_iommu_xlate(), which is really
!ops && !of_match_node(&__iommu_of_table, np))
3. audio probes, using DMA
Note: if of_dma_find_controller() would have failed (e.g. missing
dmac driver), the audio driver fell back to PIO here.
As per Morimoto-san's recent reply, audio really needs DMA, so the current
behavior of B.5. is OK. However, this may not be true for other devices
(e.g. SPI, i2c, serial, ...), where the dmac is optional.
The main issue is that if of_dma_find_controller() fails, a DMA slave driver
cannot distinguish between dmac not yet probed successfully, and dmac
driver not present.
However, as we rely more and more on probe deferral, this will cause more
issues in the future.
I guess the proper solution is to take into account dependencies described
in DT before probing devices, i.e. devices with phandles should always be
probed after the targets of these phandles?
Complication:
- Circular dependencies,
- Optional phandle targets (dmacs, oops).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Morimoto-san,
On Thu, Sep 13, 2018 at 7:48 AM Kuninori Morimoto
wrote:
> > > -
> > > commit ac6bbf0cdf4206c517ac9789814c23e372ebce4d
> > > Author: Rob Herring
> > > Date: Mon Jul 9 09:41:52 2018 -0600
> > >
> > > iommu: Remove IOMMU_OF_DECLARE
> > >
> > > Now that we use the driver core to stop deferred probe for missing
> > > drivers, IOMMU_OF_DECLARE can be removed.
> > >
> > > This is slightly less optimal than having a list of built-in drivers
> > > in
> > > that we'll now defer probe twice before giving up. This shouldn't
> > > have a
> > > significant impact on boot times as past discussions about deferred
> > > probe have given no evidence of deferred probe having a substantial
> > > impact.
> > > ...
> > > -
> (snip)
> > I assume you wrote commit 6c92d5a2744e2761 ("ASoC: rsnd: don't fallback
> > to PIO mode when -EPROBE_DEFER") to work around this?
>
> Yes, it is work around for it.
>
> > While this got rid of the error messages, and postpones sound initialization
> > until the audio DMAC is available, it does mean that the driver will _never_
> > fall back to PIO.
> >
> > I.e. if CONFIG_RCAR_DMAC=n, audio will fail to probe instead of falling
> > back to PIO.
>
> If I focus only for sound here, the purpose of PIO mode is
> checking basic HW connection, clock settings, etc.
> Without DMAC support, it can't use many sound features.
> And PIO mode is supporting "SSI" only.
>
> If DT has SRC/CTU/DVC settings for sound,
> it needs DMA support anyway.
>
> _sound {
> ...
> ports {
> rsnd_port0: port@0 {
> rsnd_endpoint0: endpoint {
> ...
> => playback = < >;
> };
> };
> };
> };
>
> Before 6c92d5a2744e2761 patch, driver will forcibly ignore
> non-SSI modules, and try to use PIO mode.
> I'm not sure it is "kindly support" or "overkill support".
>
> After this patch, it needs DMA, otherwise, probe will be failed.
> DT shouldn't have non-SSI modules if you want to use PIO mode.
>
> + /* use PIO mode */
> - playback = < >;
> + playback = <>;
>
OK, so falling back to PIO was really a "best effort" fallback, and the user
should really enable DMA?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Geert
Thank you for your feedback
> > -
> > commit ac6bbf0cdf4206c517ac9789814c23e372ebce4d
> > Author: Rob Herring
> > Date: Mon Jul 9 09:41:52 2018 -0600
> >
> > iommu: Remove IOMMU_OF_DECLARE
> >
> > Now that we use the driver core to stop deferred probe for missing
> > drivers, IOMMU_OF_DECLARE can be removed.
> >
> > This is slightly less optimal than having a list of built-in drivers in
> > that we'll now defer probe twice before giving up. This shouldn't have a
> > significant impact on boot times as past discussions about deferred
> > probe have given no evidence of deferred probe having a substantial
> > impact.
> > ...
> > -
(snip)
> I assume you wrote commit 6c92d5a2744e2761 ("ASoC: rsnd: don't fallback
> to PIO mode when -EPROBE_DEFER") to work around this?
Yes, it is work around for it.
> While this got rid of the error messages, and postpones sound initialization
> until the audio DMAC is available, it does mean that the driver will _never_
> fall back to PIO.
>
> I.e. if CONFIG_RCAR_DMAC=n, audio will fail to probe instead of falling
> back to PIO.
If I focus only for sound here, the purpose of PIO mode is
checking basic HW connection, clock settings, etc.
Without DMAC support, it can't use many sound features.
And PIO mode is supporting "SSI" only.
If DT has SRC/CTU/DVC settings for sound,
it needs DMA support anyway.
_sound {
...
ports {
rsnd_port0: port@0 {
rsnd_endpoint0: endpoint {
...
=> playback = < >;
};
};
};
};
Before 6c92d5a2744e2761 patch, driver will forcibly ignore
non-SSI modules, and try to use PIO mode.
I'm not sure it is "kindly support" or "overkill support".
After this patch, it needs DMA, otherwise, probe will be failed.
DT shouldn't have non-SSI modules if you want to use PIO mode.
+ /* use PIO mode */
- playback = < >;
+ playback = <>;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
On Tue, Sep 11, 2018 at 11:43:47AM +0200, Geert Uytterhoeven wrote: > So it seems the audio DMAC is deferred a second time, before the iommu driver > probed. Shouldn't there be at least one more round of deferred probe triggers after the IOMMU probes? signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: of_dma_request_slave_channel() failed ?
Hi Morimoto-san,
CC asoc, dma, iommu
On Tue, Sep 4, 2018 at 8:06 AM Kuninori Morimoto
wrote:
> Hi Renesas ML
> Cc Rob
>
> I noticed that Sound driver is using PIO mode
> on latest kernel.
>
> ...
> rcar_sound ec50.sound: src[0] : probe error -11
> rcar_sound ec50.sound: ssi[0] fallback to PIO mode
> rcar_sound ec50.sound: src[1] : probe error -11
> rcar_sound ec50.sound: ssi[1] fallback to PIO mode
> rcar_sound ec50.sound: ssi[2] : probe error -11
> rcar_sound ec50.sound: ssi[2] fallback to PIO mode
> rcar_sound ec50.sound: ssi[3] : probe error -11
> rcar_sound ec50.sound: ssi[3] fallback to PIO mode
> rcar_sound ec50.sound: probed
> ...
>
> Sound can use DMA mode and PIO mode.
> It automatically fallbacks to PIO mode if it can't use DMA.
>
> I bisected this issue, and found the 1st bad commit
>
> -
> commit ac6bbf0cdf4206c517ac9789814c23e372ebce4d
> Author: Rob Herring
> Date: Mon Jul 9 09:41:52 2018 -0600
>
> iommu: Remove IOMMU_OF_DECLARE
>
> Now that we use the driver core to stop deferred probe for missing
> drivers, IOMMU_OF_DECLARE can be removed.
>
> This is slightly less optimal than having a list of built-in drivers in
> that we'll now defer probe twice before giving up. This shouldn't have a
> significant impact on boot times as past discussions about deferred
> probe have given no evidence of deferred probe having a substantial
> impact.
> ...
> -
>
> In more detail, it seems it can't find DMA controller,
> and of_dma_request_slave_channel() returns
> -EPROBE_DEFER from this commit.
>
> struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> const char *name)
> {
> ...
> ofdma = of_dma_find_controller(_spec);
>
> if (ofdma) {
> chan = ofdma->of_dma_xlate(_spec, ofdma);
> } else {
> => ret_no_channel = -EPROBE_DEFER;
> chan = NULL;
> }
> ...
> => return ERR_PTR(ret_no_channel);
> }
>
> Is this known issue ?
> Or sound shouldn't fallbacks to PIO mode ?
I assume you wrote commit 6c92d5a2744e2761 ("ASoC: rsnd: don't fallback
to PIO mode when -EPROBE_DEFER") to work around this?
While this got rid of the error messages, and postpones sound initialization
until the audio DMAC is available, it does mean that the driver will _never_
fall back to PIO.
I.e. if CONFIG_RCAR_DMAC=n, audio will fail to probe instead of falling
back to PIO.
With CONFIG_RCAR_DMAC=y:
rcar-dmac e670.dma-controller: ignoring dependency for device,
assuming no driver
rcar-dmac e730.dma-controller: ignoring dependency for device,
assuming no driver
rcar-dmac e731.dma-controller: ignoring dependency for device,
assuming no driver
rcar-dmac ec70.dma-controller: ignoring dependency for device,
assuming no driver
rcar-dmac ec72.dma-controller: ignoring dependency for device,
assuming no driver
So it seems the audio DMAC is deferred a second time, before the iommu driver
probed.
subsys_initcall(ipmmu_init); calls platform_driver_register
module_platform_driver(rcar_dmac_driver);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
