mbox series

[v1,00/20] Support HDMI audio on NVIDIA Tegra20

Message ID 20211124220057.15763-1-digetx@gmail.com
Headers show
Series Support HDMI audio on NVIDIA Tegra20 | expand

Message

Dmitry Osipenko Nov. 24, 2021, 10 p.m. UTC
This series revives Tegra20 S/PDIF driver which was upstreamed long time
ago, but never was used. It also turns Tegra DRM HDMI driver into HDMI
audio CODEC provider. Finally, HDMI audio is enabled in device-trees.
For now the audio is enable only for Acer A500 tablet and Toshiba AC100
netbook because they're already supported by upstream, later on ASUS TF101
tablet will join them.

I based S/PDIF patches on Arnd's Bergmann patch from a separate series [1]
that removes obsolete slave_id. This eases merging of the patches by
removing the merge conflict.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?submitter=80402

Arnd Bergmann (1):
  ASoC: tegra20-spdif: stop setting slave_id

Dmitry Osipenko (19):
  ASoC: dt-bindings: Add binding for Tegra20 S/PDIF
  ASoC: dt-bindings: tegra20-i2s: Convert to schema
  ASoC: dt-bindings: tegra20-i2s: Document new nvidia,fixed-parent-rate
    property
  dt-bindings: host1x: Document optional HDMI sound-dai-cells
  ASoC: tegra20: spdif: Support device-tree
  ASoC: tegra20: spdif: Set FIFO trigger level
  ASoC: tegra20: spdif: Improve driver's code
  ASoC: tegra20: spdif: Use more resource-managed helpers
  ASoC: tegra20: spdif: Reset hardware
  ASoC: tegra20: spdif: Support system suspend
  ASoC: tegra20: spdif: Filter out unsupported rates
  ASoC: tegra20: i2s: Filter out unsupported rates
  drm/tegra: hdmi: Unwind tegra_hdmi_init() errors
  drm/tegra: hdmi: Register audio CODEC on Tegra20
  ARM: tegra_defconfig: Enable S/PDIF driver
  ARM: tegra: Add S/PDIF node to Tegra20 device-tree
  ARM: tegra: Add HDMI audio graph to Tegra20 device-tree
  ARM: tegra: acer-a500: Enable S/PDIF and HDMI audio
  ARM: tegra: paz00: Enable S/PDIF and HDMI audio

 .../display/tegra/nvidia,tegra20-host1x.txt   |   1 +
 .../bindings/sound/nvidia,tegra20-i2s.txt     |  30 ---
 .../bindings/sound/nvidia,tegra20-i2s.yaml    |  78 +++++++
 .../bindings/sound/nvidia,tegra20-spdif.yaml  |  88 ++++++++
 .../boot/dts/tegra20-acer-a500-picasso.dts    |   8 +
 arch/arm/boot/dts/tegra20-paz00.dts           |   8 +
 arch/arm/boot/dts/tegra20.dtsi                |  40 +++-
 arch/arm/configs/tegra_defconfig              |   1 +
 drivers/gpu/drm/tegra/Kconfig                 |   3 +
 drivers/gpu/drm/tegra/hdmi.c                  | 168 +++++++++++++--
 sound/soc/tegra/tegra20_i2s.c                 |  49 +++++
 sound/soc/tegra/tegra20_spdif.c               | 195 +++++++++++++-----
 sound/soc/tegra/tegra20_spdif.h               |   1 +
 sound/soc/tegra/tegra_pcm.c                   |   6 +
 sound/soc/tegra/tegra_pcm.h                   |   1 +
 15 files changed, 575 insertions(+), 102 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra20-i2s.txt
 create mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra20-i2s.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra20-spdif.yaml

Comments

Dmitry Osipenko Nov. 25, 2021, 11:43 a.m. UTC | #1
25.11.2021 01:00, Dmitry Osipenko пишет:
> Add S/PDIF node to Tegra20 device-tree. It's needed for enabling HDMI
> audio support.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/boot/dts/tegra20.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 63c2c2f8c0ce..799da7dc929b 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -197,6 +197,7 @@ hdmi@54280000 {
>  			reset-names = "hdmi";
>  			power-domains = <&pd_core>;
>  			operating-points-v2 = <&hdmi_dvfs_opp_table>;
> +			#sound-dai-cells = <0>;
>  			status = "disabled";
>  		};
>  
> @@ -396,6 +397,23 @@ tegra_ac97: ac97@70002000 {
>  		status = "disabled";
>  	};
>  
> +	tegra_spdif: spdif@70002400 {
> +		compatible = "nvidia,tegra20-spdif";
> +		reg = <0x70002400 0x200>;
> +		interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&tegra_car TEGRA20_CLK_SPDIF_OUT>,
> +			 <&tegra_car TEGRA20_CLK_SPDIF_IN>;
> +		clock-names = "spdif_out", "spdif_in";
> +		resets = <&tegra_car 10>;
> +		dmas = <&apbdma 3>, <&apbdma 3>;
> +		dma-names = "rx", "tx";
> +		#sound-dai-cells = <0>;
> +		status = "disabled";
> +
> +		assigned-clocks = <&tegra_car TEGRA20_CLK_SPDIF_OUT>;
> +		assigned-clock-parents = <&tegra_car TEGRA20_CLK_PLL_A_OUT0>;
> +	};
> +
>  	tegra_i2s1: i2s@70002800 {
>  		compatible = "nvidia,tegra20-i2s";
>  		reg = <0x70002800 0x200>;
> 

@Thierry, this patch is made on top of [1].

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=271954
Mark Brown Nov. 25, 2021, 12:02 p.m. UTC | #2
On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote:
> Program FIFO trigger level properly to fix x4 accelerated playback.

Fixes like this should really go before any new stuff so they can be
sent as fixes and backported.
Dmitry Osipenko Nov. 25, 2021, 12:04 p.m. UTC | #3
25.11.2021 15:02, Mark Brown пишет:
> On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote:
>> Program FIFO trigger level properly to fix x4 accelerated playback.
> 
> Fixes like this should really go before any new stuff so they can be
> sent as fixes and backported.
> 

This driver never worked before this patchset, hence there is nothing to
backport, this is explained in the cover letter. But in general you're
correct.
Mark Brown Nov. 25, 2021, 12:28 p.m. UTC | #4
On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote:
> 25.11.2021 15:02, Mark Brown пишет:
> > On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote:
> >> Program FIFO trigger level properly to fix x4 accelerated playback.

> > Fixes like this should really go before any new stuff so they can be
> > sent as fixes and backported.

> This driver never worked before this patchset, hence there is nothing to
> backport, this is explained in the cover letter. But in general you're
> correct.

That's not going to stop the stable people backporting things, and I'd
guess it might've worked at some point on some systems - I'm not seeing
anything that jumps out as making the driver completely unworkable in
your patches.
Dmitry Osipenko Nov. 25, 2021, 12:53 p.m. UTC | #5
25.11.2021 15:28, Mark Brown пишет:
> On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote:
>> 25.11.2021 15:02, Mark Brown пишет:
>>> On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote:
>>>> Program FIFO trigger level properly to fix x4 accelerated playback.
> 
>>> Fixes like this should really go before any new stuff so they can be
>>> sent as fixes and backported.
> 
>> This driver never worked before this patchset, hence there is nothing to
>> backport, this is explained in the cover letter. But in general you're
>> correct.
> 
> That's not going to stop the stable people backporting things, and I'd
> guess it might've worked at some point on some systems - I'm not seeing
> anything that jumps out as making the driver completely unworkable in
> your patches.
> 

I can change commit message with the "fix" word removed, this should
prevent patch from backporting.

This driver never worked in mainline because S/PDIF device was never
created, thus driver was never bound. Driver doesn't work properly
without this patch. Nobody used this driver as-is before this patchset.
Mark Brown Nov. 25, 2021, 1:18 p.m. UTC | #6
On Thu, Nov 25, 2021 at 03:53:52PM +0300, Dmitry Osipenko wrote:
> 25.11.2021 15:28, Mark Brown пишет:
> > On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote:

> >> This driver never worked before this patchset, hence there is nothing to
> >> backport, this is explained in the cover letter. But in general you're
> >> correct.

> > That's not going to stop the stable people backporting things, and I'd
> > guess it might've worked at some point on some systems - I'm not seeing
> > anything that jumps out as making the driver completely unworkable in
> > your patches.

> I can change commit message with the "fix" word removed, this should
> prevent patch from backporting.

I wouldn't count on it TBH.  In any case, definitely no need to resend
for this alone.

> This driver never worked in mainline because S/PDIF device was never
> created, thus driver was never bound. Driver doesn't work properly
> without this patch. Nobody used this driver as-is before this patchset.

Someone might've been using it with an out of tree board file, I guess
on an older stable at this point.
Dmitry Osipenko Nov. 25, 2021, 1:36 p.m. UTC | #7
25.11.2021 16:18, Mark Brown пишет:
> On Thu, Nov 25, 2021 at 03:53:52PM +0300, Dmitry Osipenko wrote:
>> 25.11.2021 15:28, Mark Brown пишет:
>>> On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote:
> 
>>>> This driver never worked before this patchset, hence there is nothing to
>>>> backport, this is explained in the cover letter. But in general you're
>>>> correct.
> 
>>> That's not going to stop the stable people backporting things, and I'd
>>> guess it might've worked at some point on some systems - I'm not seeing
>>> anything that jumps out as making the driver completely unworkable in
>>> your patches.
> 
>> I can change commit message with the "fix" word removed, this should
>> prevent patch from backporting.
> 
> I wouldn't count on it TBH.  In any case, definitely no need to resend
> for this alone.

Alright, I'll keep this in mind for a potential v2. I guess Rob may ask
to remove the assigned-clocks from S/PDIF DT binding because I just
found that there is no needed to specify that property explicitly anymore.

>> This driver never worked in mainline because S/PDIF device was never
>> created, thus driver was never bound. Driver doesn't work properly
>> without this patch. Nobody used this driver as-is before this patchset.
> 
> Someone might've been using it with an out of tree board file, I guess
> on an older stable at this point.
> 

I'm very doubtful. Still, this patch could be easily backported because
all code refactoring changes that potentially may cause merge conflicts
are made after this patch. Ideally, if we really needed to backport this
patch, then it should've been one of the first patches as you suggested.