On Mon, Mar 4, 2013 at 8:55 AM, Daniel Mack zonque@gmail.com wrote:
Hi Vaibhav, Hi Yegor,
On 04.03.2013 07:22, Bedia, Vaibhav wrote:
On Fri, Mar 01, 2013 at 16:23:53, Yegor Yefremov wrote:
I've solved the problem with DIT/SPDIF mode (see the issue description here: http://e2e.ti.com/support/arm/sitara_arm/f/791/p/247447/870030.aspx).
In davinci_mcasp_hw_params() the DIT or I2S params will be set in the beginning. DIT mode configures the DAVINCI_MCASP_TXMASK_REG and DAVINCI_MCASP_TXFMT_REG.
And here comes the problem:
at the end of davinci_mcasp_hw_params() the davinci_config_channel_size() will touch the same registers again and thus overwrite the settings necessary for DIT. After I commented this routine I got the sound over S/PDIF and sii9022a HDMI transmitter and I could see the proper bits appearing on my oscilloscope.
What were the best way to solve this problem?
- execute davinci_config_channel_size() only if not in DIT mode?
- for DIT only change the DAVINCI_MCASP_TXMASK_REG according to channel width?
- execute
if (dev->op_mode == DAVINCI_MCASP_DIT_MODE) davinci_hw_dit_param(dev); else davinci_hw_param(dev, substream->stream);
after davinci_config_channel_size()
AFAIK DIT mode was working on Davinci platforms some time back. Since AM335x has the same hardware block I was surprised to see this bug report. Not having a setup handy to test out DIT related changes, I looked at the commits on the mcasp file to figure out what happened. I suspect one of the recent patches which added 24bit support inadvertently broke the DIT support. Would it be possible for you to do a git-bisect to find out what change it was? It would be good to reference that change in the final patch.
I agree, but I also believe it's not easy to do, given the number of different trees you need for AM33xx support in general.
You could revert them one-by-one, but then again, you probably need most of them in order to make the driver work on AM33xx and in setups that differ from what the davici driver was originally written for.
Coming to the 3 options that you have listed I would suggest going with #1. DIT mode requires the configuration to be 32 bit slot with rotation set to 24 but looks like the current implementation of davinci_config_channel_size() doesn't handle this scenario very well.
Hmm, if that's the case, I fail to see how the 24-bit patches you mentioned could possibly account for that, given that they don't touch the logical around this area. Unless I miss anything very obvious :) Can you be a bit more specific about which of these patches you suspect?
I think the DIT support was dead on arrival ;-) Look at the initial commit: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/sound/...
Size adjustment is made after DIT mode settings and though davinci_config_channel_size(dev, word_length) doesn't overwrite all needed settings, it overwrites XSSZ value and thus disables 32-bit slot. Do I see it right?
I think the driver needs a bit of rework to ensure Tx and Rx paths don't end up overwriting the configuration but that needs to be taken up separately.
Yegor - could you send me a patch that fixes the problem for you? I can do regression tests on my platform.
I'll do.
Yegor