[alsa-devel] [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Ribeiro]

Em Seg, 2009-08-17 às 17:35 +0200, Daniel Mack escreveu: 
> > 0-1-0-1 should be 4 slots, with slots 2 and 4 active, but I think that
> > what you want is 1-0-1-0 = set_tdm_slot(5, 5, 4, 16). But... See below.
> 
> Maybe we should first decide which TDM mode _should_ be correct for the
> mode I'm working on, and then amend the actual implementation? And
> provide some documentation once the API is decided.

The correct should be set_tdm_slots(5, 5, 4, 16), no doubt on this.

The problem is that "16" becomes "32" only on the active slots because
of DMYSTOP. :(

> > * set_tdm_slot(5, 5, 4, 16) if pxa is slave of SFRM.
> 
> Wouldn't 1-0-1-0 rather be 0xa?

Well, I'm reading 1-0-1-0 as a time-line(1-2-3-4) from left to right,
not as the binary value of the TTSA register. Activating slots 1 and 3
means 0x5(0101b) on TTSA value (right to left).

Confusing... ;)

> > Consider that you have real network mode, and you are using 2 slots of
> > 16 bit mono audio. You want to ignore 16bit for each 16bit of data. ie,
> > only one channel active. You really don't want 32bit DMA in this case.
> 
> I can just say that in case the DMA material is _not_ 32bit, audio will
> play at half speed. So the mode I'm using seems to need the double amout
> of PCM data. For whatever reason.

Its stereo, 2x16. So 32b DMA is correct. I use 32b DMA for 16bit stereo
too, but with 1 slot of 32bits. :)

> > I agree that this should be (slot_width * number_of_active_slots) > 16
> > tough.
> 
> Ok. And that won't break your use case?

No, in my case number_of_active_slots is always 1. (Both with and
without network mode on the wire)

> > > > +			sspsp |= SSPSP_DMYSTOP(
> > > > +					(slot_width * slots) / 2 - width - 1);
> > > > +			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
> > > 
> > > These two calculations are wrong my case. What works here is
> > > 
> > > 	sspsp |= SSPSP_DMYSTOP(
> > > 			(slot_width * slots * chn) / 2 - width - 1);
> > > 	sspsp |= SSPSP_SFRMWDTH((slot_width * slots * chn) / 2);
> > > 
> > > ... which is another multiplication by factor 2. But I'm not sure if I
> > > got the wrong variable with value 2 :)
> > 
> > DMYSTOP itself should be accounted for slot_width, but this will only
> > work if all slots are active otherwise you will get asynchronous SFRM.
> > Something like:
> 
> [...]
> 
> Care to send modified patches? I'd be lucky to test them.

Unfortunately I would not be able to test them at the moment, not even
compile test.. :/

I will send new patches when I'm back to my workstation.

> > This should be able to deal with I2S when pxa is slave, on both pxa2xx
> > and pxa3xx, with set_tdm_slot(5, 5, 4, 16).
> > 
> > Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).
> 
> Again - shouldn't we care for a consistent calling convention here and
> handle the special cases in the implementation rather than feeding
> nebulous integer arguments to it, which vary depending on the clocking
> directions?

Yes, I think we can change the slots configuration in the case we set
DMYSTOP.

> > This still violates the "DMYSTOP must be clear on network mode" rule,
> > but as all slots are active its not really network mode. And it seems to
> > work for you, so... ;)
> 
> Yeah, that makes no sense at all. I think we should ignore that comment.
> It seems to be simply wrong, or we're not in network mode afterall, even
> though we selected it. I don't know.

The comment is right for real network mode, when you want to ignore some
timeslots. In your case, as all slots are active its not really network
mode, its just a way to increase the frame width above the 32b limit.

-- 
Daniel Ribeiro
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Esta =?ISO-8859-1?Q?=E9?= uma parte de mensagem
 assinada digitalmente
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20090817/5b868001/attachment.sig