Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [alsa-devel] [RFC PATCH 23/40] soundwire: stream: fix disable
 sequence
To:     Sanyog Kale <sanyog.r.kale@intel.com>
Cc:     alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        tiwai@suse.de, broonie@kernel.org, vkoul@kernel.org,
        gregkh@linuxfoundation.org, jank@cadence.com,
        srinivas.kandagatla@linaro.org, slawomir.blauciak@intel.com
References: <20190725234032.21152-1-pierre-louis.bossart@linux.intel.com>
 <20190725234032.21152-24-pierre-louis.bossart@linux.intel.com>
 <20190805095620.GD22437@buildpc-HP-Z230>
From:   Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-ID: <12799e97-d6e3-5027-a409-0fe37dba86fd@linux.intel.com>
Date:   Mon, 5 Aug 2019 10:33:25 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190805095620.GD22437@buildpc-HP-Z230>
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/5/19 4:56 AM, Sanyog Kale wrote:
> On Thu, Jul 25, 2019 at 06:40:15PM -0500, Pierre-Louis Bossart wrote:
>> When we disable the stream and then call hw_free, two bank switches
>> will be handled and as a result we re-enable the stream on hw_free.
>>
> 
> I didnt quite get why there will be two bank switches as part of disable flow
> which leads to enabling of stream?

You have two bank switches, one to stop streaming and on in de-prepare. 
It's symmetrical with the start sequence, where we do a bank switch to 
prepare and another to enable.

Let's assume we are using bank0 when streaming.

Before the first bank switch, the channel_enable is set to false in the 
alternate bank1. When the bank switch happens, bank1 become active and 
the streaming stops. But bank0 registers have not been modified so when 
we do the second bank switch in de-prepare we make bank0 active, and the 
ch_enable bits are still set so streaming will restart... When we stop 
streaming, we need to make sure the ch_enable bits are cleared in the 
two banks.


> 
>> Make sure the stream is disabled on both banks.
>>
>> TODO: we need to completely revisit all this and make sure we have a
>> mirroring mechanism between current and alternate banks.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   drivers/soundwire/stream.c | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
>> index 53f5e790fcd7..75b9ad1fb1a6 100644
>> --- a/drivers/soundwire/stream.c
>> +++ b/drivers/soundwire/stream.c
>> @@ -1637,7 +1637,24 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
>>   		}
>>   	}
>>   
>> -	return do_bank_switch(stream);
>> +	ret = do_bank_switch(stream);
>> +	if (ret < 0) {
>> +		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* make sure alternate bank (previous current) is also disabled */
>> +	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
>> +		bus = m_rt->bus;
>> +		/* Disable port(s) */
>> +		ret = sdw_enable_disable_ports(m_rt, false);
>> +		if (ret < 0) {
>> +			dev_err(bus->dev, "Disable port(s) failed: %d\n", ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>>   }
>>   
>>   /**
>> -- 
>> 2.20.1
>>
>