[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
On 04/11/2016 04:08 PM, Jose Abreu wrote: [...] >> Currently there is also zero support of of-graph in ASoC, so a bit of work >> is required to get this integrated properly. >> > > I also believe this would be the better option but in the meantime can't I > integrate the audio like it is being done in the dw-hdmi driver[1]? In this > driver the audio is registered as a sound card and is conditionally built > using > Kconfig, just like I was doing in the previous versions. I know you said the > HDMI audio is still an open issue but it seems that for this case it was > accepted. > > [1] > http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c Maybe, but I'm not sure it would work in this case. Resources are probably better invested in working towards a proper solution.
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
Hi Lars,
On 11-04-2016 13:23, Lars-Peter Clausen wrote:
> On 04/11/2016 01:32 PM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
Hi Lars,
On 09-04-2016 16:02, Lars-Peter Clausen wrote:
> On 04/08/2016 06:12 PM, Jose Abreu wrote:
> [...]
>>> [...]
+- adi,enable-audio: If set the ADV7511 driver will register a codec
interface
+ into ALSA SoC.
>>> This is not a description of the hardware.
>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
>> transmitter routes audio signals" ?
> I don't think we need this property. There is no problem with registering
> the audio part unconditionally. As long as there is no connection we wont
> create a sound card that is exposed to userspace.
>
This change was suggested by Laurent Pinchart and was introduced in v3.
Quoting
Laurent:
"The idea is that enabling support for ADV7511 audio in the kernel isn't
coupled
with whether the system includes audio support. It would be confusing, and
would
also waste resources, to create a Linux sound device when no sound channel
is
routed on the board."
>>> I wouldn't care too much about this at this point, the extra amount of
>>> resources required for registering the CODEC (but not the sound card) is
>>> just a few bytes (sizeof(struct snd_soc_codec)).
>>>
>>> Nevertheless what we should do is describe the hardware and from this
>>> information infer whether there is a audio connection or not and if there is
>>> none we might skip registering the CODEC. In my opinion this hardware
>>> description should be modeled using of-graph, having a connection between
>>> the SoC side and the adv7511 SPDIF or I2S port.
>>>
>> You mean something like this:
>>
>> sound_playback: sound_playback {
>> compatible = "simple-audio-card";
>> [...]
>> simple-audio-card,format = "i2s";
>> [...]
>> }
>>
>> adv7511 at xx {
>> compatible = "adi,adv7511";
>> [...]
>>
>> ports {
>> [...]
>> /* Audio Output */
>> port at x {
>> reg = ;
>> endpoint {
>> remote-endpoint = <&sound_playback>;
>> }
>> }
>> }
>> }
> Yes, something like that. Not exactly like that, but similar. One of the
> core concepts of of-graph is that there is always a description of the
> connection from both sides, this way each side can independently figure out
> where it is connected.
>
> Currently there is also zero support of of-graph in ASoC, so a bit of work
> is required to get this integrated properly.
>
I also believe this would be the better option but in the meantime can't I
integrate the audio like it is being done in the dw-hdmi driver[1]? In this
driver the audio is registered as a sound card and is conditionally built using
Kconfig, just like I was doing in the previous versions. I know you said the
HDMI audio is still an open issue but it seems that for this case it was
accepted.
[1]
http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c
Best regards,
Jose Miguel Abreu
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
On 04/11/2016 01:32 PM, Jose Abreu wrote:
> Hi Lars,
>
>
> On 11-04-2016 10:33, Lars-Peter Clausen wrote:
>> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>>> Hi Lars,
>>>
>>>
>>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
On 04/08/2016 06:12 PM, Jose Abreu wrote:
[...]
>> [...]
>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec
>>> interface
>>> + into ALSA SoC.
>> This is not a description of the hardware.
> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
> transmitter routes audio signals" ?
I don't think we need this property. There is no problem with registering
the audio part unconditionally. As long as there is no connection we wont
create a sound card that is exposed to userspace.
>>> This change was suggested by Laurent Pinchart and was introduced in v3.
>>> Quoting
>>> Laurent:
>>> "The idea is that enabling support for ADV7511 audio in the kernel isn't
>>> coupled
>>> with whether the system includes audio support. It would be confusing, and
>>> would
>>> also waste resources, to create a Linux sound device when no sound channel
>>> is
>>> routed on the board."
>> I wouldn't care too much about this at this point, the extra amount of
>> resources required for registering the CODEC (but not the sound card) is
>> just a few bytes (sizeof(struct snd_soc_codec)).
>>
>> Nevertheless what we should do is describe the hardware and from this
>> information infer whether there is a audio connection or not and if there is
>> none we might skip registering the CODEC. In my opinion this hardware
>> description should be modeled using of-graph, having a connection between
>> the SoC side and the adv7511 SPDIF or I2S port.
>>
>
> You mean something like this:
>
> sound_playback: sound_playback {
> compatible = "simple-audio-card";
> [...]
> simple-audio-card,format = "i2s";
> [...]
> }
>
> adv7511 at xx {
> compatible = "adi,adv7511";
> [...]
>
> ports {
> [...]
> /* Audio Output */
> port at x {
> reg = ;
> endpoint {
> remote-endpoint = <&sound_playback>;
> }
> }
> }
> }
Yes, something like that. Not exactly like that, but similar. One of the
core concepts of of-graph is that there is always a description of the
connection from both sides, this way each side can independently figure out
where it is connected.
Currently there is also zero support of of-graph in ASoC, so a bit of work
is required to get this integrated properly.
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
Hi Lars,
On 11-04-2016 10:33, Lars-Peter Clausen wrote:
> On 04/11/2016 11:27 AM, Jose Abreu wrote:
>> Hi Lars,
>>
>>
>> On 09-04-2016 16:02, Lars-Peter Clausen wrote:
>>> On 04/08/2016 06:12 PM, Jose Abreu wrote:
>>> [...]
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec
>> interface
>> + into ALSA SoC.
> This is not a description of the hardware.
Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?
>>> I don't think we need this property. There is no problem with registering
>>> the audio part unconditionally. As long as there is no connection we wont
>>> create a sound card that is exposed to userspace.
>>>
>> This change was suggested by Laurent Pinchart and was introduced in v3.
>> Quoting
>> Laurent:
>> "The idea is that enabling support for ADV7511 audio in the kernel isn't
>> coupled
>> with whether the system includes audio support. It would be confusing, and
>> would
>> also waste resources, to create a Linux sound device when no sound channel is
>> routed on the board."
> I wouldn't care too much about this at this point, the extra amount of
> resources required for registering the CODEC (but not the sound card) is
> just a few bytes (sizeof(struct snd_soc_codec)).
>
> Nevertheless what we should do is describe the hardware and from this
> information infer whether there is a audio connection or not and if there is
> none we might skip registering the CODEC. In my opinion this hardware
> description should be modeled using of-graph, having a connection between
> the SoC side and the adv7511 SPDIF or I2S port.
>
You mean something like this:
sound_playback: sound_playback {
compatible = "simple-audio-card";
[...]
simple-audio-card,format = "i2s";
[...]
}
adv7511 at xx {
compatible = "adi,adv7511";
[...]
ports {
[...]
/* Audio Output */
port at x {
reg = ;
endpoint {
remote-endpoint = <&sound_playback>;
}
}
}
}
?
Best regards,
Jose Miguel Abreu
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
On 04/11/2016 11:27 AM, Jose Abreu wrote: > Hi Lars, > > > On 09-04-2016 16:02, Lars-Peter Clausen wrote: >> On 04/08/2016 06:12 PM, Jose Abreu wrote: >> [...] [...] > +- adi,enable-audio: If set the ADV7511 driver will register a codec > interface > + into ALSA SoC. This is not a description of the hardware. >>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511 >>> transmitter routes audio signals" ? >> I don't think we need this property. There is no problem with registering >> the audio part unconditionally. As long as there is no connection we wont >> create a sound card that is exposed to userspace. >> > > This change was suggested by Laurent Pinchart and was introduced in v3. > Quoting > Laurent: > "The idea is that enabling support for ADV7511 audio in the kernel isn't > coupled > with whether the system includes audio support. It would be confusing, and > would > also waste resources, to create a Linux sound device when no sound channel is > routed on the board." I wouldn't care too much about this at this point, the extra amount of resources required for registering the CODEC (but not the sound card) is just a few bytes (sizeof(struct snd_soc_codec)). Nevertheless what we should do is describe the hardware and from this information infer whether there is a audio connection or not and if there is none we might skip registering the CODEC. In my opinion this hardware description should be modeled using of-graph, having a connection between the SoC side and the adv7511 SPDIF or I2S port.
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
Hi Lars, On 09-04-2016 16:02, Lars-Peter Clausen wrote: > On 04/08/2016 06:12 PM, Jose Abreu wrote: > [...] >>> [...] +- adi,enable-audio: If set the ADV7511 driver will register a codec interface + into ALSA SoC. >>> This is not a description of the hardware. >> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511 >> transmitter routes audio signals" ? > I don't think we need this property. There is no problem with registering > the audio part unconditionally. As long as there is no connection we wont > create a sound card that is exposed to userspace. > This change was suggested by Laurent Pinchart and was introduced in v3. Quoting Laurent: "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled with whether the system includes audio support. It would be confusing, and would also waste resources, to create a Linux sound device when no sound channel is routed on the board." Should I revert this? Best regards, Jose Miguel Abreu
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
On 04/08/2016 06:12 PM, Jose Abreu wrote: [...] >> >> [...] >>> +- adi,enable-audio: If set the ADV7511 driver will register a codec >>> interface >>> + into ALSA SoC. >> This is not a description of the hardware. > > Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511 > transmitter routes audio signals" ? I don't think we need this property. There is no problem with registering the audio part unconditionally. As long as there is no connection we wont create a sound card that is exposed to userspace.
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
On 04/07/2016 06:53 PM, Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
>
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> - https://github.com/analogdevicesinc/linux/
The reason why this driver is still out of tree, is because there has been
no conclusion yet on how to go forward with the whole HDMI integration. So
this is not going to get merged until that issue has been addressed.
[...]
> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
> + into ALSA SoC.
This is not a description of the hardware.
[...]
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 964b7de..539c091 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -33,6 +33,7 @@ struct snd_compr_stream;
> #define SND_SOC_DAIFMT_DSP_B 5 /* L data MSB during FRM LRC */
> #define SND_SOC_DAIFMT_AC97 6 /* AC97 */
> #define SND_SOC_DAIFMT_PDM 7 /* Pulse density modulation */
> +#define SND_SOC_DAIFMT_SPDIF 8 /* SPDIF */
This needs to be a separate patch.
[alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support
Hi Lars,
On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> - https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.
Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?
>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec
>> interface
>> + into ALSA SoC.
> This is not a description of the hardware.
Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?
>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>> #define SND_SOC_DAIFMT_DSP_B5 /* L data MSB during FRM LRC
>> */
>> #define SND_SOC_DAIFMT_AC97 6 /* AC97 */
>> #define SND_SOC_DAIFMT_PDM 7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF8 /* SPDIF */
> This needs to be a separate patch.
Ok.
>
>
> ___
> linux-snps-arc mailing list
> linux-snps-arc at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Best regards,
Jose Miguel Abreu
