Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Kuninori Morimoto

Hi Lars

Thank you for your feedback

> > int snd_soc_runtime_set_dai_fmt(xxx)
> > {
> > ...
> > /* Flip the polarity for the "CPU" end of a CODEC<->CODEC link */
> > if (cpu_dai->codec) {
> > ...
> > }
> > ...
> > }
(snip)
> This is for CODEC<->CODEC links where no CPU DAI is involved and the
> "CPU" side of the DAI link is actually a CODEC.

Wow !!

> Ideally we'd make the DAI links agnostic to what is connected, e.g. it
> shouldn't matter what type is connected to what side. But that does not
> work since things are anti-symmetric between CPU and CODEC DAIs. On
> CODEC DAIs the capture stream is output, on CPU DAIs the capture stream
> is input, similar thing for playback. So we need to know whether the DAI
> is a CPU DAI or a CODEC DAI.
> 
> Fixing this at this point is near to impossible since it requires
> invasive changes to all existing drivers. So we need this code and can't
> drop it.
> 
> The best we can do is try to come up with a generic interface that is
> DAI type independent and recommend this interface for new drivers.

Hmm...
OK, now, I'm investigating ALSA SoC struct connection.
And yes, existing code has deep relationship each other.
Thus, fixing/droping seems very dificult...
I think each struct has random pointer connection which doesn't care
about ALSA SoC's hierarchy.

I would like to indicate my opinion here.
My strange point are...

1. snd_soc_pcm_runtime has pointer to Card/Codec/Platform, but no CPU
2. snd_soc_dai has pointer to [component] and [Codec]
   [component] is its parent, [Codec] is maybe Hack
3. [component] has pointer to [card] and [Codec]
   [card] is understandable, [Codec] is maybe Hack

It seems many struct would like to access to [Codec], thus, each struct has
codec pointer (as hack ?), but it is ignoring ALSA SoC hierarchy.

It will be more clear if...

IDEA 1

   Each component (= CPU/Codec/Platform) has pointer to Card now.
   How about Card has pointer to each component ?
   if so, we can access to every struct.
   Card -> CPU/Codec/Platform, CPU/Codec/Platform -> Card
   Then, we want to have component related macro/flag

#define component_is_cpu(component) (component->flag & COMPONENT_CPU)
#define component_is_codec(component) (component->flag & 
COMPONENT_CODEC)
#define component_is_platform(component) (component->flag & 
COMPONENT_PLATFORM)

#define component_to_cpu() container_of(xxx)
#define component_to_codec() container_of(xxx)
#define component_to_platorm() container_of(xxx)

IDEA 2

   if IDEA1 was OK, snd_soc_pcm_runtime already has card pointer.
   This means snd_soc_pcm_runtime can access to every component struct.

   snd_soc_dai has its parent component.
   This means snd_soc_dai can access to Card, and Card can access to every 
component.
   And then, cpu_dai->codec has no issue if we can use component_is_xxx() macro 
?

- if (cpu_dai->codec)
+ if (component_is_codec(cpu_dai->component))

If IDEA1 / IDEA2 are OK, we can cleanup each struct,
especially hack-able random pointer ?

Best regards
---
Kuninori Morimoto


Re: Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Takashi Sakamoto

Lars,

On Jul 29 2016 05:33, Lars-Peter Clausen wrote:

Hotplug is something that always pops up sooner or later. E.g. if someone
puts a ASoC supported CODEC on a hot-pluggable device (maybe USB) we
don't want to duplicate the code, but be able to reuse.


(A bit to sidetrack)

To me, it's unclear for devices on USB. When ALSA SoC part supports 
these devices, what is the scenario you assumed? In short, assuming we 
put codes to ALSA SoC part, what is the shape of the corresponding 
devices and links of pairs of endpoints? Additionally, in this case, 
what codes are able to be reused?



Regards

Takashi Sakamoto


Re: [PATCH v3 0/6] Add R8A7794/SILK sound DT support

2016-07-28 Thread Kuninori Morimoto

Hi

>Here's the set of 6 patches against Simon Horman's 'renesas.git' repo,
> 'renesas-devel-20160725-v4.7-rc7' tag. I'm adding the sound device tree 
> support
> for the R8A7794 SoC based SILK board.
> 
> [1/6] ARM: dts: r8a7794: add audio clocks
> [2/6] ARM: dts: r8a7794: add MSTP5 clocks
> [3/6] ARM: dts: r8a7794: add MSTP10 clocks
> [4/6] ARM: dts: r8a7794: add Audio-DMAC support
> [5/6] ARM: dts: r8a7794: add sound support
> [6/6] ARM: dts: silk: add sound support

For all patches

Acked-by: Kuninori Morimoto 


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Lars-Peter Clausen
On 07/28/2016 10:42 PM, Takashi Iwai wrote:
> On Thu, 28 Jul 2016 22:33:31 +0200,
> Lars-Peter Clausen wrote:
>>
>> On 07/28/2016 05:46 AM, Vinod Koul wrote:
>>> On Wed, Jul 27, 2016 at 10:22:41PM +0200, Takashi Iwai wrote:
 On Wed, 27 Jul 2016 20:22:33 +0200,
 Mark Brown wrote:
>
> On Wed, Jul 27, 2016 at 08:11:49PM +0200, Takashi Iwai wrote:
>
>> I'm wondering whether it's a better option to block the unbind
>> behavior, either in driver base (allowing to return an error) or in
>> the sound side (waiting in remove() until the sane point). 
>
> That's certainly going to be a lot easier and part of the reason it's
> never been looked at much is that (unlike USB) there's very little
> reason why an ASoC sound card would ever be hotplugged - even in
> development these days the normal development flow involves rebooting.
>>>
>>> I agree, it makese no sense for devices to be hotplugged. And for
>>> developement flows people can do rmmond and insmod. That works fine!
>>
>> I don't agree. In my opinion hot-plug is an essential feature of a
>> modern device driver framework and if ASoC wants to claim to fall in
>> this category we ought to support it. Hotplug is something that always
>> pops up sooner or later. E.g. if someone puts a ASoC supported CODEC on
>> a hot-pluggable device (maybe USB) we don't want to duplicate the code,
>> but be able to reuse.
>>
>> One area that e.g. requires hot-plug/-unplug and were ASoC supported
>> devices are used is embedded development boards that support shields and
>> devicetree overlays. Like e.g. RPI and similar.
>>
>> The reason why we don't support hot-unplug in ASoC at the moment is
>> because it is not trivial to implement and nobody has cared enough yet.
>>
>> But if somebody wants to and has the resources to implement this we
>> should not discourage this.
>>
>> I'd very much prefer to have proper hot-plug support instead of
>> prohibiting unbinding even on systems that do not require hot-plug
>> support normally. It's a much cleaner solution.
> 
> Well, but hot-unplugging only a component like codec would be needed
> in a real scenario?  It's a different story from the hotplug in the
> card level.

With overlays I'm not sure if we can control the remove order. So the
component might be removed before the card complex.



Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Mark Brown
On Thu, Jul 28, 2016 at 10:33:31PM +0200, Lars-Peter Clausen wrote:
> On 07/28/2016 05:46 AM, Vinod Koul wrote:

> > I agree, it makese no sense for devices to be hotplugged. And for
> > developement flows people can do rmmond and insmod. That works fine!

> I don't agree. In my opinion hot-plug is an essential feature of a
> modern device driver framework and if ASoC wants to claim to fall in
> this category we ought to support it. Hotplug is something that always
> pops up sooner or later. E.g. if someone puts a ASoC supported CODEC on
> a hot-pluggable device (maybe USB) we don't want to duplicate the code,
> but be able to reuse.

Right, so there's two bits to hotplug - there's hotplugging individual
components separately to the card and there's hotplugging cards en
masse including some of their components.  The latter case definitely
does make sense and should have a reasonable chance of working already.
Hotplugging individual components is much more of a nice to have, though
as you say if someone wants to implement it that's obviously not a
problem.


signature.asc
Description: PGP signature


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Takashi Iwai
On Thu, 28 Jul 2016 22:33:31 +0200,
Lars-Peter Clausen wrote:
> 
> On 07/28/2016 05:46 AM, Vinod Koul wrote:
> > On Wed, Jul 27, 2016 at 10:22:41PM +0200, Takashi Iwai wrote:
> >> On Wed, 27 Jul 2016 20:22:33 +0200,
> >> Mark Brown wrote:
> >>>
> >>> On Wed, Jul 27, 2016 at 08:11:49PM +0200, Takashi Iwai wrote:
> >>>
>  I'm wondering whether it's a better option to block the unbind
>  behavior, either in driver base (allowing to return an error) or in
>  the sound side (waiting in remove() until the sane point). 
> >>>
> >>> That's certainly going to be a lot easier and part of the reason it's
> >>> never been looked at much is that (unlike USB) there's very little
> >>> reason why an ASoC sound card would ever be hotplugged - even in
> >>> development these days the normal development flow involves rebooting.
> > 
> > I agree, it makese no sense for devices to be hotplugged. And for
> > developement flows people can do rmmond and insmod. That works fine!
> 
> I don't agree. In my opinion hot-plug is an essential feature of a
> modern device driver framework and if ASoC wants to claim to fall in
> this category we ought to support it. Hotplug is something that always
> pops up sooner or later. E.g. if someone puts a ASoC supported CODEC on
> a hot-pluggable device (maybe USB) we don't want to duplicate the code,
> but be able to reuse.
> 
> One area that e.g. requires hot-plug/-unplug and were ASoC supported
> devices are used is embedded development boards that support shields and
> devicetree overlays. Like e.g. RPI and similar.
> 
> The reason why we don't support hot-unplug in ASoC at the moment is
> because it is not trivial to implement and nobody has cared enough yet.
> 
> But if somebody wants to and has the resources to implement this we
> should not discourage this.
> 
> I'd very much prefer to have proper hot-plug support instead of
> prohibiting unbinding even on systems that do not require hot-plug
> support normally. It's a much cleaner solution.

Well, but hot-unplugging only a component like codec would be needed
in a real scenario?  It's a different story from the hotplug in the
card level.


Takashi


Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Lars-Peter Clausen
On 07/28/2016 05:46 AM, Vinod Koul wrote:
> On Wed, Jul 27, 2016 at 10:22:41PM +0200, Takashi Iwai wrote:
>> On Wed, 27 Jul 2016 20:22:33 +0200,
>> Mark Brown wrote:
>>>
>>> On Wed, Jul 27, 2016 at 08:11:49PM +0200, Takashi Iwai wrote:
>>>
 I'm wondering whether it's a better option to block the unbind
 behavior, either in driver base (allowing to return an error) or in
 the sound side (waiting in remove() until the sane point). 
>>>
>>> That's certainly going to be a lot easier and part of the reason it's
>>> never been looked at much is that (unlike USB) there's very little
>>> reason why an ASoC sound card would ever be hotplugged - even in
>>> development these days the normal development flow involves rebooting.
> 
> I agree, it makese no sense for devices to be hotplugged. And for
> developement flows people can do rmmond and insmod. That works fine!

I don't agree. In my opinion hot-plug is an essential feature of a
modern device driver framework and if ASoC wants to claim to fall in
this category we ought to support it. Hotplug is something that always
pops up sooner or later. E.g. if someone puts a ASoC supported CODEC on
a hot-pluggable device (maybe USB) we don't want to duplicate the code,
but be able to reuse.

One area that e.g. requires hot-plug/-unplug and were ASoC supported
devices are used is embedded development boards that support shields and
devicetree overlays. Like e.g. RPI and similar.

The reason why we don't support hot-unplug in ASoC at the moment is
because it is not trivial to implement and nobody has cared enough yet.

But if somebody wants to and has the resources to implement this we
should not discourage this.

I'd very much prefer to have proper hot-plug support instead of
prohibiting unbinding even on systems that do not require hot-plug
support normally. It's a much cleaner solution.

- Lars



Re: [alsa-devel] Question about struct snd_soc_dai() :: cpu_dai->codec

2016-07-28 Thread Lars-Peter Clausen
On 07/26/2016 07:41 AM, Kuninori Morimoto wrote:
> 
> Hi ALSA SoC
> 
> My current headache is ALSA SoC's each modules (= Card/Codec/CPU/Platform)
> doesn't care about "unbind/rmmod".
> For example, if someone unbinded/rmmoded "Codec", Card or other modules
> doesn't know about it. Thus, user can continue to use this sound card,
> and kernel will be Oops.
> 
> So, I would like to solve this issue, and for this purpose,
> now I'm investigating about ALSA SoC structures.
> It is now super complex, and I noticed that it is having duplicated
> struct members.
> 
> If possible, I would like to cleanup this issue.
> 
> During this investigating, I noticed 1 strange operation
> 
> ${LINUX}/sound/soc/soc-core.c :: snd_soc_runtime_set_dai_fmt
> 
> int snd_soc_runtime_set_dai_fmt(xxx)
> {
>   ...
>   /* Flip the polarity for the "CPU" end of a CODEC<->CODEC link */
>   if (cpu_dai->codec) {
>   ...
>   }
>   ...
> }
> 
> struct snd_soc_dai {
>...
>   /* parent platform/codec */
>   struct snd_soc_codec *codec;
>   struct snd_soc_component *component;
>   ...
> }
> 
> According to snd_soc_dai, *codec is its "parent".
> I wonder does "cpu_dai" really can have "codec" parent ??
> I think this is not correct, and we can remove this operation ?

This is for CODEC<->CODEC links where no CPU DAI is involved and the
"CPU" side of the DAI link is actually a CODEC.

Ideally we'd make the DAI links agnostic to what is connected, e.g. it
shouldn't matter what type is connected to what side. But that does not
work since things are anti-symmetric between CPU and CODEC DAIs. On
CODEC DAIs the capture stream is output, on CPU DAIs the capture stream
is input, similar thing for playback. So we need to know whether the DAI
is a CPU DAI or a CODEC DAI.

Fixing this at this point is near to impossible since it requires
invasive changes to all existing drivers. So we need this code and can't
drop it.

The best we can do is try to come up with a generic interface that is
DAI type independent and recommend this interface for new drivers.



Re: [PATCH 1/3] drm: bridge: add DesignWare HDMI I2S audio support

2016-07-28 Thread Jose Abreu
Hi,


On 24-06-2016 03:40, Kuninori Morimoto wrote:
> From: Kuninori Morimoto 
>
> Current dw-hdmi is supporting sound via AHB bus, but it has
> I2S audio feature too. This patch adds I2S audio support to dw-hdmi.
> This HDMI I2S is supported by using ALSA SoC common HDMI encoder
> driver.
>
> Signed-off-by: Kuninori Morimoto 
> ---
>

I just tested this patch and everything seems ok. Should I give
my tested-by?

Best regards,
Jose Miguel Abreu