Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: UDMcF1IBjSbeVvBlcSTVrP0XEhstyDZgsrRnJxbv68wp9UEYViKAH+Fi+PKi+zRd17AneCZWEO
 lSmgM+bvW2iQ==
IronPort-SDR: 3221SinK08aDM3DBF4iZ1zeOb2Dw1HuzNYvFcqfiP5278XKel9wUEhTOKsl79dqCj69wLyapBb
 TRFE+iC8pR/w==
Subject: Re: [PATCH 07/11] soundwire: intel: Only call sdw stream APIs for the
 first cpu_dai
To:     Vinod Koul <vkoul@kernel.org>, "Liao, Bard" <bard.liao@intel.com>
Cc:     "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        "tiwai@suse.de" <tiwai@suse.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "ranjani.sridharan@linux.intel.com" 
        <ranjani.sridharan@linux.intel.com>,
        "hui.wang@canonical.com" <hui.wang@canonical.com>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "srinivas.kandagatla@linaro.org" <srinivas.kandagatla@linaro.org>,
        "jank@cadence.com" <jank@cadence.com>,
        "Lin, Mengdong" <mengdong.lin@intel.com>,
        "Kale, Sanyog R" <sanyog.r.kale@intel.com>,
        Bard Liao <yung-chuan.liao@linux.intel.com>,
        "rander.wang@linux.intel.com" <rander.wang@linux.intel.com>
References: <20200818024120.20721-1-yung-chuan.liao@linux.intel.com>
 <20200818024120.20721-8-yung-chuan.liao@linux.intel.com>
 <20200826094636.GB2639@vkoul-mobl>
 <DM6PR11MB407494AEF6D05EFF627CFAFBFF520@DM6PR11MB4074.namprd11.prod.outlook.com>
 <20200828074544.GM2639@vkoul-mobl>
From:   Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-ID: <9eac0f85-1b0f-d2c2-6d1b-15b77707c9b7@linux.intel.com>
Date:   Fri, 28 Aug 2020 10:07:38 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200828074544.GM2639@vkoul-mobl>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 8/28/20 2:45 AM, Vinod Koul wrote:
> On 28-08-20, 01:47, Liao, Bard wrote:
>>> snd_pcm_substream *substream,
>>>>   			goto err;
>>>>   	}
>>>>
>>>> -	ret = sdw_prepare_stream(dma->stream);
>>>> +	/*
>>>> +	 * All cpu dais belong to a stream. To ensure sdw_prepare_stream
>>>> +	 * is called once per stream, we should call it only when
>>>> +	 * dai = first_cpu_dai.
>>>> +	 */
>>>> +	if (first_cpu_dai == dai)
>>>> +		ret = sdw_prepare_stream(dma->stream);
>>>
>>> Hmmm why not use the one place which is unique in the card to call this,
>>> hint machine dais are only called once.
>>
>> Yes, we can call it in machine driver. But, shouldn't it belong to platform
>> level? The point is that if we move the stuff to machine driver, it will
>> force people to implement these stuff on their own Intel machine driver.
> 
> nothing stops anyone from doing that right! machine driver is another
> component so it can be moved there as well

What Bard is saying is that there is nothing board-specific here. This 
is platform-driver code that is independent of the actual machine 
configuration.

Machine drivers can be board-specific, so we would have to add the code 
for prepare/deprepare/trigger to every machine driver.

Today it's true that we worked to have a single machine driver for all 
SoundWire-based devices, so the change is a 1:1 move, but we can't 
guarantee that this would be the case moving forward. In fact, we *know* 
we will need a different machine driver when we parse platform firmware 
to create the card for SDCA support. So in the end there would be 
duplication of code.

See the code we worked on at 
https://github.com/thesofproject/linux/commit/7322e1d25ce2ec9bb78c6e861919f61e0be7cc0b.patch

it'd really a bit silly to have this generic code in the machine driver.

it would be fine to call a set of helpers called from the machine driver 
dailink, but where would we put these helpers? on the ASoC or SoundWire 
sides?