From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Mukunda,Vijendar" Subject: Re: [PATCH 3/9] ASoC: amd: dma driver changes for BT I2S controller instance Date: Fri, 23 Feb 2018 12:31:26 +0530 Message-ID: References: <1518766434-7911-1-git-send-email-Vijendar.Mukunda@amd.com> <1518766434-7911-4-git-send-email-Vijendar.Mukunda@amd.com> <20180219164525.GJ32761@sirena.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from NAM01-BN3-obe.outbound.protection.outlook.com (mail-bn3nam01on0041.outbound.protection.outlook.com [104.47.33.41]) by alsa0.perex.cz (Postfix) with ESMTP id 5A9802671D2 for ; Fri, 23 Feb 2018 07:59:51 +0100 (CET) In-Reply-To: <20180219164525.GJ32761@sirena.org.uk> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Mark Brown Cc: alsa-devel@alsa-project.org, tiwai@suse.de, lgirdwood@gmail.com, Akshu Agrawal , Alexander.Deucher@amd.com List-Id: alsa-devel@alsa-project.org 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.