| Message ID | 1497300766-23675-1-git-send-email-john.stultz@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote: > + sound { > + compatible = "simple-audio-card"; > + simple-audio-card,name = "hikey-hdmi"; Now the graph card has been merged it's probably a good idea to be using that, it's generally a better idea for pretty much all use cases.
On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote: > On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote: > >> + sound { >> + compatible = "simple-audio-card"; >> + simple-audio-card,name = "hikey-hdmi"; > > Now the graph card has been merged it's probably a good idea to be using > that, it's generally a better idea for pretty much all use cases. I've taken a shot today trying to convert over to the audio-graph-card method (which isn't yet upstream, but in -next), but I've been running into some quirks. Part of the issue is the adv7533 bridge already has a endpoint port entry for the dsi output. So when I add the codec_endpoint port, it wants the two port entries to be enumerated, so I get something like: port@0 { adv7533_in: endpoint { remote-endpoint = <&dsi_out0>; }; }; port@1 { codec_endpoint: endpoint { remote-endpoint = <&i2s0_cpu_endpoint>; }; }; But this causes it to try to link to hdmi-hifi.1 (which doesn't exist) instead of hdmi-hifi.0. If I instead swap the entries, so the codec_endpoint is first on port 0, then the the audio link is properly setup, but the dsi initialization falls over. Any suggestions here? thanks -john
Hi John > On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote: > > > >> + sound { > >> + compatible = "simple-audio-card"; > >> + simple-audio-card,name = "hikey-hdmi"; > > > > Now the graph card has been merged it's probably a good idea to be using > > that, it's generally a better idea for pretty much all use cases. > > I've taken a shot today trying to convert over to the audio-graph-card > method (which isn't yet upstream, but in -next), but I've been running > into some quirks. > > Part of the issue is the adv7533 bridge already has a endpoint port > entry for the dsi output. So when I add the codec_endpoint port, it > wants the two port entries to be enumerated, so I get something like: > > port@0 { > adv7533_in: endpoint { > remote-endpoint = <&dsi_out0>; > }; > }; > port@1 { > codec_endpoint: endpoint { > remote-endpoint = <&i2s0_cpu_endpoint>; > }; > }; > > But this causes it to try to link to hdmi-hifi.1 (which doesn't exist) > instead of hdmi-hifi.0. > > If I instead swap the entries, so the codec_endpoint is first on port > 0, then the the audio link is properly setup, but the dsi > initialization falls over. I think you want to exchange port@1 as ID=0 for ALSA SoC ? If so, we already has .of_xlate_dai_id callback for this purpose. Does below help your issue ? commit 73b17f1a65c881fcf97109d77056006da2d40152 commit a180e8b988437b3e84a1b501ac4d073467602ca6 Samplle codes are linux/sound/soc/codecs/hdmi-codec.c :: hdmi_of_xlate_dai_id https://patchwork.kernel.org/patch/9732285/ (I posted, but not yet applied) Best regards --- Kuninori Morimoto
On Mon, Jun 12, 2017 at 7:09 PM, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > > Hi John > >> On Mon, Jun 12, 2017 at 3:10 PM, Mark Brown <broonie@kernel.org> wrote: >> > On Mon, Jun 12, 2017 at 01:52:46PM -0700, John Stultz wrote: >> > >> >> + sound { >> >> + compatible = "simple-audio-card"; >> >> + simple-audio-card,name = "hikey-hdmi"; >> > >> > Now the graph card has been merged it's probably a good idea to be using >> > that, it's generally a better idea for pretty much all use cases. >> >> I've taken a shot today trying to convert over to the audio-graph-card >> method (which isn't yet upstream, but in -next), but I've been running >> into some quirks. >> >> Part of the issue is the adv7533 bridge already has a endpoint port >> entry for the dsi output. So when I add the codec_endpoint port, it >> wants the two port entries to be enumerated, so I get something like: >> >> port@0 { >> adv7533_in: endpoint { >> remote-endpoint = <&dsi_out0>; >> }; >> }; >> port@1 { >> codec_endpoint: endpoint { >> remote-endpoint = <&i2s0_cpu_endpoint>; >> }; >> }; >> >> But this causes it to try to link to hdmi-hifi.1 (which doesn't exist) >> instead of hdmi-hifi.0. >> >> If I instead swap the entries, so the codec_endpoint is first on port >> 0, then the the audio link is properly setup, but the dsi >> initialization falls over. > > I think you want to exchange port@1 as ID=0 for ALSA SoC ? > If so, we already has .of_xlate_dai_id callback for this purpose. > > Does below help your issue ? > > commit 73b17f1a65c881fcf97109d77056006da2d40152 > commit a180e8b988437b3e84a1b501ac4d073467602ca6 > > Samplle codes are > > linux/sound/soc/codecs/hdmi-codec.c :: hdmi_of_xlate_dai_id > https://patchwork.kernel.org/patch/9732285/ (I posted, but not yet applied) :/ So with the above approach I did manage to get it working. And I'm not opposed to migrating to this, but it doesn't really feel finished at the moment (the magic hard coding of port 2 ==> 0 feels a bit hackish, but I'm not sure how else one can use the same port style dts description across the two namespaces of display and audio). While I'm sure for many cases the graph approach is much cleaner, I'm not sure for this one how its improving over the much simpler simple-audio-card binding. So yea, I'm ok with migrating to this, but I'm also not super enthusiastic about delaying (I'm guessing likely to 4.14 since we have new unqueued code dependencies to get in) the enabling of this functionality in order to moving to something that at least for this case "seems" a bit more hackish. The simple-audio-card bindings are still valid, so maybe could we merge the proposed dts change I've submitted and then look at migrating over to the audio-card-graph once the dependencies are all in place? thanks -john
On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote: > description across the two namespaces of display and audio). While > I'm sure for many cases the graph approach is much cleaner, I'm not > sure for this one how its improving over the much simpler > simple-audio-card binding. It meana you're not limited to single point to point digital linka which scales better (and is much the same reason why the video stuff uses graphs too). > The simple-audio-card bindings are still valid, so maybe could we > merge the proposed dts change I've submitted and then look at > migrating over to the audio-card-graph once the dependencies are all > in place? Which dependencies sorry?
On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote: > >> description across the two namespaces of display and audio). While >> I'm sure for many cases the graph approach is much cleaner, I'm not >> sure for this one how its improving over the much simpler >> simple-audio-card binding. > > It meana you're not limited to single point to point digital linka which > scales better (and is much the same reason why the video stuff uses > graphs too). > >> The simple-audio-card bindings are still valid, so maybe could we >> merge the proposed dts change I've submitted and then look at >> migrating over to the audio-card-graph once the dependencies are all >> in place? > > Which dependencies sorry? So the adv7511 driver needs to add a .get_dai_id() hook to renumber the port to dai id. thanks -john
On Tue, Jun 13, 2017 at 12:57:44PM -0700, John Stultz wrote: > On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote: > > On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote: > >> The simple-audio-card bindings are still valid, so maybe could we > >> merge the proposed dts change I've submitted and then look at > >> migrating over to the audio-card-graph once the dependencies are all > >> in place? > > Which dependencies sorry? > So the adv7511 driver needs to add a .get_dai_id() hook to renumber > the port to dai id. Oh, that should be pretty trivial though? Especially given that it seems like it's been difficult to get DT changes to this platform merged I'm a bit nervous about the followup here.
On Tue, Jun 13, 2017 at 1:16 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jun 13, 2017 at 12:57:44PM -0700, John Stultz wrote: >> On Tue, Jun 13, 2017 at 12:54 PM, Mark Brown <broonie@kernel.org> wrote: >> > On Tue, Jun 13, 2017 at 12:40:28PM -0700, John Stultz wrote: > >> >> The simple-audio-card bindings are still valid, so maybe could we >> >> merge the proposed dts change I've submitted and then look at >> >> migrating over to the audio-card-graph once the dependencies are all >> >> in place? > >> > Which dependencies sorry? > >> So the adv7511 driver needs to add a .get_dai_id() hook to renumber >> the port to dai id. > > Oh, that should be pretty trivial though? Especially given that it > seems like it's been difficult to get DT changes to this platform merged > I'm a bit nervous about the followup here. Its a trivial bit of code, but its in the drm directory and I suspect at this stage I'm not going to be able to get maintainer attention for it in time for 4.13 (plus since it depends on things in the ASoC tree, so it needs cross-maintainer acks and probably to go through the ASoC tree). Plus the new dts changes with the graph need review (where as the existing ones have been sent out 5 or so times w/o comment) before they are merged. Its just a ton of extra friction that doesn't gain much and just keeps functionality from working upstream. As for your concern about follow up, I've got the patches lined up here: https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003 https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da And will revise and submit the adv7511 one for initial review today. So again, I'm happy to migrate over. But I just think, as the graph stuff doesn't actually benefit us much in this specific case (there is only one supported audio out path right now), its not worth blocking getting the functionality working upstream for another 3-6 months. thanks -john
On Tue, Jun 13, 2017 at 01:42:15PM -0700, John Stultz wrote: > Its a trivial bit of code, but its in the drm directory and I suspect > at this stage I'm not going to be able to get maintainer attention for > it in time for 4.13 (plus since it depends on things in the ASoC tree, > so it needs cross-maintainer acks and probably to go through the ASoC > tree). Can I apply it directly? Given that it's all ASoC code it might be the most expedient way to handle this. It doesn't look like there's any DRM changes to meaningfully conflict with. > Plus the new dts changes with the graph need review (where as the > existing ones have been sent out 5 or so times w/o comment) before > they are merged. Its just a ton of extra friction that doesn't gain > much and just keeps functionality from working upstream. What's happened here is that because you've been sending it for so long the code around it changed so there's new review comments. > As for your concern about follow up, I've got the patches lined up here: > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003 > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da > And will revise and submit the adv7511 one for initial review today. > So again, I'm happy to migrate over. But I just think, as the graph > stuff doesn't actually benefit us much in this specific case (there is > only one supported audio out path right now), its not worth blocking > getting the functionality working upstream for another 3-6 months. If it might take two kernel releases to get such a trivial change in then that's not giving me a warm fuzzy feeling... there's something not working very well with the HiKey support overall with frequent process problems (only today I had someone resubmitting already applied code).
On Tue, Jun 13, 2017 at 2:46 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jun 13, 2017 at 01:42:15PM -0700, John Stultz wrote: > >> Its a trivial bit of code, but its in the drm directory and I suspect >> at this stage I'm not going to be able to get maintainer attention for >> it in time for 4.13 (plus since it depends on things in the ASoC tree, >> so it needs cross-maintainer acks and probably to go through the ASoC >> tree). > > Can I apply it directly? Given that it's all ASoC code it might be the > most expedient way to handle this. It doesn't look like there's any DRM > changes to meaningfully conflict with. I just sent it out, and I'll leave it to your discretion as to if it can be picked up and go through your tree or if we need to wait for acks or the next merge window. >> Plus the new dts changes with the graph need review (where as the >> existing ones have been sent out 5 or so times w/o comment) before >> they are merged. Its just a ton of extra friction that doesn't gain >> much and just keeps functionality from working upstream. > > What's happened here is that because you've been sending it for so long > the code around it changed so there's new review comments. > >> As for your concern about follow up, I've got the patches lined up here: >> https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=31e8b980d93087cf30cd708fb0b2cc48e906d003 > >> https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=f24a0ccdeeb46ce3eb8864fd43f7a34c673ff9da > >> And will revise and submit the adv7511 one for initial review today. > >> So again, I'm happy to migrate over. But I just think, as the graph >> stuff doesn't actually benefit us much in this specific case (there is >> only one supported audio out path right now), its not worth blocking >> getting the functionality working upstream for another 3-6 months. > > If it might take two kernel releases to get such a trivial change in > then that's not giving me a warm fuzzy feeling... there's something > not working very well with the HiKey support overall with frequent > process problems (only today I had someone resubmitting already applied > code). Well, I can't speak to other things, but we do seem to have trouble getting dts changes picked up via Wei, and we probably need to add some extra maintainers to make sure things don't drop through. I think Ulf was considering this? thanks -john
On Tue, Jun 13, 2017 at 03:08:09PM -0700, John Stultz wrote: > Well, I can't speak to other things, but we do seem to have trouble > getting dts changes picked up via Wei, and we probably need to add > some extra maintainers to make sure things don't drop through. I think > Ulf was considering this? How about you? :)
On Wed, Jun 14, 2017 at 7:42 AM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jun 13, 2017 at 03:08:09PM -0700, John Stultz wrote: > >> Well, I can't speak to other things, but we do seem to have trouble >> getting dts changes picked up via Wei, and we probably need to add >> some extra maintainers to make sure things don't drop through. I think >> Ulf was considering this? > > How about you? :) I'm open to it. Though its a bit self-serving at the moment. :) I also worry my expertise around the hardware isn't high enough to be able to properly review submissions, but I am happy to look over, collect patches, and test on the hardware I have. thanks -john
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts index 5cdfe73..c916929 100644 --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts @@ -468,6 +468,21 @@ method = "smc"; }; }; + + sound { + compatible = "simple-audio-card"; + simple-audio-card,name = "hikey-hdmi"; + + simple-audio-card,dai-link@0 { /* I2S - HDMI */ + format = "i2s"; + cpu { + sound-dai = <&i2s0 0>; + }; + codec { + sound-dai = <&adv7533>; + }; + }; + }; }; &uart2 { @@ -508,6 +523,7 @@ interrupts = <1 2>; pd-gpio = <&gpio0 4 0>; adi,dsi-lanes = <4>; + #sound-dai-cells = <0>; port { adv7533_in: endpoint { diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi index 5013e4b..f2e218c 100644 --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi @@ -332,6 +332,19 @@ status = "disabled"; }; + dma0: dma@f7370000 { + compatible = "hisilicon,k3-dma-1.0"; + reg = <0x0 0xf7370000 0x0 0x1000>; + #dma-cells = <1>; + dma-channels = <15>; + dma-requests = <32>; + interrupts = <0 84 4>; + clocks = <&sys_ctrl HI6220_EDMAC_ACLK>; + dma-no-cci; + dma-type = "hi6220_dma"; + status = "ok"; + }; + dual_timer0: timer@f8008000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x0 0xf8008000 0x0 0x1000>; @@ -805,6 +818,19 @@ #thermal-sensor-cells = <1>; }; + i2s0: i2s@f7118000{ + compatible = "hisilicon,hi6210-i2s"; + reg = <0x0 0xf7118000 0x0 0x8000>; /* i2s unit */ + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>; /* 155 "DigACodec_intr"-32 */ + clocks = <&sys_ctrl HI6220_DACODEC_PCLK>, + <&sys_ctrl HI6220_BBPPLL0_DIV>; + clock-names = "dacodec", "i2s-base"; + dmas = <&dma0 15 &dma0 14>; + dma-names = "rx", "tx"; + hisilicon,sysctrl-syscon = <&sys_ctrl>; + #sound-dai-cells = <1>; + }; + thermal-zones { cls0: cls0 {
Add entry for k3-dma driver and i2s/hdmi audio devices. This enables HDMI audio output. Cc: Zhangfei Gao <zhangfei.gao@linaro.org> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Jaroslav Kysela <perex@perex.cz> Cc: Takashi Iwai <tiwai@suse.com> Cc: Wei Xu <xuwei5@hisilicon.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Andy Green <andy@warmcat.com> Cc: Dave Long <dave.long@linaro.org> Cc: Guodong Xu <guodong.xu@linaro.org> Cc: Antonio Borneo <borneo.antonio@gmail.com> Cc: Olof Johansson <olof@lixom.net> Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: John Stultz <john.stultz@linaro.org> --- Olof/Arnd: I'm having trouble getting a repsonse from Wei on this patch, and want to try to avoid missing another merge window. Might you consider queuing this directly? v2: * Split core i2s entry into dtsi and hdmi specific bits into hikey dts v4: * Rework simple-card to use many-dai-links method, as there may be other links in the future --- arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++++++ arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 26 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) -- 2.7.4