Message-ID: <09bf8e07-6275-654f-4a70-d46b54e9b853@collabora.com>
Date:   Tue, 21 Feb 2023 08:28:42 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v5 3/4] ALSA: cs35l41: Add shared boost feature
To:     David Rhodes <drhodes@opensource.cirrus.com>,
        Charles Keepax <ckeepax@opensource.cirrus.com>
Cc:     David Rhodes <david.rhodes@cirrus.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Mark Brown <broonie@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Takashi Iwai <tiwai@suse.com>, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org, patches@opensource.cirrus.com,
        linux-kernel@vger.kernel.org, kernel@collabora.com
References: <20230210091942.10866-1-lucas.tanure@collabora.com>
 <20230210091942.10866-4-lucas.tanure@collabora.com>
 <20230210134341.GF68926@ediswmail.ad.cirrus.com>
 <cfacc3d6-2daa-6aa3-ba19-281b7e48bb47@collabora.com>
 <20230211170638.GG68926@ediswmail.ad.cirrus.com>
 <1e3ef067-9b39-dc19-5fbc-75436c67f206@collabora.com>
 <d86d989b-0d82-74b3-a5da-9972324e9477@opensource.cirrus.com>
Content-Language: en-US
From:   Lucas Tanure <lucas.tanure@collabora.com>
In-Reply-To: <d86d989b-0d82-74b3-a5da-9972324e9477@opensource.cirrus.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 20-02-2023 19:25, David Rhodes wrote:
> 
> On 2/12/23 03:28, Lucas Tanure wrote:
>> On 11-02-2023 17:06, Charles Keepax wrote:
>>> On Fri, Feb 10, 2023 at 02:39:56PM +0000, Lucas Tanure wrote:
>>>> On 10-02-2023 13:43, Charles Keepax wrote:
>>>>> On Fri, Feb 10, 2023 at 09:19:41AM +0000, Lucas Tanure wrote:
>>>>>> + {CS35L41_MDSYNC_EN,        0x00001000},
>>>>> David's internal patch appears to set 0x3000 on the active side,
>>>>> not sure where that difference snuck in, or which is the correct
>>>>> value. Your settings appear to make logical sense to me though, TX
>>>>> on the active side, RX on the passive side.
>>>> And as the patch sets TX and RX in the same chip I changed to follow
>>>> the documentation.
>>>
>>> Yeah I mean I suspect this is sensible, unless there is some
>>> reason the controller side also needs to have RX enabled. Perhaps
>>> for feedback or something from the passive side, but I imagine
>>> this is just a typo in the original patch.
>>
>> Ok, but the other side doesn't have both RX and TX enabled.
>> If the active side needed RX to receive information for the other 
>> side, the passive one would need TX enabled too.
>> So if a feedback is necessary, both channels on both sides would be 
>> enabled, not one channel in one side and both on the other.
>>
> Both amps need to transmit their boost targets to the MDSYNC bus. The 
> active amp needs to receive the combined boost target from the MDSYNC 
> bus. That is why the active amp should enable both RX and TX, and the 
> passive amp only needs to enable TX. It is not simply a unidirectional 
> flow of data from one amp to the other.
> 
> Sorry if the documentation has been mismatched or confusing at times. 
> It's taken me a while to gather the right understanding of how this all 
> works.
> 
> 
> On 2/10/23 03:19, Lucas Tanure wrote:
>> +      Shared boost allows two amplifiers to share a single boost circuit by
>> +      communicating on the MDSYNC bus. The passive amplifier does not control
>> +      the boost and receives data from the active amplifier. GPIO1 should be
> 
> Not quite correct. I would suggest: "Shared boost allows two amplifiers 
> to share a single boost circuit by communicating on the MDSYNC bus. The 
> active amplifier controls the boost circuit using combined data from 
> both amplifiers."
Ok

> 
> 
> On 2/10/23 08:39, Lucas Tanure wrote:
>>
>> This write here is to select the boost control source, which for the 
>> active should be "Class H tracking value". 
> 
> Active should use the MDSYNC value. Otherwise it will not provide boost 
> to the passive amp when there is only audio on the passive amp's channel.
David can you confirm that both sides should use MDSYNC for boost 
control source?


> 
> 
> I believe there is another change needed for the Deck, to handle the 
> 'legacy' property names instead of bst-type?
I am working with valve to update their bios.

> 
> Other than the changes needed to the reg_sequences this looks good.
> 
> 
> Thanks,
> 
> David
>