| Message ID | 20250227-apple-codec-changes-v3-17-cbb130030acf@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: tas27{64,70}: improve support for Apple codec variants | expand |
On Thu, Feb 27, 2025 at 10:07:44PM +1000, James Calligeros wrote: > TAS2770 can pull down and zero-fill SDOUT when not actively transmitting > TDM slot data. Zero-fill is useful when there are no other amps on the > bus. Pulldown is useful when the chip is attached to a shared bus. > > Signed-off-by: James Calligeros <jcalligeros99@gmail.com> > --- > .../bindings/sound/ti,tas2770.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/ti,tas2770.yaml b/Documentation/devicetree/bindings/sound/ti,tas2770.yaml > index 8eab98a0f7a25a9c87d2c56fd0635ff8ecee17d0..3eba9bb34a581526f68b6bf2e8437e1f1e03d26f 100644 > --- a/Documentation/devicetree/bindings/sound/ti,tas2770.yaml > +++ b/Documentation/devicetree/bindings/sound/ti,tas2770.yaml > @@ -57,6 +57,18 @@ properties: > - 0 # Rising edge > - 1 # Falling edge > > + ti,sdout-pull-down: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + If present, SDOUT will be pulled low when not > + transmitting. > + > + ti,sdout-zero-fill: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + If present, SDOUT will be zero-filled when not > + transmitting. Can't you align this with the other property you added? Or extend the existing TDM properties we have. Rob
On Tue, Mar 4, 2025 at 1:50 PM Rob Herring <robh@kernel.org> wrote: > Can't you align this with the other property you added? Or extend the > existing TDM properties we have. I don't think either option makes sense given the functionality. This chip behaves differently to TAS2764, and instead of using a bitmask to determine which slots to ignore, we only get a single bit to tell the chip whether we want it to fill or pull down *all* inactive slots. The property being a u32 mask therefore does not make sense here. Building the logic off the existing generic TDM slot properties would alter behaviour of existing implementations where zero-fill and pulldown may not be required or even wanted. This may continue to be the case going forward so I'd rather make it an explicit opt-in rather than some unconditional thing we try to turn on heuristically. I gave some thought to flipping these bits if a TDM slot mask is passed to the driver, however it can still be the case that we don't want both zero-fill *and* pulldown active at the same time, or as above some implementations may want neither, so we still need to be able to specify them individually. Regards, James
On Wed, Mar 05, 2025 at 01:19:15AM +0000, James Calligeros wrote: > On Tue, Mar 4, 2025 at 1:50 PM Rob Herring <robh@kernel.org> wrote: > > Can't you align this with the other property you added? Or extend the > > existing TDM properties we have. > > I don't think either option makes sense given the functionality. This chip > behaves differently to TAS2764, and instead of using a bitmask to determine > which slots to ignore, we only get a single bit to tell the chip whether we want > it to fill or pull down *all* inactive slots. The property being a u32 mask > therefore does not make sense here. If there's a single bit control, then that just means there's only 1 valid value for a mask in that case. Or maybe a mask is overkill. What's the usecase for fill or pulldown *some* inactive slots? > Building the logic off the existing generic TDM slot properties would alter > behaviour of existing implementations where zero-fill and pulldown may not be > required or even wanted. This may continue to be the case going forward so I'd > rather make it an explicit opt-in rather than some unconditional thing we try to > turn on heuristically. Existing implementations would not have the new/extra properties and would continue to operate as before. Or those drivers could simply ignore the properties. > I gave some thought to flipping these bits if a TDM slot mask is passed to the > driver, however it can still be the case that we don't want both zero-fill *and* > pulldown active at the same time, or as above some implementations may want > neither, so we still need to be able to specify them individually. This just feels like something common because any TDM interface may need to control this. It's not really a property of the chip, but requirement of the TDM interface. Rob
On Wed, Mar 5, 2025 at 11:22 PM Rob Herring <robh@kernel.org> wrote: > This just feels like something common because any TDM interface may need > to control this. It's not really a property of the chip, but requirement > of the TDM interface. What I'm imagining then is something like: dai-link@0 { cpu { sound-dai = <&some_cpu>; }; codec { sound-dai = <&some_codec>; dai-tdm-tx-zerofill; dai-tdm-tx-pulldown; /* either or, having both makes no sense */ }; }; Codec drivers would then provide a function to set TDM TX behaviour if they support it, and export that as a dai op for use by machine drivers when they parse the dai link similar to dai-tdm-tx-slot and friends. Is that close to what you have in mind? Regards, James
On Fri, Mar 07, 2025 at 04:18:31PM +1000, James Calligeros wrote: > On Wed, Mar 5, 2025 at 11:22 PM Rob Herring <robh@kernel.org> wrote: > > This just feels like something common because any TDM interface may need > > to control this. It's not really a property of the chip, but requirement > > of the TDM interface. > > What I'm imagining then is something like: > > dai-link@0 { > cpu { > sound-dai = <&some_cpu>; > }; > codec { > sound-dai = <&some_codec>; > dai-tdm-tx-zerofill; > dai-tdm-tx-pulldown; /* either or, having both makes no sense */ If they are mutually exclusive, it's best to design the properties that way. So something like: dai-tdm-tx-idle = "zerofill"; dai-tdm-tx-idle = "pulldown"; > }; > }; > > Codec drivers would then provide a function to set TDM TX behaviour if they > support it, and export that as a dai op for use by machine drivers > when they parse > the dai link similar to dai-tdm-tx-slot and friends. Is that close to > what you have > in mind? How would it work when you need a mask? "dai-tdm-slot-tx-mask" is enough? Rob
On Sat, Mar 8, 2025 at 6:51 AM Rob Herring <robh@kernel.org> wrote: > How would it work when you need a mask? "dai-tdm-slot-tx-mask" is > enough? The existing TX/RX slot masks are used to control which slots the codec is operating on, AIUI. I don't know if it makes sense to alter how codecs deal with this. Could we combine the suggested dai-tdm-slot-tx-idle with an optional dai-tdm-slot-tx-idle-mask property? From the machine driver's perspective, the API would then be similar to the existing set_tdm_slot ops. The current downstream macaudio machine driver builds its links by allowing multiple codecs and CPUs to be linked to a DAI, like so: dai-link@0 { cpu { sound-dai = <&cpu0>, <&cpu1>; }; codec { sound-dai = <&speaker0>, ..., <&speaker6>; }; }; In this case, the codec-specific mask property was added so that a mask could be applied to a specific codec rather than the whole dai, however from upstream drivers tt looks like the way this should be handled is to have "dai-tdm-slot-tx-idle-mask-n" properties at the dai level, then have the machine driver set the mask for the appropriate codec during setup. So for macaudio, assuming speaker5 requires this zerofill mask, we would have something like this: dai-link@0 { cpu { sound-dai = <&cpu0>, <&cpu1>; }; codec { sound-dai = <&speaker0>, ..., <&speaker6>; }; /* equivalent to ti,sdout-force-zero-mask = <0xf0f0f0> */ dai-tdm-slot-tx-idle-mode-5 = "zerofill"; /* should this be an array like the slot masks? */ dai-tdm-slot-tx-idle-mask-5 = <0xf0f0f0>; }; The machine driver then calls something like snd_soc_dai_set_tdm_idle(speaker5_dai, SND_SOC_TDM_IDLE_MODE_ZEROFILL, SND_SOC_TDM_IDLE_MODE_NONE, tx_idle_mask, 0); and the codec driver deals with it however it needs to. Regards, James
diff --git a/Documentation/devicetree/bindings/sound/ti,tas2770.yaml b/Documentation/devicetree/bindings/sound/ti,tas2770.yaml index 8eab98a0f7a25a9c87d2c56fd0635ff8ecee17d0..3eba9bb34a581526f68b6bf2e8437e1f1e03d26f 100644 --- a/Documentation/devicetree/bindings/sound/ti,tas2770.yaml +++ b/Documentation/devicetree/bindings/sound/ti,tas2770.yaml @@ -57,6 +57,18 @@ properties: - 0 # Rising edge - 1 # Falling edge + ti,sdout-pull-down: + $ref: /schemas/types.yaml#/definitions/flag + description: + If present, SDOUT will be pulled low when not + transmitting. + + ti,sdout-zero-fill: + $ref: /schemas/types.yaml#/definitions/flag + description: + If present, SDOUT will be zero-filled when not + transmitting. + '#sound-dai-cells': # The codec has a single DAI, the #sound-dai-cells=<1>; case is left in for backward # compatibility but is deprecated.
TAS2770 can pull down and zero-fill SDOUT when not actively transmitting TDM slot data. Zero-fill is useful when there are no other amps on the bus. Pulldown is useful when the chip is attached to a shared bus. Signed-off-by: James Calligeros <jcalligeros99@gmail.com> --- .../bindings/sound/ti,tas2770.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+)