On Monday 19 February 2018 10:15 PM, Mark Brown wrote:
On Fri, Feb 16, 2018 at 01:03:48PM +0530, Vijendar Mukunda wrote:
Implemented dma driver changes to support BT I2S controller instance.
Some sort of description of the changes would make this a lot easier to review.
We will update the description as mentioned below. With in ACP, There are three I2S controllers can be configured/enabled( I2S SP, I2S MICSP, I2S BT). Default enabled I2S controller instance is I2S SP. This patch provides required changes to support I2S BT controller Instance.
if (strcmp(prtd->cpu_dai->name, "designware-i2s.1.auto") == 0) {adata->i2s_play_instance = I2S_SP_INSTANCE;adata->i2ssp_renderbytescount = 0;}if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {adata->i2s_play_instance = I2S_BT_INSTANCE;adata->i2sbt_renderbytescount = 0;}This strcmp on what looks like an autogenerated DAI name seems a bit fragile, especially given that we just silently accept cases where we fail to match anything. Why are we doing things this way rather than at least using explicitly set names?
ACP DMA Driver implemented based on ACP 2.x stack .It uses Designware I2S controller.
AMD Gpu Driver creates device instances for playback & capture scenarios for both the I2S controller instances.( I2S SP & I2S BT).
It has fixed mapping as mentioned below.
designware-i2s.1.auto cpu dai will be used for I2S SP Instance playback device. designware-i2s.2.auto cpu dai will be used for I2S SP Instance capture device. designware-i2s.3.auto cpu dai will be used for I2S BT Instance playback device. designware-i2s.4.auto cpu dai will be used for I2S BT Instance capture device.
We will add error checks for failure case.
- if (adata->asic_type != CHIP_CARRIZO) {
if (adata->play_i2sbt_stream &&adata->play_i2sbt_stream->runtime) {As ever please use switch statements for quirking, it makes life easier when more variants appear.
There are only two variants (Carrizo & Stoney) based on ACP 2.x stack. Switch statement is not required here as this condition is specific to Carrizo only.