[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Lars-Peter Clausen]

On 02/05/2014 07:07 PM, Jean-Francois Moine wrote:
> On Wed, 05 Feb 2014 10:19:22 +0100
> Lars-Peter Clausen  wrote:
>
>>> So, in the CODEC, I don't see how I could update the parameters
>>> dictated by the EDID otherwise in changing the DAI driver parameters.
>>>
>>
>> The startup function is the right place. But instead of modifying the DAI
>> use snd_pcm_hw_constraint_mask64(), snd_pcm_hw_constraint_list(), etc. to
>> setup the additional constraints that come from the EDID.
>
> It is more complicated, but it works. Nevertheless, I have 2 problems:
>
> - snd_pcm_hw_constraint_list() keeps a pointer to the list, so, it
>cannot be in the stack. It fix this with static struct and rate array.
>

Right. If the struct is modified though it should be per device and not 
global. I think the best way to implement this is to make the array static 
and specify a mask for the constraint based on the EDID. E.g.

static unsigned int hdmi_rates[] = {
32000,
44100,
48000,
88200,
96000,
176400,
192000,
};

rate_mask = 0;

while (...) {
...
rate_mask |= 1 << sad[1];
}

rate_constraints->list = hdmi_rates;
rate_constraints->count = ARRAY_SIZE(hdmi_rates);
rate_constraints->mask = rate_mask;

> - snd_pcm_hw_constraint_mask64() is not exported.
>Is there an other way to set constraints on the formats/sample widths?

I think that's a bug. Both snd_pcm_hw_constraint_mask() and 
snd_pcm_hw_constraint_mask64() should be exported. Can you send a patch?

- Lars

[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Jean-Francois Moine]

On Wed, 05 Feb 2014 10:19:22 +0100
Lars-Peter Clausen  wrote:

> > So, in the CODEC, I don't see how I could update the parameters
> > dictated by the EDID otherwise in changing the DAI driver parameters.
> >  
> 
> The startup function is the right place. But instead of modifying the DAI 
> use snd_pcm_hw_constraint_mask64(), snd_pcm_hw_constraint_list(), etc. to 
> setup the additional constraints that come from the EDID.

It is more complicated, but it works. Nevertheless, I have 2 problems:

- snd_pcm_hw_constraint_list() keeps a pointer to the list, so, it
  cannot be in the stack. It fix this with static struct and rate array.

- snd_pcm_hw_constraint_mask64() is not exported.
  Is there an other way to set constraints on the formats/sample widths?

-- 
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/

[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Lars-Peter Clausen]

On 02/05/2014 12:18 PM, Mark Brown wrote:
> On Wed, Feb 05, 2014 at 10:19:22AM +0100, Lars-Peter Clausen wrote:
[..]
>> Bonus points for making this a generic helper function that takes a
>> runtime and a EDID and then applies the EDID constraints on the
>> runtime.
>
> ...as previously requested.  I know there was some discusion of broader
> moves to factor this stuff out but it'd still be good to keep it
> separated out even prior to a final non-EDID based solution so it's
> easier to refactor.

I think it will always be EDID (or ELD) based. As I understood it the 
re-factoring Takashi was talking about is related to how this data is passed 
from the graphics driver to the audio driver. The way things work right now 
in HDA land is that the graphics driver reads the EDID from the monitor, 
converts it to ELD and writes it to a special memory region in the graphics 
controller. This generates an interrupt in the audio driver and the audio 
driver reads the ELD from the hardware and sets up the constraints based on 
that. And I think that the plan is to change this to pass the EDID directly 
from the graphics driver to the audio driver without taking the detour 
through the hardware. This is what we'll need for embedded systems anyway. A 
system that allows to associate a sound driver with a specific HDMI port and 
status updates for that port, e.g. new EDID or cable connected/disconnected 
are passed from the graphics driver handling that port to the audio driver.

- Lars

[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Mark Brown]

On Wed, Feb 05, 2014 at 02:31:09PM +0100, Lars-Peter Clausen wrote:
> On 02/05/2014 12:18 PM, Mark Brown wrote:

> >...as previously requested.  I know there was some discusion of broader
> >moves to factor this stuff out but it'd still be good to keep it
> >separated out even prior to a final non-EDID based solution so it's
> >easier to refactor.

> I think it will always be EDID (or ELD) based. As I understood it
> the re-factoring Takashi was talking about is related to how this
> data is passed from the graphics driver to the audio driver. The way

Right, the data of course has to come from there originally, it's about
how we pass the data and updates to it around.
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: 

[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Mark Brown]

On Wed, Feb 05, 2014 at 10:19:22AM +0100, Lars-Peter Clausen wrote:
> On 02/05/2014 10:11 AM, Jean-Francois Moine wrote:

> >So, in the CODEC, I don't see how I could update the parameters
> >dictated by the EDID otherwise in changing the DAI driver parameters.

> The startup function is the right place. But instead of modifying
> the DAI use snd_pcm_hw_constraint_mask64(),
> snd_pcm_hw_constraint_list(), etc. to setup the additional
> constraints that come from the EDID.

Right.

> Bonus points for making this a generic helper function that takes a
> runtime and a EDID and then applies the EDID constraints on the
> runtime.

...as previously requested.  I know there was some discusion of broader
moves to factor this stuff out but it'd still be good to keep it
separated out even prior to a final non-EDID based solution so it's
easier to refactor.
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: 

[alsa-devel] [PATCH v3 4/5] ASoC: tda998x: adjust the audio hw parameters from EDID

[Lars-Peter Clausen]

On 02/05/2014 10:11 AM, Jean-Francois Moine wrote:
> On Tue, 4 Feb 2014 18:06:25 +
> Mark Brown  wrote:
>
>> On Mon, Jan 27, 2014 at 09:48:54AM +0100, Jean-Francois Moine wrote:
>>
>>> +   /* change the snd_soc_pcm_stream values of the driver */
>>> +   stream->rates = rate_mask;
>>> +   stream->channels_max = max_channels;
>>> +   stream->formats = formats;
>>
>>> +   /* copy the DAI driver to a writable area */
>>> +   dai_drv = devm_kzalloc(&pdev->dev, sizeof(tda998x_dai), GFP_KERNEL);
>>> +   if (!dai_drv)
>>> +   return -ENOMEM;
>>> +   memcpy(dai_drv, tda998x_dai, sizeof(tda998x_dai));
>>> +
>>
>> The code should be doing this by setting constraints based on the
>> current setup rather than by editing the data structure - the expecation
>> is very much that the data won't change so this prevents surprises with
>> future work on the core.
>
> As it is done in the soc core, in soc_pcm_open(), the runtime hw_params
> are initialized after the call to the CODEC startup, and the next CODEC
> event is hw_params() when the user has already chosen all the parameters.
>
> So, in the CODEC, I don't see how I could update the parameters
> dictated by the EDID otherwise in changing the DAI driver parameters.
>

The startup function is the right place. But instead of modifying the DAI 
use snd_pcm_hw_constraint_mask64(), snd_pcm_hw_constraint_list(), etc. to 
setup the additional constraints that come from the EDID.

Bonus points for making this a generic helper function that takes a runtime 
and a EDID and then applies the EDID constraints on the runtime.

- Lars