| Message ID | d3c3f645d39338df0ee9686cb030255ce36a249d.1387535302.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote: > Add machine driver support for BeagleBone-Black and other boards with > tilcdc support and NXP TDA998X HDMI transmitter connected to McASP > port in I2S mode. The 44100 Hz sample-rate and it's multiples can not > be supported on Beaglebone-Black because of limited clock-rate Can the drivers infer this from the clocks? > support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. > The 8 least significant bits are ignored. Where does this constraint come from? > + struct snd_soc_card_drvdata_davinci *drvdata = > + (struct snd_soc_card_drvdata_davinci *) > + snd_soc_card_get_drvdata(soc_card); Again with the casting. > + runtime->hw.rate_min = drvdata->rate_constraint->list[0]; > + runtime->hw.rate_max = drvdata->rate_constraint->list[ > + drvdata->rate_constraint->count - 1]; > + runtime->hw.rates = SNDRV_PCM_RATE_KNOT; > + > + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, > + drvdata->rate_constraint); > + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, > + 2, 2); Why not just set all this statically when registering the DAI? > +static unsigned int evm_get_bclk(struct snd_pcm_hw_params *params) > +{ > + int sample_size = snd_pcm_format_width(params_format(params)); > + int rate = params_rate(params); > + int channels = params_channels(params); > + > + return sample_size * channels * rate; > +} snd_soc_params_to_frame_size(). > +static int evm_tda998x_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; > + struct snd_soc_codec *codec = rtd->codec; > + struct snd_soc_card *soc_card = codec->card; > + struct platform_device *pdev = to_platform_device(soc_card->dev); > + unsigned int bclk_freq = evm_get_bclk(params); > + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) > + snd_soc_card_get_drvdata(soc_card))->sysclk; > + int ret; > + > + ret = snd_soc_dai_set_clkdiv(cpu_dai, 1, sysclk / bclk_freq); > + if (ret < 0) { > + dev_err(&pdev->dev, "can't set CPU DAI clock divider %d\n", > + ret); > + return ret; > + } This looks like something the DAI driver ought to be able to work out for itself based on the clock rate and sample format. > +static unsigned int tda998x_hdmi_rates[] = { > + 32000, > + 44100, > + 48000, > + 88200, > + 96000, > +}; The changelog said that 44.1kHz and its multiples couldn't be supported - is that just the multiples? > +static struct snd_pcm_hw_constraint_list *evm_tda998x_rate_constraint( > + struct snd_soc_card *soc_card) > +{ > + struct platform_device *pdev = to_platform_device(soc_card->dev); > + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) > + snd_soc_card_get_drvdata(soc_card))->sysclk; > + struct snd_pcm_hw_constraint_list *ret; > + unsigned int *rates; > + int i = 0, j = 0; > + > + ret = devm_kzalloc(soc_card->dev, sizeof(*ret) + > + sizeof(tda998x_hdmi_rates), GFP_KERNEL); > + if (!ret) { > + dev_err(&pdev->dev, "Unable to allocate rate constraint!\n"); OOM is already very verbose, don't bother. > + return NULL; > + } > + > + rates = (unsigned int *)&ret[1]; > + ret->list = rates; > + ret->mask = 0; > + for (; i < ARRAY_SIZE(tda998x_hdmi_rates); i++) { This is all very hard to read. Why has the assignment of i been moved up to the declaration rather than put here as is idiomatic, what's all the casting going on with ret and in general?
On 12/31/2013 03:25 PM, Mark Brown wrote: > On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote: > >> Add machine driver support for BeagleBone-Black and other boards with >> tilcdc support and NXP TDA998X HDMI transmitter connected to McASP >> port in I2S mode. The 44100 Hz sample-rate and it's multiples can not >> be supported on Beaglebone-Black because of limited clock-rate > > Can the drivers infer this from the clocks? It does. The commit message is referring to a BBB HW specific feature. Guess I should remove that note from the commit message, since it does not concern the code itself. > >> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. >> The 8 least significant bits are ignored. > > Where does this constraint come from? > From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N register statically to a value that works only with 4 byte samples. According to my tests it is possible to support 3 and 2 byte samples too by changing the CTS_N register value, but I am not sure if the configuration can be changed on the fly. My data sheet of the nxp chip is very vague about the register definitions, but I suppose the register configures some clock divider on the chip. HDMI supports only upto 24bit audio and the data sheet states that any extraneous least significant bits are ignored. >> + struct snd_soc_card_drvdata_davinci *drvdata = >> + (struct snd_soc_card_drvdata_davinci *) >> + snd_soc_card_get_drvdata(soc_card); > > Again with the casting. > I'll fix that. >> + runtime->hw.rate_min = drvdata->rate_constraint->list[0]; >> + runtime->hw.rate_max = drvdata->rate_constraint->list[ >> + drvdata->rate_constraint->count - 1]; >> + runtime->hw.rates = SNDRV_PCM_RATE_KNOT; >> + >> + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, >> + drvdata->rate_constraint); >> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, >> + 2, 2); > > Why not just set all this statically when registering the DAI? Because there is no relevant DAI to where to put these limitations. I did not want to add yet another dummy codec driver, but decided to use the already existing ASoC HDMI codec. By default the driver support all audio params supported by HDMI. The limitations are coming from NXP chip, the NXP driver, and because the chip is used in i2s mode. In other words the limitation is coming from machine setup, not from the DAIs. > >> +static unsigned int evm_get_bclk(struct snd_pcm_hw_params *params) >> +{ >> + int sample_size = snd_pcm_format_width(params_format(params)); >> + int rate = params_rate(params); >> + int channels = params_channels(params); >> + >> + return sample_size * channels * rate; >> +} > > snd_soc_params_to_frame_size(). > Rather snd_soc_params_to_bclk(), but thanks. I'll use that. >> +static int evm_tda998x_hw_params(struct snd_pcm_substream *substream, >> + struct snd_pcm_hw_params *params) >> +{ >> + struct snd_soc_pcm_runtime *rtd = substream->private_data; >> + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; >> + struct snd_soc_codec *codec = rtd->codec; >> + struct snd_soc_card *soc_card = codec->card; >> + struct platform_device *pdev = to_platform_device(soc_card->dev); >> + unsigned int bclk_freq = evm_get_bclk(params); >> + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) >> + snd_soc_card_get_drvdata(soc_card))->sysclk; >> + int ret; >> + >> + ret = snd_soc_dai_set_clkdiv(cpu_dai, 1, sysclk / bclk_freq); >> + if (ret < 0) { >> + dev_err(&pdev->dev, "can't set CPU DAI clock divider %d\n", >> + ret); >> + return ret; >> + } > > This looks like something the DAI driver ought to be able to work out > for itself based on the clock rate and sample format. > I guess that could be done. Peter, what do you say if I set BCLK divider automatically if mcasp set_sysclk() has been called with SND_SOC_CLOCK_IN? >> +static unsigned int tda998x_hdmi_rates[] = { >> + 32000, >> + 44100, >> + 48000, >> + 88200, >> + 96000, >> +}; > > The changelog said that 44.1kHz and its multiples couldn't be supported > - is that just the multiples? > As I mentioned earlier, that is a BBB HW limitation only, the code bellow is able to decide what rates are available based on the sysclk rate. >> +static struct snd_pcm_hw_constraint_list *evm_tda998x_rate_constraint( >> + struct snd_soc_card *soc_card) >> +{ >> + struct platform_device *pdev = to_platform_device(soc_card->dev); >> + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) >> + snd_soc_card_get_drvdata(soc_card))->sysclk; >> + struct snd_pcm_hw_constraint_list *ret; >> + unsigned int *rates; >> + int i = 0, j = 0; >> + >> + ret = devm_kzalloc(soc_card->dev, sizeof(*ret) + >> + sizeof(tda998x_hdmi_rates), GFP_KERNEL); >> + if (!ret) { >> + dev_err(&pdev->dev, "Unable to allocate rate constraint!\n"); > > OOM is already very verbose, don't bother. > Ok, I'll remove that. >> + return NULL; >> + } >> + >> + rates = (unsigned int *)&ret[1]; >> + ret->list = rates; >> + ret->mask = 0; >> + for (; i < ARRAY_SIZE(tda998x_hdmi_rates); i++) { > > This is all very hard to read. Why has the assignment of i been moved > up to the declaration rather than put here as is idiomatic, what's all > the casting going on with ret and in general? > No excuse for i initialization, I'll fix that. The casting is just to survive with just one kmalloc call instead of separate memory blobs for struct snd_pcm_hw_constraint_list and referred list of supported sample rates. I'll allocate a second blob, if that is easier to read. Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
15.01.2014 13:27, Jyri Sarha kirjoitti: > On 12/31/2013 03:25 PM, Mark Brown wrote: >> On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote: >>> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. >>> The 8 least significant bits are ignored. >> >> Where does this constraint come from? >> > > From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N > register statically to a value that works only with 4 byte samples. > According to my tests it is possible to support 3 and 2 byte samples too > by changing the CTS_N register value, but I am not sure if the > configuration can be changed on the fly. My data sheet of the nxp chip > is very vague about the register definitions, but I suppose the register > configures some clock divider on the chip. HDMI supports only upto 24bit > audio and the data sheet states that any extraneous least significant > bits are ignored. That sounds strange, CTS/N values only depend on audio sample rate and TMDS/video clock, not on the audio format or the size of samples (HDMI spec sec 7.2 - Audio Sample Clock Capture and Regeneration). Sure there isn't anything more going on (like the used HDMI sink being more tolerant to wrong CTS/N with 4-byte samples, or more likely something else I can't immediately think of)? BTW, radeon driver has some code that calculates static CTS/N values since the hw autogeneration is not reliable there: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/r600_hdmi.c Not that it directly helps now, but just for the record...
On Wed, 15 Jan 2014 13:27:21 +0200 Jyri Sarha <jsarha@ti.com> wrote: > From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N > register statically to a value that works only with 4 byte samples. > According to my tests it is possible to support 3 and 2 byte samples too > by changing the CTS_N register value, but I am not sure if the > configuration can be changed on the fly. My data sheet of the nxp chip > is very vague about the register definitions, but I suppose the register > configures some clock divider on the chip. HDMI supports only upto 24bit > audio and the data sheet states that any extraneous least significant > bits are ignored. In the tda998x driver, the CTS_N is automatic (AIP_CNTRL_0_ACR_MAN is not set). Then, in my Cubox (Marvell A510 + tda19988), the 16, 24 and 32 bits formats are working well with I2S input at any rate.
On 01/15/2014 03:48 PM, Anssi Hannula wrote: > 15.01.2014 13:27, Jyri Sarha kirjoitti: >> On 12/31/2013 03:25 PM, Mark Brown wrote: >>> On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote: >>>> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. >>>> The 8 least significant bits are ignored. >>> >>> Where does this constraint come from? >>> >> >> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N >> register statically to a value that works only with 4 byte samples. >> According to my tests it is possible to support 3 and 2 byte samples too >> by changing the CTS_N register value, but I am not sure if the >> configuration can be changed on the fly. My data sheet of the nxp chip >> is very vague about the register definitions, but I suppose the register >> configures some clock divider on the chip. HDMI supports only upto 24bit >> audio and the data sheet states that any extraneous least significant >> bits are ignored. > > That sounds strange, CTS/N values only depend on audio sample rate and > TMDS/video clock, not on the audio format or the size of samples (HDMI > spec sec 7.2 - Audio Sample Clock Capture and Regeneration). > > Sure there isn't anything more going on (like the used HDMI sink being > more tolerant to wrong CTS/N with 4-byte samples, or more likely > something else I can't immediately think of)? > On theoretical level I am not really sure about anything, because have not been able to get my hands on proper NXP TDA19988 documentation. However, while playing around with my Beaglebone-Black I have have found out in practice that by changing CTS_N register's (page 0x11 reg 0x0c) K_SEL bits (bit 2 to 0)[1] I can get 2, 3 and 4 byte samples to work consistently by setting: K_SEL = 1 for SNDRV_PCM_FORMAT_S16_LE K_SEL = 2 for SNDRV_PCM_FORMAT_S24_3LE K_SEL = 3 for SNDRV_PCM_FORMAT_S32_LE Because I do not really know what is going on, I did not want to suggest any changes to the driver and just use the format that works with the current driver version. The HDMI sinks I have been using are a Toshiba 22B2LF1G and Thomson 42E90NF32 televisions. About those I just know that they both behave the same way. Best regards, Jyri [1] The closest data sheet that has any description about the chip registers I have found is this: http://media.digikey.com/pdf/Data%20Sheets/NXP%20PDFs/TDA9983B.pdf -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 15, 2014 at 01:27:21PM +0200, Jyri Sarha wrote: > On 12/31/2013 03:25 PM, Mark Brown wrote: > >>support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. > >>The 8 least significant bits are ignored. > >Where does this constraint come from? > From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N > register statically to a value that works only with 4 byte samples. > According to my tests it is possible to support 3 and 2 byte samples > too by changing the CTS_N register value, but I am not sure if the > configuration can be changed on the fly. My data sheet of the nxp > chip is very vague about the register definitions, but I suppose the > register configures some clock divider on the chip. HDMI supports > only upto 24bit audio and the data sheet states that any extraneous > least significant bits are ignored. Hrm. This sounds like it ought to be presenting as an ASoC CODEC driver. > >>+ snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, > >>+ 2, 2); > >Why not just set all this statically when registering the DAI? > Because there is no relevant DAI to where to put these limitations. > I did not want to add yet another dummy codec driver, but decided to > use the already existing ASoC HDMI codec. By default the driver > support all audio params supported by HDMI. The limitations are > coming from NXP chip, the NXP driver, and because the chip is used > in i2s mode. In other words the limitation is coming from machine > setup, not from the DAIs. OK, but it sounds like it's got register configuration as well? That really does sound like a device that ought to have a driver... > No excuse for i initialization, I'll fix that. The casting is just > to survive with just one kmalloc call instead of separate memory > blobs for > struct snd_pcm_hw_constraint_list and referred list of supported > sample rates. I'll allocate a second blob, if that is easier to > read. Yes, please. Unless it's something where memory is getting tight (eg, due to be allocated a lot) it's more important that we can read the code than that we save a few bytes.
On 01/15/2014 05:51 PM, Jean-Francois Moine wrote: > On Wed, 15 Jan 2014 13:27:21 +0200 > Jyri Sarha <jsarha@ti.com> wrote: > >> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N >> register statically to a value that works only with 4 byte samples. >> According to my tests it is possible to support 3 and 2 byte samples too >> by changing the CTS_N register value, but I am not sure if the >> configuration can be changed on the fly. My data sheet of the nxp chip >> is very vague about the register definitions, but I suppose the register >> configures some clock divider on the chip. HDMI supports only upto 24bit >> audio and the data sheet states that any extraneous least significant >> bits are ignored. > > In the tda998x driver, the CTS_N is automatic (AIP_CNTRL_0_ACR_MAN is > not set). > > Then, in my Cubox (Marvell A510 + tda19988), the 16, 24 and 32 bits > formats are working well with I2S input at any rate. > Could you refer the kernel version (main line?) and the involved ASoC drivers so could take I a look if there is something I could do differently? Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 22 Jan 2014 11:20:32 +0200 Jyri Sarha <jsarha@ti.com> wrote: > On 01/15/2014 05:51 PM, Jean-Francois Moine wrote: > > On Wed, 15 Jan 2014 13:27:21 +0200 > > Jyri Sarha <jsarha@ti.com> wrote: > > > >> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N > >> register statically to a value that works only with 4 byte samples. > >> According to my tests it is possible to support 3 and 2 byte samples too > >> by changing the CTS_N register value, but I am not sure if the > >> configuration can be changed on the fly. My data sheet of the nxp chip > >> is very vague about the register definitions, but I suppose the register > >> configures some clock divider on the chip. HDMI supports only upto 24bit > >> audio and the data sheet states that any extraneous least significant > >> bits are ignored. > > > > In the tda998x driver, the CTS_N is automatic (AIP_CNTRL_0_ACR_MAN is > > not set). > > > > Then, in my Cubox (Marvell A510 + tda19988), the 16, 24 and 32 bits > > formats are working well with I2S input at any rate. > > > > Could you refer the kernel version (main line?) and the involved ASoC > drivers so could take I a look if there is something I could do differently? Both drivers are in the kernel main line. The ASoC is in sound/soc/kirkwood/. kirkwood-i2s.c defines the DAIs I2S and S/PDIF outputs. As both I2S and S/PDIF may be used for HDMI output in the Cubox, I wrote a tda998x CODEC which gets the audio ports from the DT and dynamically sets these ports and the audio type (i2s / spdif) on audio streaming start. I have not submitted yet this codec and the associated changes in tda998x, because I am waiting for a first patch series on the tda998x to be applied (http://lists.freedesktop.org/archives/dri-devel/2014-January/051552.html).
On Wed, 22 Jan 2014 11:19:53 +0100 Jean-Francois Moine <moinejf@free.fr> wrote: > As both I2S and S/PDIF may be used for HDMI output in the Cubox, > I wrote a tda998x CODEC which gets the audio ports from the DT and > dynamically sets these ports and the audio type (i2s / spdif) on audio > streaming start. I have not submitted yet this codec and the associated > changes in tda998x, because I am waiting for a first patch series on the > tda998x to be applied > (http://lists.freedesktop.org/archives/dri-devel/2014-January/051552.html). Sorry, the last patch request is http://lists.freedesktop.org/archives/dri-devel/2014-January/052201.html
On 01/22/2014 12:46 PM, Jean-Francois Moine wrote: > On Wed, 22 Jan 2014 11:19:53 +0100 > Jean-Francois Moine <moinejf@free.fr> wrote: > >> As both I2S and S/PDIF may be used for HDMI output in the Cubox, >> I wrote a tda998x CODEC which gets the audio ports from the DT and >> dynamically sets these ports and the audio type (i2s / spdif) on audio >> streaming start. I have not submitted yet this codec and the associated >> changes in tda998x, because I am waiting for a first patch series on the >> tda998x to be applied >> (http://lists.freedesktop.org/archives/dri-devel/2014-January/051552.html). > > Sorry, the last patch request is > > http://lists.freedesktop.org/archives/dri-devel/2014-January/052201.html > I checked the sample format support again with and without your patches. On my other TV all the formats produce something that sounds Ok if you do not listen too carefully or use sine sweep etc. make the short comings audible. The other TV is completely silent when playing the "non supported" formats. Could you give me a link to a git repo with your tda998x codec code so I could prepare to use that too? Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/21/2014 09:15 PM, Mark Brown wrote: > On Wed, Jan 15, 2014 at 01:27:21PM +0200, Jyri Sarha wrote: >> On 12/31/2013 03:25 PM, Mark Brown wrote: > >>>> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. >>>> The 8 least significant bits are ignored. > >>> Where does this constraint come from? > >> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N >> register statically to a value that works only with 4 byte samples. >> According to my tests it is possible to support 3 and 2 byte samples >> too by changing the CTS_N register value, but I am not sure if the >> configuration can be changed on the fly. My data sheet of the nxp >> chip is very vague about the register definitions, but I suppose the >> register configures some clock divider on the chip. HDMI supports >> only upto 24bit audio and the data sheet states that any extraneous >> least significant bits are ignored. > > Hrm. This sounds like it ought to be presenting as an ASoC CODEC > driver. > I do not disagree. I just do no not have a clear understanding how something like that should be done. Either the tda998x_drv it self should provide the ASoC codec driver or there should be some kind of API that could be accessed by some driver under sound/soc/codecs. Anyway it sound like Jean-Francois Moine is already doing that. I'll take his driver into use as soon as his code is merged. However, for now a simple implementation that I have does not really need any interaction with the HDMI encoder and thus no codec driver either. >>>> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, >>>> + 2, 2); > >>> Why not just set all this statically when registering the DAI? > >> Because there is no relevant DAI to where to put these limitations. >> I did not want to add yet another dummy codec driver, but decided to >> use the already existing ASoC HDMI codec. By default the driver >> support all audio params supported by HDMI. The limitations are >> coming from NXP chip, the NXP driver, and because the chip is used >> in i2s mode. In other words the limitation is coming from machine >> setup, not from the DAIs. > > OK, but it sounds like it's got register configuration as well? That > really does sound like a device that ought to have a driver... > Sure, but it would not save form making runtime constraints. The usage of i2s mode, which limits the number of channels, is selected by dai_fmt call after dai registering. Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 24 Jan 2014 14:57:09 +0200 Jyri Sarha <jsarha@ti.com> wrote: > Could you give me a link to a git repo with your tda998x codec code so I > could prepare to use that too? I have no git repo. Instead, here is the source. It goes into sound/soc/codecs and is selected with CONFIG_OF and CONFIG_DRM_I2C_NXP_TDA998X. It needs 2 exported functions of the tda998x driver: the first one to select the audio port (tda998x_audio_update) and the other one to get the ELD (tda998x_audio_get_eld). For these functions to receive the correct i2c_client, the codec searches the tda998x driver in the DT by its compatible "nxp,tda998x". If the tda998x is loaded by drm_i2c_encoder_init(), it should not be declared in the DT, so, this raises a problem. I don't know what must be done in this case. The codec is used with the simple card as the sound card. For you, the DT could be: hdmi_codec: hdmi-codec { compatible = "nxp,tda998x-codec"; #sound-dai-cells = <1>; audio-ports = <0x03>, <0x04>; /* i2s - s/pdif */ }; sound { compatible = "simple-audio-card"; simple-audio-card,cpu { sound-dai = <&audio1 1>; format = "i2s"; }; simple-audio-card,codec { sound-dai = <&hdmi_codec 0>; /* i2s */ }; }; ('audio1' is the audio controller) -------------------8<---------------- source sound/soc/codecs/tda998x.c /* * ALSA SoC TDA998X driver * * This driver is used by the NXP TDA998x HDMI transmitter. * * Copyright (C) 2014 Jean-Francois Moine * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ #include <linux/module.h> #include <sound/soc.h> #include <sound/pcm.h> #include <linux/of.h> #include <linux/i2c.h> #include <drm/drm_encoder_slave.h> #include <drm/i2c/tda998x.h> #define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \ SNDRV_PCM_FMTBIT_S20_3LE | \ SNDRV_PCM_FMTBIT_S24_LE | \ SNDRV_PCM_FMTBIT_S32_LE) struct tda_priv { struct i2c_client *i2c_client; struct snd_soc_codec *codec; u32 ports[2]; int dai_id; u8 *eld; }; static void tda_get_encoder(struct tda_priv *priv) { struct snd_soc_codec *codec = priv->codec; struct device_node *np; struct i2c_client *i2c_client; static const struct of_device_id tda_dt[] = { { .compatible = "nxp,tda998x" }, { }, }; /* search the tda998x device (only one!) */ np = of_find_matching_node_and_match(NULL, tda_dt, NULL); if (!np || !of_device_is_available(np)) { dev_err(codec->dev, "No tda998x in DT\n"); return; } i2c_client = of_find_i2c_device_by_node(np); of_node_put(np); if (!i2c_client) { dev_err(codec->dev, "no tda998x i2c client\n"); return; } if (!i2c_get_clientdata(i2c_client)) { dev_err(codec->dev, "tda998x not initialized\n"); return; } priv->i2c_client = i2c_client; } static int tda_start_stop(struct tda_priv *priv, int start) { int format; u32 port; if (!priv->i2c_client) { tda_get_encoder(priv); if (!priv->i2c_client) return -EINVAL; } /* give the audio input type and ports to the HDMI encoder */ format = start ? priv->dai_id : 0; switch (format) { case AFMT_I2S: port = priv->ports[0]; break; default: case AFMT_SPDIF: port = priv->ports[1]; break; } tda998x_audio_update(priv->i2c_client, format, port); return 0; } static int tda_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec); u8 *eld = NULL; static unsigned rates_mask[] = { SNDRV_PCM_RATE_32000, SNDRV_PCM_RATE_44100, SNDRV_PCM_RATE_48000, SNDRV_PCM_RATE_88200, SNDRV_PCM_RATE_96000, SNDRV_PCM_RATE_176400, SNDRV_PCM_RATE_192000, }; /* memorize the used DAI */ priv->dai_id = dai->id; /* get the ELD from the tda998x driver */ if (!priv->i2c_client) tda_get_encoder(priv); if (priv->i2c_client) eld = tda998x_audio_get_eld(priv->i2c_client); /* limit the hw params from the ELD (EDID) */ if (eld) { struct snd_soc_dai_driver *dai_drv = dai->driver; struct snd_soc_pcm_stream *stream = &dai_drv->playback; u8 *sad; int sad_count; unsigned eld_ver, mnl, rates, rate_mask, i; unsigned max_channels; eld_ver = eld[0] >> 3; if (eld_ver != 2 && eld_ver != 31) return 0; mnl = eld[4] & 0x1f; if (mnl > 16) return 0; sad_count = eld[5] >> 4; sad = eld + 20 + mnl; /* Start from the basic audio settings */ max_channels = 2; rates = 0; while (sad_count--) { switch (sad[0] & 0x78) { case 0x08: /* PCM */ max_channels = max(max_channels, (sad[0] & 7) + 1u); rates |= sad[1]; break; } sad += 3; } for (rate_mask = i = 0; i < ARRAY_SIZE(rates_mask); i++) if (rates & 1 << i) rate_mask |= rates_mask[i]; /* change the snd_soc_pcm_stream values of the driver */ stream->rates = rate_mask; stream->channels_max = max_channels; } /* start the TDA998x audio */ return tda_start_stop(priv, 1); } static void tda_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec); priv->dai_id = 0; tda_start_stop(priv, 0); } static const struct snd_soc_dai_ops tda_ops = { .startup = tda_startup, .shutdown = tda_shutdown, }; static const struct snd_soc_dai_driver tda998x_dai[] = { { .name = "i2s-hifi", .id = AFMT_I2S, .playback = { .stream_name = "HDMI I2S Playback", .channels_min = 1, .channels_max = 8, .rates = SNDRV_PCM_RATE_CONTINUOUS, .rate_min = 5512, .rate_max = 192000, .formats = TDA998X_FORMATS, }, .ops = &tda_ops, }, { .name = "spdif-hifi", .id = AFMT_SPDIF, .playback = { .stream_name = "HDMI SPDIF Playback", .channels_min = 1, .channels_max = 2, .rates = SNDRV_PCM_RATE_CONTINUOUS, .rate_min = 22050, .rate_max = 192000, .formats = TDA998X_FORMATS, }, .ops = &tda_ops, }, }; static const struct snd_soc_dapm_widget tda_widgets[] = { SND_SOC_DAPM_OUTPUT("hdmi-out"), }; static const struct snd_soc_dapm_route tda_routes[] = { { "hdmi-out", NULL, "HDMI I2S Playback" }, { "hdmi-out", NULL, "HDMI SPDIF Playback" }, }; static int tda_probe(struct snd_soc_codec *codec) { struct tda_priv *priv; struct device_node *np; int i, ret; priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; snd_soc_codec_set_drvdata(codec, priv); priv->codec = codec; /* get the audio input ports (I2s and S/PDIF) */ np = codec->dev->of_node; for (i = 0; i < 2; i++) { ret = of_property_read_u32_index(np, "audio-ports", i, &priv->ports[i]); if (ret) { if (i == 0) dev_err(codec->dev, "bad or missing audio-ports\n"); break; } } return 0; } static const struct snd_soc_codec_driver soc_codec_tda998x = { .probe = tda_probe, .dapm_widgets = tda_widgets, .num_dapm_widgets = ARRAY_SIZE(tda_widgets), .dapm_routes = tda_routes, .num_dapm_routes = ARRAY_SIZE(tda_routes), }; static int tda998x_dev_probe(struct platform_device *pdev) { struct snd_soc_dai_driver *dai_drv; /* 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)); return snd_soc_register_codec(&pdev->dev, &soc_codec_tda998x, dai_drv, ARRAY_SIZE(tda998x_dai)); } static int tda998x_dev_remove(struct platform_device *pdev) { snd_soc_unregister_codec(&pdev->dev); return 0; } static const struct of_device_id tda998x_codec_ids[] = { { .compatible = "nxp,tda998x-codec", }, { } }; MODULE_DEVICE_TABLE(of, tda998x_codec_ids); static struct platform_driver tda998x_driver = { .probe = tda998x_dev_probe, .remove = tda998x_dev_remove, .driver = { .name = "tda998x-codec", .owner = THIS_MODULE, .of_match_table = tda998x_codec_ids, }, }; module_platform_driver(tda998x_driver); MODULE_AUTHOR("Jean-Francois Moine"); MODULE_DESCRIPTION("TDA998x codec driver"); MODULE_LICENSE("GPL"); -------------------8<----------------
diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt index 4aa00f6..f1e1031 100644 --- a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt @@ -1,7 +1,9 @@ * Texas Instruments SoC audio setups with TLV320AIC3X Codec Required properties: -- compatible : "ti,da830-evm-audio" : forDM365/DA8xx/OMAPL1x/AM33xx +- compatible : + "ti,da830-evm-audio" : for DM365/DA8xx/OMAPL1x/AM33xx + "ti,am33xx-beaglebone-black-audio" : for Beaglebone-black HDMI audio - ti,model : The user-visible name of this sound complex. - ti,audio-codec : The phandle of the TLV320AIC3x audio codec - ti,mcasp-controller : The phandle of the McASP controller diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index b28c9fd..3d3138d 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -21,6 +21,7 @@ #include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> +#include <sound/pcm_params.h> #include <asm/dma.h> #include <asm/mach-types.h> @@ -33,8 +34,13 @@ struct snd_soc_card_drvdata_davinci { struct clk *mclk; unsigned sysclk; + struct snd_pcm_hw_constraint_list *rate_constraint; }; +/* If changing sample format the tda998x configuration (REG_CTS_N) needs + to be changed. */ +#define TDA998X_SAMPLE_FORMAT SNDRV_PCM_FORMAT_S32_LE + static int evm_startup(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -80,12 +86,80 @@ static int evm_hw_params(struct snd_pcm_substream *substream, return 0; } +static int evm_tda998x_startup(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + (struct snd_soc_card_drvdata_davinci *) + snd_soc_card_get_drvdata(soc_card); + struct snd_mask *fmt = constrs_mask(&runtime->hw_constraints, + SNDRV_PCM_HW_PARAM_FORMAT); + snd_mask_none(fmt); + snd_mask_set(fmt, TDA998X_SAMPLE_FORMAT); + + runtime->hw.rate_min = drvdata->rate_constraint->list[0]; + runtime->hw.rate_max = drvdata->rate_constraint->list[ + drvdata->rate_constraint->count - 1]; + runtime->hw.rates = SNDRV_PCM_RATE_KNOT; + + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, + drvdata->rate_constraint); + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, + 2, 2); + + return evm_startup(substream); +} + +static unsigned int evm_get_bclk(struct snd_pcm_hw_params *params) +{ + int sample_size = snd_pcm_format_width(params_format(params)); + int rate = params_rate(params); + int channels = params_channels(params); + + return sample_size * channels * rate; +} + +static int evm_tda998x_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_codec *codec = rtd->codec; + struct snd_soc_card *soc_card = codec->card; + struct platform_device *pdev = to_platform_device(soc_card->dev); + unsigned int bclk_freq = evm_get_bclk(params); + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) + snd_soc_card_get_drvdata(soc_card))->sysclk; + int ret; + + ret = snd_soc_dai_set_clkdiv(cpu_dai, 1, sysclk / bclk_freq); + if (ret < 0) { + dev_err(&pdev->dev, "can't set CPU DAI clock divider %d\n", + ret); + return ret; + } + + ret = snd_soc_dai_set_sysclk(cpu_dai, 0, sysclk, SND_SOC_CLOCK_IN); + if (ret < 0) + return ret; + + return ret; +} + static struct snd_soc_ops evm_ops = { .startup = evm_startup, .shutdown = evm_shutdown, .hw_params = evm_hw_params, }; +static struct snd_soc_ops evm_tda998x_ops = { + .startup = evm_tda998x_startup, + .shutdown = evm_shutdown, + .hw_params = evm_tda998x_hw_params, +}; + /* davinci-evm machine dapm widgets */ static const struct snd_soc_dapm_widget aic3x_dapm_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), @@ -152,6 +226,81 @@ static int evm_aic3x_init(struct snd_soc_pcm_runtime *rtd) return 0; } +static unsigned int tda998x_hdmi_rates[] = { + 32000, + 44100, + 48000, + 88200, + 96000, +}; + +static struct snd_pcm_hw_constraint_list *evm_tda998x_rate_constraint( + struct snd_soc_card *soc_card) +{ + struct platform_device *pdev = to_platform_device(soc_card->dev); + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) + snd_soc_card_get_drvdata(soc_card))->sysclk; + struct snd_pcm_hw_constraint_list *ret; + unsigned int *rates; + int i = 0, j = 0; + + ret = devm_kzalloc(soc_card->dev, sizeof(*ret) + + sizeof(tda998x_hdmi_rates), GFP_KERNEL); + if (!ret) { + dev_err(&pdev->dev, "Unable to allocate rate constraint!\n"); + return NULL; + } + + rates = (unsigned int *)&ret[1]; + ret->list = rates; + ret->mask = 0; + for (; i < ARRAY_SIZE(tda998x_hdmi_rates); i++) { + unsigned int bclk_freq = tda998x_hdmi_rates[i] * 2 * + snd_pcm_format_width(TDA998X_SAMPLE_FORMAT); + if (sysclk % bclk_freq == 0) { + rates[j++] = tda998x_hdmi_rates[i]; + dev_dbg(soc_card->dev, "Allowing rate %u\n", + tda998x_hdmi_rates[i]); + } + } + ret->count = j; + return ret; +} + +static const struct snd_soc_dapm_widget tda998x_dapm_widgets[] = { + SND_SOC_DAPM_OUTPUT("HDMI Out"), +}; + +static int evm_tda998x_init(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dapm_context *dapm = &rtd->codec->dapm; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + (struct snd_soc_card_drvdata_davinci *) + snd_soc_card_get_drvdata(soc_card); + int ret; + + ret = snd_soc_dai_set_clkdiv(cpu_dai, 0, 1); + if (ret < 0) + return ret; + + drvdata->rate_constraint = evm_tda998x_rate_constraint(soc_card); + + snd_soc_dapm_new_controls(dapm, tda998x_dapm_widgets, + ARRAY_SIZE(tda998x_dapm_widgets)); + + ret = snd_soc_of_parse_audio_routing(soc_card, "ti,audio-routing"); + + /* not connected */ + snd_soc_dapm_disable_pin(dapm, "RX"); + + /* always connected */ + snd_soc_dapm_enable_pin(dapm, "HDMI Out"); + + return 0; +} + /* davinci-evm digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link dm6446_evm_dai = { .name = "TLV320AIC3X", @@ -337,7 +486,7 @@ static struct snd_soc_card da850_snd_soc_card = { #if defined(CONFIG_OF) /* - * The struct is used as place holder. It will be completely + * The structs are used as place holders. They will be completely * filled with data from dt node. */ static struct snd_soc_dai_link evm_dai_tlv320aic3x = { @@ -350,10 +499,24 @@ static struct snd_soc_dai_link evm_dai_tlv320aic3x = { SND_SOC_DAIFMT_IB_NF, }; +static struct snd_soc_dai_link evm_dai_tda998x_hdmi = { + .name = "NXP TDA998x HDMI Chip", + .stream_name = "HDMI", + .codec_dai_name = "hdmi-hifi", + .ops = &evm_tda998x_ops, + .init = evm_tda998x_init, + .dai_fmt = (SND_SOC_DAIFMT_CBS_CFS | SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_IB_NF), +}; + static const struct of_device_id davinci_evm_dt_ids[] = { { .compatible = "ti,da830-evm-audio", - .data = (void *) &evm_dai_tlv320aic3x, + .data = &evm_dai_tlv320aic3x, + }, + { + .compatible = "ti,am33xx-beaglebone-black-audio", + .data = &evm_dai_tda998x_hdmi, }, { /* sentinel */ } };
Add machine driver support for BeagleBone-Black and other boards with tilcdc support and NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. The 44100 Hz sample-rate and it's multiples can not be supported on Beaglebone-Black because of limited clock-rate support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE. The 8 least significant bits are ignored. Signed-off-by: Jyri Sarha <jsarha@ti.com> cc: bcousson@baylibre.com --- .../bindings/sound/davinci-evm-audio.txt | 4 +- sound/soc/davinci/davinci-evm.c | 167 +++++++++++++++++++- 2 files changed, 168 insertions(+), 3 deletions(-)