Re: [PATCH] ASoC: cpcap: Implement set_tdm_slot for voice call support

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 4062 bytes --]

Hi,

On Tue, Feb 18, 2020 at 07:32:11AM -0800, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [200218 15:16]:
> > On 18/02/2020 1.23, Tony Lindgren wrote:
> > > * Peter Ujfalusi <peter.ujfalusi@ti.com> [200214 13:30]:
> > >> Hi Tony,
> > >>
> > >> On 12/02/2020 16.46, Tony Lindgren wrote:
> > >>> * Peter Ujfalusi <peter.ujfalusi@ti.com> [200212 09:18]:
> > >>>> On 11/02/2020 20.10, Tony Lindgren wrote:
> > >>>>> +static int cpcap_voice_set_tdm_slot(struct snd_soc_dai *dai,
> > >>>>> +				    unsigned int tx_mask, unsigned int rx_mask,
> > >>>>> +				    int slots, int slot_width)
> > >>>>> +{
> > >>>>> +	struct snd_soc_component *component = dai->component;
> > >>>>> +	struct cpcap_audio *cpcap = snd_soc_component_get_drvdata(component);
> > >>>>> +	int err, ts_mask, mask;
> > >>>>> +	bool voice_call;
> > >>>>> +
> > >>>>> +	/*
> > >>>>> +	 * Primitive test for voice call, probably needs more checks
> > >>>>> +	 * later on for 16-bit calls detected, Bluetooth headset etc.
> > >>>>> +	 */
> > >>>>> +	if (tx_mask == 0 && rx_mask == 1 && slot_width == 8)
> > >>>>> +		voice_call = true;
> > >>>>> +	else
> > >>>>> +		voice_call = false;
> > >>>>
> > >>>> You only have voice call if only rx slot0 is in use?
> > >>>
> > >>> Yeah so it seems. Then there's the modem to wlcore bluetooth path that
> > >>> I have not looked at. But presumably that's again just configuring some
> > >>> tdm slot on the PMIC.
> > >>>
> > >>>> If you record mono on the voice DAI, then rx_mask is also 1, no?
> > >>>
> > >>> It is above :) But maybe I don't follow what you're asking here
> > >>
> > >> If you arecrod -Dvoice_pcm -c1 -fS8 > /dev/null
> > >> then it is reasonable that the machine driver will set rx_mask = 1
> > >>
> > >>> and maybe you have some better check in mind.
> > >>
> > >> Not sure, but relying on set_tdm_slots to decide if we are in a call
> > >> case does not sound right.
> > > 
> > > OK yeah seems at least bluetooth would need to be also handled
> > > in the set_tdm_slots.
> > 
> > set_tdm_slots() is for setting how the TDM slots supposed to be used by
> > the component and not really for things to configure different operating
> > modes.
> > 
> > If you hardwire things in set_tdm_slots() for the droid4 then how the
> > codec driver can be reused in other setups?
> 
> Right, I'm all go for better solutions :)
> 
> > >>>> You will also set the sampling rate for voice in
> > >>>> cpcap_voice_hw_params(), but that is for normal playback/capture, right?
> > >>>
> > >>> Yeah so normal playback/capture is already working with cpcap codec driver
> > >>> with mainline Linux. The voice call needs to set rate to 8000.
> > >>
> > >> But if you have a voice call initiated should not the rate be set by the
> > >> set_sysclk()?
> > > 
> > > Hmm does set_sysclk called from modem codec know that cpcap codec
> > > is the clock master based on bitclock-master and set the rate
> > > for cpcap codec?
> > 
> > Neither component should call set_sysclk, set_tdm_slots. The machine
> > driver should as it is the only one who know how things are wired...
> 
> OK, but so what's the machine driver part in this case?

simple-graph-card is the current machine driver. We might have to
introduce a Droid 4 specific driver instead. I used simple(-graph)-card
instead of introducing a new driver, since the setup was simple enough
without modem and bluetooth. The simple card was perfect to test the CPCAP
codec driver. The TDM things might be complex enough to create
a new machine driver (as I mentioned in the original patchset
adding CPCAP codec support).

Note: Don't use Motorola's tree to learn about ASoC. Their soundcard
and cpcap codec drivers are full of weird hacks. I'm pretty sure the
author(s) did not really understand how ASoC works. From my
experience you should only use their code to understand the
hardware wiring. You might also want to look into the MC13783
datasheet for the keyword "network mode".

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]