Skip to contentSkip to conversation

Qualcomm's lpass-hdmi ASoC driver to support audio over dp port

PATCH Superseded Srinivasa Rao Mandadapu 5 participants 15 messages Sep 18, 2020 – Sep 23, 2020

Subsystems (5)

v6 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

v6 cover Qualcomm's lpass-hdmi ASoC driver to support audio over dp port

These patches are to support audio over DP port on Qualcomm's SC7180 LPASS Asoc. It includes machine driver, cpu driver, platform driver updates for HDMI path support, device tree documention, lpass variant structure optimization and configuration changes. These patches depends on the DP patch series
https://patchwork.kernel.org/project/dri-devel/list/?series=332029

changes since V5:
-- Removed unused struct regmap *map in lpass_platform_alloc_hdmidmactl_fields. -- DMA alloc and free API signature change in lpass-apq8016.c, lpass-ipq806x.c -- Keeping API "irqreturn_t lpass_platform_hdmiif_irq" under ifdef macro
Changes Since v4:
-- Updated with single compatible node for both I2S and HDMI.
Changes Since v3:
-- Removed id in lpass variant structure and used snd_soc_dai_driver id.
Changes Since v2:
-- Audio buffer size(i.e. LPASS_PLATFORM_BUFFER_SIZE) in lpass-platform.c increased.
Changes Since v1:
-- Commit messages are updated
-- Addressed Rob Herring review comments

V Sujith Kumar Reddy (5):
ASoC: Add sc7180-lpass binding header hdmi define
ASoC: dt-bindings: Add dt binding for lpass hdmi Asoc:qcom:lpass-cpu:Update dts property read API
ASoC: qcom: Add support for lpass hdmi driver
ASoC: qcom: sc7180: Add support for audio over DP

 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  |  74 ++-
 include/dt-bindings/sound/sc7180-lpass.h           |   1 +
 sound/soc/qcom/Kconfig                             |   5 +
 sound/soc/qcom/Makefile                            |   2 +
 sound/soc/qcom/lpass-apq8016.c                     |   4 +-
 sound/soc/qcom/lpass-cpu.c                         |  39 +-
 sound/soc/qcom/lpass-hdmi.c                        | 596 +++++++++++++++++++++
 sound/soc/qcom/lpass-hdmi.h                        | 129 +++++
 sound/soc/qcom/lpass-ipq806x.c                     |   4 +-
 sound/soc/qcom/lpass-lpaif-reg.h                   |  52 +-
 sound/soc/qcom/lpass-platform.c                    | 468 ++++++++++++----
 sound/soc/qcom/lpass-sc7180.c                      | 116 +++-
 sound/soc/qcom/lpass.h                             | 119 +++-
 13 files changed, 1451 insertions(+), 158 deletions(-)
 create mode 100644 sound/soc/qcom/lpass-hdmi.c
 create mode 100644 sound/soc/qcom/lpass-hdmi.h
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#1 lore raw

PATCH v6 1/5 ASoC: Add sc7180-lpass binding header hdmi define

From: V Sujith Kumar Reddy <[email protected]>

Add header defining hdmi dai-id for SC7180 lpass soc in dt bindings.

---
 include/dt-bindings/sound/sc7180-lpass.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/sound/sc7180-lpass.h +1 -0
index 7d988f6..56ecaaf 100644
@@ -4,6 +4,7 @@
 
 #define MI2S_PRIMARY	0
 #define MI2S_SECONDARY	1
+#define LPASS_DP_RX	2
 
 #define LPASS_MCLK0	0
 
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#2 lore raw

PATCH v6 2/5 ASoC: dt-bindings: Add dt binding for lpass hdmi

In reply to Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Adds bindings for lpass hdmi interface
which can support audio path over dp.

---
 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 74 +++++++++++++++-------
 1 file changed, 52 insertions(+), 22 deletions(-)
Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml +52 -22
index 09c9bd2..f95ef70 100644
@@ -24,9 +24,10 @@ properties:
       - qcom,sc7180-lpass-cpu
 
   reg:
-    maxItems: 1
+    maxItems: 2
     description: LPAIF core registers
-
+  reg-names:
+     maxItems: 2
   clocks:
     minItems: 3
     maxItems: 6
@@ -36,15 +37,16 @@ properties:
     maxItems: 6
 
   interrupts:
-    maxItems: 1
+    maxItems: 2
     description: LPAIF DMA buffer interrupt
-
+  interrupt-names:
+    maxItems: 2
   qcom,adsp:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: Phandle for the audio DSP node
 
   iommus:
-    maxItems: 1
+    maxItems: 2
     description: Phandle to apps_smmu node with sid mask
 
   power-domains:
@@ -60,10 +62,12 @@ properties:
     const: 0
 
 patternProperties:
-  "(^mi2s-[0-9a-f]$|mi2s)":
+  "^dai-link@[0-9a-f]$":
     type: object
-    description: Required properties for each DAI
-
+    description: |
+      LPASS CPU dai node for each I2S device. Bindings of each node
+      depends on the specific driver providing the functionality and
+      properties.
     properties:
       reg:
         maxItems: 1
@@ -85,9 +89,11 @@ patternProperties:
 required:
   - compatible
   - reg
+  - reg-names
   - clocks
   - clock-names
   - interrupts
+  - interrupt-names
   - '#sound-dai-cells'
 
 additionalProperties: false
@@ -134,13 +140,32 @@ allOf:
     then:
       properties:
         clock-names:
-          items:
-            - const: pcnoc-sway-clk
-            - const: audio-core
-            - const: mclk0
-            - const: pcnoc-mport-clk
-            - const: mi2s-bit-clk0
-            - const: mi2s-bit-clk1
+          oneOf:
+           - items:   #for I2S
+              - const: pcnoc-sway-clk
+              - const: audio-core
+              - const: mclk0
+              - const: pcnoc-mport-clk
+              - const: mi2s-bit-clk0
+              - const: mi2s-bit-clk1
+           - items:   #for HDMI
+              - const: pcnoc-sway-clk
+              - const: audio-core
+              - const: pcnoc-mport-clk
+        reg-names:
+          anyOf:
+            - items:   #for I2S and HDMI
+              - const: lpass-hdmiif
+              - const: lpass-lpaif
+            - items:   #for I2S
+              - const: lpass-lpaif
+        interrupt-names:
+          anyOf:
+            - items:   #for I2S and HDMI
+              - const: lpass-irq-lpaif
+              - const: lpass-irq-hdmi
+            - items:   #for I2S
+              - const: lpass-irq-lpaif
       required:
         - iommus
         - power-domains
@@ -152,12 +177,15 @@ examples:
     soc {
         #address-cells = <2>;
         #size-cells = <2>;
-        lpass@62f00000 {
+        lpass@62d80000 {
             compatible = "qcom,sc7180-lpass-cpu";
 
-            reg = <0 0x62f00000  0 0x29000>;
-
-            iommus = <&apps_smmu 0x1020 0>;
+            reg = <0 0x62d87000 0 0x68000>,
+                  <0 0x62f00000 0 0x29000>;
+            reg-names = "lpass-hdmiif",
+                        "lpass-lpaif";
+            iommus = <&apps_smmu 0x1020 0>,
+                     <&apps_smmu 0x1032 0>;
             power-domains = <&lpass_hm 0>;
 
             clocks = <&gcc 131>,
@@ -171,14 +199,16 @@ examples:
                           "mclk0", "pcnoc-mport-clk",
                           "mi2s-bit-clk0", "mi2s-bit-clk1";
 
-            interrupts = <0 160 1>;
-
+            interrupts = <0 160 1>,
+                         <0 268 1>;
+            interrupt-names = "lpass-irq-lpaif",
+                              "lpass-irq-hdmi";
             #sound-dai-cells = <1>;
 
             #address-cells = <1>;
             #size-cells = <0>;
             /* Optional to set different MI2S SD lines */
-            mi2s-primary@0 {
+            dai-link@0 {
                 reg = <MI2S_PRIMARY>;
                 qcom,playback-sd-lines = <1>;
                 qcom,capture-sd-lines = <0>;
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
maintainer Reviewed-by on v6 2/5
#3 lore raw

Re: [PATCH v6 2/5] ASoC: dt-bindings: Add dt binding for lpass hdmi

Quoted 94 lines · Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Adds bindings for lpass hdmi interface
which can support audio path over dp.

---
 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 74 +++++++++++++++-------
 1 file changed, 52 insertions(+), 22 deletions(-)
Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml +21 -15
index 09c9bd2..f95ef70 100644
@@ -24,9 +24,10 @@ properties:
       - qcom,sc7180-lpass-cpu
 
   reg:
-    maxItems: 1
+    maxItems: 2
     description: LPAIF core registers
-
+  reg-names:
+     maxItems: 2
   clocks:
     minItems: 3
     maxItems: 6
@@ -36,15 +37,16 @@ properties:
     maxItems: 6
 
   interrupts:
-    maxItems: 1
+    maxItems: 2
     description: LPAIF DMA buffer interrupt
-
+  interrupt-names:
+    maxItems: 2
   qcom,adsp:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: Phandle for the audio DSP node
 
   iommus:
-    maxItems: 1
+    maxItems: 2
     description: Phandle to apps_smmu node with sid mask
 
   power-domains:
@@ -60,10 +62,12 @@ properties:
     const: 0
 
 patternProperties:
-  "(^mi2s-[0-9a-f]$|mi2s)":
+  "^dai-link@[0-9a-f]$":
     type: object
-    description: Required properties for each DAI
-
+    description: |
+      LPASS CPU dai node for each I2S device. Bindings of each node
+      depends on the specific driver providing the functionality and
+      properties.
     properties:
       reg:
         maxItems: 1
@@ -85,9 +89,11 @@ patternProperties:
 required:
   - compatible
   - reg
+  - reg-names
   - clocks
   - clock-names
   - interrupts
+  - interrupt-names
   - '#sound-dai-cells'
 
 additionalProperties: false
@@ -134,13 +140,32 @@ allOf:
     then:
       properties:
         clock-names:
-          items:
-            - const: pcnoc-sway-clk
-            - const: audio-core
-            - const: mclk0
-            - const: pcnoc-mport-clk
-            - const: mi2s-bit-clk0
-            - const: mi2s-bit-clk1
+          oneOf:
+           - items:   #for I2S
+              - const: pcnoc-sway-clk
+              - const: audio-core
+              - const: mclk0
+              - const: pcnoc-mport-clk
+              - const: mi2s-bit-clk0

Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml allOf: · line 159

Quoted 10 lines
Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml +10 -0
+              - const: mi2s-bit-clk1
+           - items:   #for HDMI
+              - const: pcnoc-sway-clk
+              - const: audio-core
+              - const: pcnoc-mport-clk
+        reg-names:
+          anyOf:
+            - items:   #for I2S and HDMI
+              - const: lpass-hdmiif
+              - const: lpass-lpaif

It would be a bit cleaner if you switch the order. Then you are just adding on to the end.

Otherwise,

Quoted 56 lines
Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml +21 -7
+            - items:   #for I2S
+              - const: lpass-lpaif
+        interrupt-names:
+          anyOf:
+            - items:   #for I2S and HDMI
+              - const: lpass-irq-lpaif
+              - const: lpass-irq-hdmi
+            - items:   #for I2S
+              - const: lpass-irq-lpaif
       required:
         - iommus
         - power-domains
@@ -152,12 +177,15 @@ examples:
     soc {
         #address-cells = <2>;
         #size-cells = <2>;
-        lpass@62f00000 {
+        lpass@62d80000 {
             compatible = "qcom,sc7180-lpass-cpu";
 
-            reg = <0 0x62f00000  0 0x29000>;
-
-            iommus = <&apps_smmu 0x1020 0>;
+            reg = <0 0x62d87000 0 0x68000>,
+                  <0 0x62f00000 0 0x29000>;
+            reg-names = "lpass-hdmiif",
+                        "lpass-lpaif";
+            iommus = <&apps_smmu 0x1020 0>,
+                     <&apps_smmu 0x1032 0>;
             power-domains = <&lpass_hm 0>;
 
             clocks = <&gcc 131>,
@@ -171,14 +199,16 @@ examples:
                           "mclk0", "pcnoc-mport-clk",
                           "mi2s-bit-clk0", "mi2s-bit-clk1";
 
-            interrupts = <0 160 1>;
-
+            interrupts = <0 160 1>,
+                         <0 268 1>;
+            interrupt-names = "lpass-irq-lpaif",
+                              "lpass-irq-hdmi";
             #sound-dai-cells = <1>;
 
             #address-cells = <1>;
             #size-cells = <0>;
             /* Optional to set different MI2S SD lines */
-            mi2s-primary@0 {
+            dai-link@0 {
                 reg = <MI2S_PRIMARY>;
                 qcom,playback-sd-lines = <1>;
                 qcom,capture-sd-lines = <0>;
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#4 lore raw

PATCH v6 3/5 Asoc:qcom:lpass-cpu:Update dts property read API

In reply to Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

---
 sound/soc/qcom/lpass-cpu.c      | 2 +-
 sound/soc/qcom/lpass-platform.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
sound/soc/qcom/lpass-cpu.c +1 -1
index 1ee6d8b..5d84f63 100644
@@ -575,7 +575,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
 	of_lpass_cpu_parse_dai_data(dev, drvdata);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");
 
 	drvdata->lpaif = devm_ioremap_resource(dev, res);
 	if (IS_ERR((void const __force *)drvdata->lpaif)) {
sound/soc/qcom/lpass-platform.c +1 -1
index df692ed..35aead1 100644
@@ -638,7 +638,7 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
 	struct lpass_variant *v = drvdata->variant;
 	int ret;
 
-	drvdata->lpaif_irq = platform_get_irq(pdev, 0);
+	drvdata->lpaif_irq = platform_get_irq_byname(pdev, "lpass-irq-lpaif");
 	if (drvdata->lpaif_irq < 0)
 		return -ENODEV;
 
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#5 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 14 lines · Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

---
  sound/soc/qcom/lpass-cpu.c      | 2 +-
  sound/soc/qcom/lpass-platform.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
sound/soc/qcom/lpass-cpu.c +0 -0
index 1ee6d8b..5d84f63 100644

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 578

Quoted 5 lines
sound/soc/qcom/lpass-cpu.c +1 -1
@@ -575,7 +575,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
  	of_lpass_cpu_parse_dai_data(dev, drvdata);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");

Interesting!! this patch https://lore.kernel.org/alsa-devel/[email protected]/ just took it out and you add it back in!

Index is always preferred over name w.r.t device tree bindings, so lets stick with that for now! Unless you have any strong reason to lookup resource by name?

--srini

Quoted 17 lines
sound/soc/qcom/lpass-cpu.c +0 -0
  	drvdata->lpaif = devm_ioremap_resource(dev, res);
  	if (IS_ERR((void const __force *)drvdata->lpaif)) {
sound/soc/qcom/lpass-platform.c +1 -1
index df692ed..35aead1 100644
@@ -638,7 +638,7 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
  	struct lpass_variant *v = drvdata->variant;
  	int ret;
 
-	drvdata->lpaif_irq = platform_get_irq(pdev, 0);
+	drvdata->lpaif_irq = platform_get_irq_byname(pdev, "lpass-irq-lpaif");
  	if (drvdata->lpaif_irq < 0)
  		return -ENODEV;
 
maintainer
#6 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 7 lines · Srinivas Kandagatla

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 578

Quoted 2 lines · Srinivasa Rao Mandadapu
sound/soc/qcom/lpass-cpu.c +1 -1
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");

Index is always preferred over name w.r.t device tree bindings, so lets stick with that for now!

It is? That's not usually the case...

Quoted 1 line

Unless you have any strong reason to lookup resource by name?

Looking things up by name tends to make the DT easier to read (since things are named).

#7 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 10 lines · Mark Brown
Quoted 5 lines · Srinivasa Rao Mandadapu

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 578

Quoted 2 lines
sound/soc/qcom/lpass-cpu.c +1 -1
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");

Index is always preferred over name w.r.t device tree bindings, so lets stick with that for now!

It is? That's not usually the case...

Quoted 1 line

Unless you have any strong reason to lookup resource by name?

Looking things up by name tends to make the DT easier to read (since things are named).

I agree with you on this and I see the point, but Rob had a very different opinion about the reg-names bindings to start with.

This topic been discussed in the past with Rob in many instances ex: https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/

According to him, reg-names seems to be highly discouraged as it came along for the OMAP folks and was related to the hwmods stuff.

--srini

maintainer
#8 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 9 lines · Srinivas Kandagatla

On 22/09/2020 12:08, Mark Brown wrote:

I agree with you on this and I see the point, but Rob had a very different opinion about the reg-names bindings to start with.

This topic been discussed in the past with Rob in many instances ex: https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/

According to him, reg-names seems to be highly discouraged as it came along for the OMAP folks and was related to the hwmods stuff.

That's very much specific to reg, it's not true of the use of names in general - Rob mentions cases like interrupts for example.

#9 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 9 lines · Mark Brown
Quoted 6 lines · Srinivas Kandagatla

On 22/09/2020 12:08, Mark Brown wrote:
I agree with you on this and I see the point, but Rob had a very different
opinion about the reg-names bindings to start with.
This topic been discussed in the past with Rob in many instances ex: https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/
According to him, reg-names seems to be highly discouraged as it came along
for the OMAP folks and was related to the hwmods stuff.

That's very much specific to reg, it's not true of the use of names in general - Rob mentions cases like interrupts for example.

I see that patch to support hdmi adds another reg-name along with "lpass-lpaif".

So, platform_get_resource_byname() is better option.

+       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-hdmiif");

Thanks,

Rohit

Signature · 2 lines
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.
#10 lore raw

Re: [PATCH v6 3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Quoted 3 lines · Rohit Kumar
Quoted 2 lines

That's very much specific to reg, it's not true of the use of names in general - Rob mentions cases like interrupts for example.

Ofcourse using names suffix for clocks and interrupts has been justified!

I don't mind having dependency on reg-names as long as Rob is happy with DT Bindings!

--srini

Quoted 7 lines

I see that patch to support hdmi adds another reg-name along with "lpass-lpaif".

So, platform_get_resource_byname() is better option.

+       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-hdmiif");

#11 lore raw

PATCH v6 4/5 ASoC: qcom: Add support for lpass hdmi driver

In reply to Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Upadate lpass cpu and platform driver to support audio over dp. Also add lpass-hdmi.c and lpass-hdmi.h.

---
 sound/soc/qcom/Kconfig           |   5 +
 sound/soc/qcom/Makefile          |   2 +
 sound/soc/qcom/lpass-apq8016.c   |   4 +-
 sound/soc/qcom/lpass-cpu.c       |  37 ++-
 sound/soc/qcom/lpass-hdmi.c      | 596 +++++++++++++++++++++++++++++++++++++++
 sound/soc/qcom/lpass-hdmi.h      | 129 +++++++++
 sound/soc/qcom/lpass-ipq806x.c   |   4 +-
 sound/soc/qcom/lpass-lpaif-reg.h |  52 +++-
 sound/soc/qcom/lpass-platform.c  | 466 ++++++++++++++++++++++++------
 sound/soc/qcom/lpass.h           | 119 +++++++-
 10 files changed, 1294 insertions(+), 120 deletions(-)
 create mode 100644 sound/soc/qcom/lpass-hdmi.c
 create mode 100644 sound/soc/qcom/lpass-hdmi.h
sound/soc/qcom/Kconfig +5 -0
index a607ace..509584c 100644
@@ -12,6 +12,10 @@ config SND_SOC_LPASS_CPU
 	tristate
 	select REGMAP_MMIO
 
+config SND_SOC_LPASS_HDMI
+	tristate
+	select REGMAP_MMIO
+
 config SND_SOC_LPASS_PLATFORM
 	tristate
 	select REGMAP_MMIO
@@ -30,6 +34,7 @@ config SND_SOC_LPASS_SC7180
 	tristate
 	select SND_SOC_LPASS_CPU
 	select SND_SOC_LPASS_PLATFORM
+	select SND_SOC_LPASS_HDMI
 
 config SND_SOC_STORM
 	tristate "ASoC I2S support for Storm boards"
sound/soc/qcom/Makefile +2 -0
index 7972c94..0bd90d7 100644
@@ -1,12 +1,14 @@
 # SPDX-License-Identifier: GPL-2.0
 # Platform
 snd-soc-lpass-cpu-objs := lpass-cpu.o
+snd-soc-lpass-hdmi-objs := lpass-hdmi.o
 snd-soc-lpass-platform-objs := lpass-platform.o
 snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o
 snd-soc-lpass-apq8016-objs := lpass-apq8016.o
 snd-soc-lpass-sc7180-objs := lpass-sc7180.o
 
 obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o
+obj-$(CONFIG_SND_SOC_LPASS_HDMI) += snd-soc-lpass-hdmi.o
 obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o
 obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o
 obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o
sound/soc/qcom/lpass-apq8016.c +2 -2
index 5c8ae22..0aedb3a 100644
@@ -125,7 +125,7 @@ static struct snd_soc_dai_driver apq8016_lpass_cpu_dai_driver[] = {
 };
 
 static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
-					   int direction)
+					   int direction, unsigned int dai_id)
 {
 	struct lpass_variant *v = drvdata->variant;
 	int chan = 0;
@@ -151,7 +151,7 @@ static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
 	return chan;
 }
 
-static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan, unsigned int dai_id)
 {
 	clear_bit(chan, &drvdata->dma_ch_bit_map);
 
sound/soc/qcom/lpass-cpu.c +34 -3
index 5d84f63..41a2470 100644
@@ -589,13 +589,33 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 						variant->wrdma_channel_start);
 
 	drvdata->lpaif_map = devm_regmap_init_mmio(dev, drvdata->lpaif,
-			&lpass_cpu_regmap_config);
+								&lpass_cpu_regmap_config);
 	if (IS_ERR(drvdata->lpaif_map)) {
 		dev_err(dev, "error initializing regmap: %ld\n",
 			PTR_ERR(drvdata->lpaif_map));
 		return PTR_ERR(drvdata->lpaif_map);
 	}
+#ifdef CONFIG_SND_SOC_LPASS_SC7180
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-hdmiif");
 
+	drvdata->hdmiif = devm_ioremap_resource(dev, res);
+	if (IS_ERR((void const __force *)drvdata->hdmiif)) {
+		dev_err(dev, "error mapping reg resource: %ld\n",
+				PTR_ERR((void const __force *)drvdata->hdmiif));
+		return PTR_ERR((void const __force *)drvdata->hdmiif);
+	}
+
+	lpass_hdmi_regmap_config.max_register = LPAIF_HDMI_RDMAPER_REG(variant,
+				variant->hdmi_rdma_channels);
+	drvdata->hdmiif_map = devm_regmap_init_mmio(dev, drvdata->hdmiif,
+				&lpass_hdmi_regmap_config);
+	if (IS_ERR(drvdata->hdmiif_map)) {
+		dev_err(dev, "error initializing regmap: %ld\n",
+		PTR_ERR(drvdata->hdmiif_map));
+		return PTR_ERR(drvdata->hdmiif_map);
+	}
+
+#endif
 	if (variant->init) {
 		ret = variant->init(pdev);
 		if (ret) {
@@ -606,6 +626,9 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
 	for (i = 0; i < variant->num_dai; i++) {
 		dai_id = variant->dai_driver[i].id;
+		if (dai_id == LPASS_DP_RX)
+			continue;
+
 		drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(dev,
 					     variant->dai_osr_clk_names[i]);
 		if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
@@ -625,10 +648,10 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 				"error getting %s: %ld\n",
 				variant->dai_bit_clk_names[i],
 				PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
+
 			return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
 		}
 	}
-
 	/* Allocation for i2sctl regmap fields */
 	drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct lpaif_i2sctl),
 					GFP_KERNEL);
@@ -636,10 +659,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 	/* Initialize bitfields for dai I2SCTL register */
 	ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
 						drvdata->lpaif_map);
-	if (ret) {
+	if (ret)
 		dev_err(dev, "error init i2sctl field: %d\n", ret);
+
+#ifdef CONFIG_SND_SOC_LPASS_SC7180
+	ret = lpass_hdmi_init_bitfields(dev, drvdata->hdmiif_map);
+	if (ret) {
+		dev_err(dev, "%s error  hdmi init failed\n", __func__);
 		return ret;
 	}
+#else
+	return -EINVAL;
+#endif
 
 	ret = devm_snd_soc_register_component(dev,
 					      &lpass_cpu_comp_driver,
sound/soc/qcom/lpass-hdmi.c new file +596 -0

596 lines not on this page · raw message

sound/soc/qcom/lpass-hdmi.h new file +129 -0

129 lines not on this page · raw message

sound/soc/qcom/lpass-ipq806x.c +2 -2
index 72f09b3..832a916 100644
@@ -96,7 +96,7 @@ static int ipq806x_lpass_exit(struct platform_device *pdev)
 	return 0;
 }
 
-static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
+static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir, unsigned int dai_id)
 {
 	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
 		return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
@@ -104,7 +104,7 @@ static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
 		return -EINVAL;
 }
 
-static int ipq806x_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int ipq806x_lpass_free_dma_channel(struct lpass_data *drvdata, int chan, unsigned int dai_id)
 {
 	return 0;
 }
sound/soc/qcom/lpass-lpaif-reg.h +40 -12

78 lines not on this page · raw message

sound/soc/qcom/lpass-platform.c +373 -93

743 lines not on this page · raw message

sound/soc/qcom/lpass.h +111 -8

197 lines not on this page · raw message

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#12 lore raw

Re: [PATCH v6 4/5] ASoC: qcom: Add support for lpass hdmi driver

Quoted 91 lines · Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Upadate lpass cpu and platform driver to support audio over dp. Also add lpass-hdmi.c and lpass-hdmi.h.

---
  sound/soc/qcom/Kconfig           |   5 +
  sound/soc/qcom/Makefile          |   2 +
  sound/soc/qcom/lpass-apq8016.c   |   4 +-
  sound/soc/qcom/lpass-cpu.c       |  37 ++-
  sound/soc/qcom/lpass-hdmi.c      | 596 +++++++++++++++++++++++++++++++++++++++
  sound/soc/qcom/lpass-hdmi.h      | 129 +++++++++
  sound/soc/qcom/lpass-ipq806x.c   |   4 +-
  sound/soc/qcom/lpass-lpaif-reg.h |  52 +++-
  sound/soc/qcom/lpass-platform.c  | 466 ++++++++++++++++++++++++------
  sound/soc/qcom/lpass.h           | 119 +++++++-
  10 files changed, 1294 insertions(+), 120 deletions(-)

create mode 100644 sound/soc/qcom/lpass-hdmi.c
create mode 100644 sound/soc/qcom/lpass-hdmi.h

sound/soc/qcom/Kconfig +5 -0
index a607ace..509584c 100644
@@ -12,6 +12,10 @@ config SND_SOC_LPASS_CPU
  	tristate
  	select REGMAP_MMIO
 
+config SND_SOC_LPASS_HDMI
+	tristate
+	select REGMAP_MMIO
+
  config SND_SOC_LPASS_PLATFORM
  	tristate
  	select REGMAP_MMIO
@@ -30,6 +34,7 @@ config SND_SOC_LPASS_SC7180
  	tristate
  	select SND_SOC_LPASS_CPU
  	select SND_SOC_LPASS_PLATFORM
+	select SND_SOC_LPASS_HDMI
 
  config SND_SOC_STORM
  	tristate "ASoC I2S support for Storm boards"
sound/soc/qcom/Makefile +2 -0
index 7972c94..0bd90d7 100644
@@ -1,12 +1,14 @@
  # SPDX-License-Identifier: GPL-2.0
  # Platform
  snd-soc-lpass-cpu-objs := lpass-cpu.o
+snd-soc-lpass-hdmi-objs := lpass-hdmi.o
  snd-soc-lpass-platform-objs := lpass-platform.o
  snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o
  snd-soc-lpass-apq8016-objs := lpass-apq8016.o
  snd-soc-lpass-sc7180-objs := lpass-sc7180.o
 
  obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o
+obj-$(CONFIG_SND_SOC_LPASS_HDMI) += snd-soc-lpass-hdmi.o
  obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o
  obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o
  obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o
sound/soc/qcom/lpass-apq8016.c +2 -2
index 5c8ae22..0aedb3a 100644
@@ -125,7 +125,7 @@ static struct snd_soc_dai_driver apq8016_lpass_cpu_dai_driver[] = {
  };
 
  static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
-					   int direction)
+					   int direction, unsigned int dai_id)
  {
  	struct lpass_variant *v = drvdata->variant;
  	int chan = 0;
@@ -151,7 +151,7 @@ static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
  	return chan;
  }
 
-static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan, unsigned int dai_id)
  {
  	clear_bit(chan, &drvdata->dma_ch_bit_map);
 
sound/soc/qcom/lpass-cpu.c +0 -0
index 5d84f63..41a2470 100644

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 592

Quoted 5 lines
sound/soc/qcom/lpass-cpu.c +1 -1
@@ -589,13 +589,33 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
  						variant->wrdma_channel_start);
 
  	drvdata->lpaif_map = devm_regmap_init_mmio(dev, drvdata->lpaif,
-			&lpass_cpu_regmap_config);
+								&lpass_cpu_regmap_config);

change not related to this patch!

Have you run checkpatch on this patch

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 598

Quoted 6 lines
sound/soc/qcom/lpass-cpu.c +1 -0
  	if (IS_ERR(drvdata->lpaif_map)) {
  		dev_err(dev, "error initializing regmap: %ld\n",
  			PTR_ERR(drvdata->lpaif_map));
  		return PTR_ERR(drvdata->lpaif_map);
  	}
+#ifdef CONFIG_SND_SOC_LPASS_SC7180

No, this is not the right way to do it!
We can not have any ifdef with in the code, we should be able to cope up with finding if hdmiif resources are available or not!

You should proably look at variant data to see which resources are required or not! That should work and also get rid of this #ifdef

Quoted 34 lines
sound/soc/qcom/lpass-cpu.c +22 -0
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-hdmiif");
 
+	drvdata->hdmiif = devm_ioremap_resource(dev, res);
+	if (IS_ERR((void const __force *)drvdata->hdmiif)) {
+		dev_err(dev, "error mapping reg resource: %ld\n",
+				PTR_ERR((void const __force *)drvdata->hdmiif));
+		return PTR_ERR((void const __force *)drvdata->hdmiif);
+	}
+
+	lpass_hdmi_regmap_config.max_register = LPAIF_HDMI_RDMAPER_REG(variant,
+				variant->hdmi_rdma_channels);
+	drvdata->hdmiif_map = devm_regmap_init_mmio(dev, drvdata->hdmiif,
+				&lpass_hdmi_regmap_config);
+	if (IS_ERR(drvdata->hdmiif_map)) {
+		dev_err(dev, "error initializing regmap: %ld\n",
+		PTR_ERR(drvdata->hdmiif_map));
+		return PTR_ERR(drvdata->hdmiif_map);
+	}
+
+#endif
  	if (variant->init) {
  		ret = variant->init(pdev);
  		if (ret) {
@@ -606,6 +626,9 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
  	for (i = 0; i < variant->num_dai; i++) {
  		dai_id = variant->dai_driver[i].id;
+		if (dai_id == LPASS_DP_RX)
+			continue;
+
  		drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(dev,
  					     variant->dai_osr_clk_names[i]);
  		if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 651

Quoted 4 lines
sound/soc/qcom/lpass-cpu.c +1 -0
@@ -625,10 +648,10 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
  				"error getting %s: %ld\n",
  				variant->dai_bit_clk_names[i],
  				PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
+

change not related to this patch!

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 631

Quoted 4 lines
sound/soc/qcom/lpass-cpu.c +0 -1
  			return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
  		}
  	}
-

change not related to this patch!

Quoted 29 lines
sound/soc/qcom/lpass-cpu.c +9 -1
  	/* Allocation for i2sctl regmap fields */
  	drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct lpaif_i2sctl),
  					GFP_KERNEL);
@@ -636,10 +659,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
  	/* Initialize bitfields for dai I2SCTL register */
  	ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
  						drvdata->lpaif_map);
-	if (ret) {
+	if (ret)
  		dev_err(dev, "error init i2sctl field: %d\n", ret);
+
+#ifdef CONFIG_SND_SOC_LPASS_SC7180
+	ret = lpass_hdmi_init_bitfields(dev, drvdata->hdmiif_map);
+	if (ret) {
+		dev_err(dev, "%s error  hdmi init failed\n", __func__);
  		return ret;
  	}
+#else
+	return -EINVAL;
+#endif
 
  	ret = devm_snd_soc_register_component(dev,
  					      &lpass_cpu_comp_driver,
sound/soc/qcom/lpass-hdmi.c new file +0 -0
new file mode 100644
index 0000000..7cb2ba3

sound/soc/qcom/lpass-hdmi.c line 8

Quoted 8 lines
sound/soc/qcom/lpass-hdmi.c new file +8 -0
@@ -0,0 +1,596 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ *
+ * lpass-hdmi.c -- ALSA SoC HDMI-CPU DAI driver for QTi LPASS HDMI
+ */
+
+#include <linux/clk.h>

Do you need this header?

sound/soc/qcom/lpass-hdmi.c line 10

Quoted 2 lines
sound/soc/qcom/lpass-hdmi.c new file +2 -0
+#include <linux/kernel.h>
+#include <linux/module.h>

<<<

Quoted 4 lines
sound/soc/qcom/lpass-hdmi.c new file +3 -0
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
Quoted 1 line
Quoted 1 line

Do you need these headers?

Quoted 29 lines
sound/soc/qcom/lpass-hdmi.c new file +29 -0
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <linux/regmap.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <dt-bindings/sound/sc7180-lpass.h>
+#include "lpass-lpaif-reg.h"
+#include "lpass.h"
+#include "lpass-hdmi.h"
+
+int lpass_hdmi_init_bitfields(struct device *dev, struct regmap *map)
+{
+	struct lpass_data *drvdata = dev_get_drvdata(dev);
+	struct lpass_variant *v = drvdata->variant;
+	unsigned int i;
+	struct lpass_hdmi_tx_ctl *tx_ctl;
+	struct lpass_hdmitx_legacy *legacy;
+	struct lpass_vbit_ctrl *vbit_ctl;
+	struct lpass_hdmi_tx_parity *tx_parity;
+	struct lpass_dp_metadata_ctl *meta_ctl;
+	struct lpass_sstream_ctl *sstream_ctl;
+	struct lpass_hdmi_tx_ch_msb *ch_msb;
+	struct lpass_hdmi_tx_ch_lsb *ch_lsb;
+	struct lpass_hdmitx_dmactl *hdmi_tx_dmactl;
+
+	drvdata->tx_ctl = devm_kzalloc(dev, sizeof(struct lpass_hdmi_tx_ctl),
+					GFP_KERNEL);
+	if (drvdata->tx_ctl == NULL)
+		return -ENOMEM;

sound/soc/qcom/lpass-hdmi.c line 52

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+
+	tx_ctl = drvdata->tx_ctl;
+	tx_ctl->soft_reset = devm_regmap_field_alloc(dev, map,
+				v->soft_reset);
+	tx_ctl->force_reset = devm_regmap_field_alloc(dev, map,
+				v->force_reset);
+	if (IS_ERR(tx_ctl->soft_reset) || IS_ERR(tx_ctl->force_reset))
+		return -EINVAL;
+
+	drvdata->legacy = devm_kzalloc(dev, sizeof(struct lpass_hdmitx_legacy),

consider doing something like this to save uncessary long lines here! which makes code unreadable.

legacy = devm_kzalloc(dev, sizeof(*legacy), GFP_KERNEL); if (!legacy)
return -ENOMEM

drvdata->legacy = legacy;

use sizeof(*legacy) and same comment to other parts of the code!

Quoted 43 lines
sound/soc/qcom/lpass-hdmi.c new file +43 -0
+					GFP_KERNEL);
+	if (drvdata->legacy == NULL)
+		return -ENOMEM;
+
+	legacy = drvdata->legacy;
+	legacy->legacy_en = devm_regmap_field_alloc(dev, map,
+				v->legacy_en);
+	if (IS_ERR(legacy->legacy_en))
+		return -EINVAL;
+
+	drvdata->vbit_ctl = devm_kzalloc(dev, sizeof(struct lpass_vbit_ctrl),
+					GFP_KERNEL);
+	if (drvdata->vbit_ctl == NULL)
+		return -ENOMEM;
+
+	vbit_ctl = drvdata->vbit_ctl;
+	vbit_ctl->replace_vbit = devm_regmap_field_alloc(dev, map,
+					v->replace_vbit);
+	vbit_ctl->vbit_stream = devm_regmap_field_alloc(dev, map,
+					v->vbit_stream);
+	if (IS_ERR(vbit_ctl->replace_vbit) || IS_ERR(vbit_ctl->vbit_stream))
+		return -EINVAL;
+
+	drvdata->tx_parity = devm_kzalloc(dev,
+		sizeof(struct  lpass_hdmi_tx_parity), GFP_KERNEL);
+
+	if (drvdata->tx_parity == NULL)
+		return -ENOMEM;
+
+	tx_parity = drvdata->tx_parity;
+	tx_parity->calc_en = devm_regmap_field_alloc(dev, map, v->calc_en);
+
+	if (IS_ERR(tx_parity->calc_en))
+		return -EINVAL;
+
+	drvdata->meta_ctl = devm_kzalloc(dev,
+		sizeof(struct lpass_dp_metadata_ctl), GFP_KERNEL);
+
+	if (drvdata->meta_ctl == NULL)
+		return -ENOMEM;
+
+	meta_ctl = drvdata->meta_ctl;
+	meta_ctl->mute = devm_regmap_field_alloc(dev, map, v->mute);

sound/soc/qcom/lpass-hdmi.c line 105

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+	meta_ctl->as_sdp_cc = devm_regmap_field_alloc(dev, map,
+						v->as_sdp_cc);
+	meta_ctl->as_sdp_ct = devm_regmap_field_alloc(dev, map,
+						v->as_sdp_ct);
+	meta_ctl->aif_db4 = devm_regmap_field_alloc(dev, map,
+						v->aif_db4);
+	meta_ctl->frequency = devm_regmap_field_alloc(dev, map,
+						v->frequency);
+	meta_ctl->mst_index = devm_regmap_field_alloc(dev, map,
+						v->mst_index);

<<<<<

Quoted 9 lines
sound/soc/qcom/lpass-hdmi.c new file +8 -0
+	meta_ctl->dptx_index = devm_regmap_field_alloc(dev, map,
+						v->dptx_index);
+
+	if (IS_ERR(meta_ctl->mute) || IS_ERR(meta_ctl->as_sdp_cc) ||
+		IS_ERR(meta_ctl->as_sdp_ct) || IS_ERR(meta_ctl->aif_db4) ||
+		IS_ERR(meta_ctl->frequency) || IS_ERR(meta_ctl->mst_index) ||
+		IS_ERR(meta_ctl->dptx_index))
+		return -EINVAL;
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

These type of code is allover the place, why not just add a simple macro to do this.

something like

#define QCOM_REGMAP_FILED_ALLOC(d, m, f)	\
	do { \
		f = devm_regmap_field_alloc(d, m, f); 	\
		if (IS_ERR(f))				\
			return -EINVAL; 		\
	} while (0)

and call

	QCOM_REGMAP_FILED_ALLOC(dev, map, meta_ctl->dptx_index);

this will make code much cleaner IMO.

Also we can consider adding some bluk apis to allocate regmap field
or something like!https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/?h=wip/regmap&id=03551a6e2941d71b99e2e4a5035a9de5f0df6d5d

and call
devm_regmap_field_bulk_alloc(dev, map, &sstream_ctl->sstream_en, &v->sstream_en, 9);

this should work as long as order of reg_field and regmap_field are maintained, which in this case is true!

Quoted 283 lines
sound/soc/qcom/lpass-hdmi.c new file +283 -0
+
+	drvdata->sstream_ctl = devm_kzalloc(dev,
+		sizeof(struct lpass_sstream_ctl), GFP_KERNEL);
+	if (drvdata->sstream_ctl == NULL)
+		return -ENOMEM;
+
+	sstream_ctl = drvdata->sstream_ctl;
+	sstream_ctl->sstream_en = devm_regmap_field_alloc(dev, map,
+						v->sstream_en);
+	sstream_ctl->dma_sel = devm_regmap_field_alloc(dev, map,
+						v->dma_sel);
+	sstream_ctl->auto_bbit_en = devm_regmap_field_alloc(dev, map,
+						v->auto_bbit_en);
+	sstream_ctl->layout = devm_regmap_field_alloc(dev, map,
+						v->layout);
+	sstream_ctl->layout_sp = devm_regmap_field_alloc(dev, map,
+						v->layout_sp);
+	sstream_ctl->dp_audio = devm_regmap_field_alloc(dev, map,
+						v->dp_audio);
+	sstream_ctl->set_sp_on_en = devm_regmap_field_alloc(dev, map,
+						v->set_sp_on_en);
+	sstream_ctl->dp_staffing_en = devm_regmap_field_alloc(dev, map,
+						v->dp_staffing_en);
+	sstream_ctl->dp_sp_b_hw_en = devm_regmap_field_alloc(dev, map,
+						v->dp_sp_b_hw_en);
+
+	if (IS_ERR(sstream_ctl->sstream_en) || IS_ERR(sstream_ctl->dma_sel) ||
+		IS_ERR(sstream_ctl->auto_bbit_en) ||
+		IS_ERR(sstream_ctl->layout) || IS_ERR(sstream_ctl->layout_sp) ||
+		IS_ERR(sstream_ctl->dp_audio) ||
+		IS_ERR(sstream_ctl->set_sp_on_en) ||
+		IS_ERR(sstream_ctl->dp_staffing_en) ||
+		IS_ERR(sstream_ctl->dp_sp_b_hw_en))
+		return -EINVAL;
+
+	for (i = 0; i < LPASS_MAX_HDMI_DMA_CHANNELS; i++) {
+		drvdata->ch_msb[i] = devm_kzalloc(dev,
+			sizeof(struct lpass_hdmi_tx_ch_msb), GFP_KERNEL);
+		if (drvdata->ch_msb[i] == NULL)
+			return -ENOMEM;
+
+		ch_msb = drvdata->ch_msb[i];
+
+		ch_msb->msb_bits = devm_regmap_field_alloc(dev, map,
+							v->msb_bits);
+		if (IS_ERR(ch_msb->msb_bits))
+			return -EINVAL;
+
+		drvdata->ch_lsb[i] = devm_kzalloc(dev,
+			sizeof(struct lpass_hdmi_tx_ch_lsb), GFP_KERNEL);
+		if (drvdata->ch_lsb[i] == NULL)
+			return -ENOMEM;
+
+		ch_lsb = drvdata->ch_lsb[i];
+		ch_lsb->lsb_bits = devm_regmap_field_alloc(dev, map,
+					v->lsb_bits);
+		if (IS_ERR(ch_lsb->lsb_bits))
+			return -EINVAL;
+
+
+		drvdata->hdmi_tx_dmactl[i] = devm_kzalloc(dev,
+			sizeof(struct lpass_hdmitx_dmactl), GFP_KERNEL);
+		if (drvdata->hdmi_tx_dmactl[i] == NULL)
+			return -ENOMEM;
+
+		hdmi_tx_dmactl = drvdata->hdmi_tx_dmactl[i];
+		hdmi_tx_dmactl->use_hw_chs = devm_regmap_field_alloc(dev, map,
+						v->use_hw_chs);
+		hdmi_tx_dmactl->use_hw_usr = devm_regmap_field_alloc(dev, map,
+						v->use_hw_usr);
+		hdmi_tx_dmactl->hw_chs_sel = devm_regmap_field_alloc(dev, map,
+						v->hw_chs_sel);
+		hdmi_tx_dmactl->hw_usr_sel = devm_regmap_field_alloc(dev, map,
+						v->hw_usr_sel);
+		if (IS_ERR(hdmi_tx_dmactl->use_hw_chs) ||
+			IS_ERR(hdmi_tx_dmactl->use_hw_usr) ||
+			IS_ERR(hdmi_tx_dmactl->hw_chs_sel) ||
+			IS_ERR(hdmi_tx_dmactl->hw_usr_sel))
+			return -EINVAL;
+	}
+	return 0;
+
+}
+EXPORT_SYMBOL(lpass_hdmi_init_bitfields);
+
+static int lpass_hdmi_daiops_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
+{
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	snd_pcm_format_t format = params_format(params);
+	unsigned int rate = params_rate(params);
+	unsigned int channels = params_channels(params);
+	unsigned int ret;
+	unsigned int bitwidth;
+	unsigned int word_length;
+	unsigned int ch_sts_buf0;
+	unsigned int ch_sts_buf1;
+	unsigned int data_format;
+	unsigned int sampling_freq;
+	unsigned int ch = 0;
+
+	bitwidth = snd_pcm_format_width(format);
+	if (bitwidth < 0) {
+		dev_err(dai->dev, "%s invalid bit width given : %d\n",
+					__func__, bitwidth);
+		return bitwidth;
+	}
+
+	switch (bitwidth) {
+	case 16:
+		word_length = LPASS_DP_AUDIO_BITWIDTH16;
+		break;
+	case 24:
+		word_length = LPASS_DP_AUDIO_BITWIDTH24;
+		break;
+	default:
+		dev_err(dai->dev, "%s invalid bit width given : %d\n",
+					__func__, bitwidth);
+		return -EINVAL;
+	}
+
+	switch (rate) {
+	case 32000:
+		sampling_freq = LPASS_SAMPLING_FREQ32;
+		break;
+	case 44100:
+		sampling_freq = LPASS_SAMPLING_FREQ44;
+		break;
+	case 48000:
+		sampling_freq = LPASS_SAMPLING_FREQ48;
+		break;
+
+	default:
+		dev_err(dai->dev, "%s invalid bit width given : %d\n",
+					__func__, bitwidth);
+		return -EINVAL;
+	}
+	data_format = LPASS_DATA_FORMAT_LINEAR;
+	ch_sts_buf0 = (((data_format << LPASS_DATA_FORMAT_SHIFT) & LPASS_DATA_FORMAT_MASK)
+				| ((sampling_freq << LPASS_FREQ_BIT_SHIFT) & LPASS_FREQ_BIT_MASK));
+	ch_sts_buf1 = (word_length) & LPASS_WORDLENGTH_MASK;
+
+	ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_RESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_CLEAR);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->legacy->legacy_en,
+				LPASS_HDMITX_LEGACY_DISABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->tx_parity->calc_en,
+				HDMITX_PARITY_CALC_EN);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->vbit_ctl->replace_vbit,
+					REPLACE_VBIT);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->vbit_ctl->vbit_stream,
+					LINEAR_PCM_DATA);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->ch_msb[0]->msb_bits, ch_sts_buf1);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->ch_lsb[0]->lsb_bits, ch_sts_buf0);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_chs,
+				HW_MODE);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_chs_sel,
+				SW_MODE);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_usr,
+				HW_MODE);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_usr_sel,
+				SW_MODE);
+	if (ret)
+
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->mute,
+				LPASS_MUTE_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->as_sdp_cc,
+				channels - 1);
+	if (ret)
+		return ret;
+
+
+	ret = regmap_field_write(drvdata->meta_ctl->as_sdp_ct,
+				LPASS_META_DEFAULT_VAL);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->aif_db4,
+				LPASS_META_DEFAULT_VAL);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->frequency, sampling_freq);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->mst_index,
+				LPASS_META_DEFAULT_VAL);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->dptx_index,
+				LPASS_META_DEFAULT_VAL);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+				LPASS_SSTREAM_DISABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->dma_sel, ch);
+	if (ret)
+		return ret;
+
+
+	ret = regmap_field_write(drvdata->sstream_ctl->auto_bbit_en,
+				LPASS_SSTREAM_DEFAULT_ENABLE);
+	if (ret)
+		return ret;
+
+
+	ret = regmap_field_write(drvdata->sstream_ctl->layout,
+				LPASS_SSTREAM_DEFAULT_DISABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->layout_sp,
+				LPASS_LAYOUT_SP_DEFAULT);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->dp_audio,
+				LPASS_SSTREAM_DEFAULT_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->set_sp_on_en,
+				LPASS_SSTREAM_DEFAULT_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->dp_sp_b_hw_en,
+				LPASS_SSTREAM_DEFAULT_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->dp_staffing_en,

sound/soc/qcom/lpass-hdmi.c line 406

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+				LPASS_SSTREAM_DEFAULT_ENABLE);
+	if (ret)
+
+		return ret;
+
+	return ret;
+}
+
+
+

too many empty lines !

Quoted 20 lines
sound/soc/qcom/lpass-hdmi.c new file +20 -0
+static int lpass_hdmi_daiops_prepare(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	int ret;
+
+	ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+					LPASS_SSTREAM_ENABLE);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(drvdata->meta_ctl->mute,
+					LPASS_MUTE_DISABLE);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static int lpass_hdmi_daiops_trigger(struct snd_pcm_substream *substream,

sound/soc/qcom/lpass-hdmi.c line 436

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+		int cmd, struct snd_soc_dai *dai)
+{
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	int ret = -EINVAL;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+

unnecessary space!

Quoted 34 lines
sound/soc/qcom/lpass-hdmi.c new file +34 -0
+		ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+					LPASS_SSTREAM_ENABLE);
+		if (ret)
+			return ret;
+
+		ret = regmap_field_write(drvdata->meta_ctl->mute,
+					LPASS_MUTE_DISABLE);
+		if (ret)
+			return ret;
+
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+
+		ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+					LPASS_SSTREAM_DISABLE);
+		if (ret)
+			return ret;
+
+
+		ret = regmap_field_write(drvdata->meta_ctl->mute,
+					LPASS_MUTE_ENABLE);
+		if (ret)
+			return ret;
+
+		ret = regmap_field_write(drvdata->sstream_ctl->dp_audio, 0);
+		if (ret)
+			return ret;
+
+		break;
+	}
+	return ret;
+}

sound/soc/qcom/lpass-hdmi.c line 480

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+
+const struct snd_soc_dai_ops asoc_qcom_lpass_hdmi_dai_ops = {
+	.hw_params	= lpass_hdmi_daiops_hw_params,
+	.prepare	= lpass_hdmi_daiops_prepare,
+	.trigger	= lpass_hdmi_daiops_trigger,
+};
+EXPORT_SYMBOL_GPL(asoc_qcom_lpass_hdmi_dai_ops);
+
+
+

too many empty lines??

sound/soc/qcom/lpass-hdmi.c line 493

Quoted 13 lines
sound/soc/qcom/lpass-hdmi.c new file +13 -0
+static bool lpass_hdmi_regmap_writeable(struct device *dev, unsigned int reg)
+{
+	struct lpass_data *drvdata = dev_get_drvdata(dev);
+	struct lpass_variant *v = drvdata->variant;
+	int i;
+
+	if (reg == LPASS_HDMI_TX_CTL_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v))
+		return true;
+

switch case should make this much cleaner.

Quoted 37 lines
sound/soc/qcom/lpass-hdmi.c new file +37 -0
+	for (i = 0; i < v->hdmi_rdma_channels; i++) {
+		if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i))
+			return true;
+		if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i))
+			return true;
+		if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i))
+			return true;
+	}
+
+	if (reg == LPASS_HDMI_TX_PARITY_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_DP_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v))
+		return true;
+
+	if (reg == LPASS_HDMITX_APP_IRQEN_REG(v))
+		return true;
+	if (reg == LPASS_HDMITX_APP_IRQCLEAR_REG(v))
+		return true;
+
+	for (i = 0; i < v->rdma_channels; ++i) {
+		if (reg == LPAIF_HDMI_RDMACTL_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMABASE_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMAPER_REG(v, i))
+			return true;
+
+	}
+	return false;
+}
+
+static bool lpass_hdmi_regmap_readable(struct device *dev, unsigned int reg)
+{

sound/soc/qcom/lpass-hdmi.c line 540

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+	struct lpass_data *drvdata = dev_get_drvdata(dev);
+	struct lpass_variant *v = drvdata->variant;
+	int i;
+
+	if (reg == LPASS_HDMI_TX_CTL_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v))
+		return true;

Same here, use switch case

Quoted 32 lines
sound/soc/qcom/lpass-hdmi.c new file +31 -0
+
+	for (i = 0; i < v->hdmi_rdma_channels; i++) {
+		if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i))
+			return true;
+		if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i))
+			return true;
+		if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i))
+			return true;
+	}
 
+
+	if (reg == LPASS_HDMI_TX_PARITY_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_DP_ADDR(v))
+		return true;
+	if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v))
+		return true;
+
+	if (reg == LPASS_HDMITX_APP_IRQEN_REG(v))
+		return true;
+	if (reg == LPASS_HDMITX_APP_IRQSTAT_REG(v))
+		return true;
+
+	for (i = 0; i < v->rdma_channels; ++i) {
+		if (reg == LPAIF_HDMI_RDMACTL_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMABASE_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i))
+			return true;
+		if (reg == LPAIF_HDMI_RDMAPER_REG(v, i))
+			return true;

sound/soc/qcom/lpass-hdmi.c line 581

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+		if (reg == LPAIF_HDMI_RDMACURR_REG(v, i))
+			return true;
+	}
+
+	return false;
+}
+
+static bool lpass_hdmi_regmap_volatile(struct device *dev, unsigned int reg)
+{
+	return true;

Are all the register range really volatile?

sound/soc/qcom/lpass-hdmi.c line 591

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+
+}
+struct regmap_config lpass_hdmi_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.writeable_reg = lpass_hdmi_regmap_writeable,
+	.readable_reg = lpass_hdmi_regmap_readable,
+	.volatile_reg = lpass_hdmi_regmap_volatile,
+	.cache_type = REGCACHE_FLAT,

Any reason for using FLAT cache type ??

Quoted 24 lines
sound/soc/qcom/lpass-hdmi.c new file +5 -0
+};
+EXPORT_SYMBOL(lpass_hdmi_regmap_config);
+
+MODULE_DESCRIPTION("QTi LPASS HDMI Driver");
+MODULE_LICENSE("GPL v2");
sound/soc/qcom/lpass-hdmi.h new file +13 -0
new file mode 100644
index 0000000..f91f322
@@ -0,0 +1,129 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ *
+ * lpass_hdmi.h - Definitions for the QTi LPASS HDMI
+ */
+
+#ifndef __LPASS_HDMI_H__
+#define __LPASS_HDMI_H__
+
+#include <linux/clk.h>
+#include <linux/compiler.h>
+#include <linux/platform_device.h>

sound/soc/qcom/lpass-hdmi.h line 23

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.h new file +10 -0
+#include <linux/regmap.h>
+
+#define LPASS_HDMITX_LEGACY_DISABLE		0x0
+#define LPASS_HDMITX_LEGACY_ENABLE		0x1
+#define LPASS_DP_AUDIO_BITWIDTH16		0x0
+#define LPASS_DP_AUDIO_BITWIDTH24		0xb
+#define LPASS_DATA_FORMAT_SHIFT			0x1
+#define LPASS_DATA_FORMAT_MASK			0x2
+#define LPASS_FREQ_BIT_SHIFT			24
+#define LPASS_FREQ_BIT_MASK			0xf000000

use GENMASK macro for all such masks.

Quoted 24 lines
sound/soc/qcom/lpass-hdmi.h new file +24 -0
+#define LPASS_DATA_FORMAT_LINEAR		0x0
+#define LPASS_DATA_FORMAT_NON_LINEAR		0x1
+#define LPASS_SAMPLING_FREQ32			0x3
+#define LPASS_SAMPLING_FREQ44			0x0
+#define LPASS_SAMPLING_FREQ48			0x2
+#define LPASS_TX_CTL_RESET			0x1
+#define LPASS_TX_CTL_CLEAR			0x0
+#define LPASS_SSTREAM_ENABLE			1
+#define LPASS_SSTREAM_DISABLE			0
+#define LPASS_LAYOUT_SP_DEFAULT			0xf
+#define LPASS_SSTREAM_DEFAULT_ENABLE		1
+#define LPASS_SSTREAM_DEFAULT_DISABLE		0
+#define LPASS_WORDLENGTH_MASK			0xf
+#define LPASS_MUTE_ENABLE			1
+#define LPASS_MUTE_DISABLE			0
+#define LPASS_META_DEFAULT_VAL			0
+#define HW_MODE					1
+#define SW_MODE					0
+#define LEGACY_LPASS_LPAIF			1
+#define LEGACY_LPASS_HDMI			0
+#define REPLACE_VBIT				0x1
+#define LINEAR_PCM_DATA				0x0
+#define NON_LINEAR_PCM_DATA			0x1
+#define HDMITX_PARITY_CALC_EN			0x1

sound/soc/qcom/lpass-hdmi.h line 57

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.h new file +10 -0
+#define HDMITX_PARITY_CALC_DIS			0x0
+
+
+#define LPASS_HDMI_TX_CTL_ADDR(v) \
+		(v->hdmi_tx_ctl_addr)
+
+#define LPASS_HDMI_TX_LEGACY_ADDR(v) \
+		(v->hdmi_legacy_addr)
+#define LPASS_HDMI_TX_VBIT_CTL_ADDR(v) \
+		(v->hdmi_vbit_addr)

above three macros should be able to go in single lines!! Not sure why you are adding an extra break, which makes it very odd for reading.

Quoted 234 lines
sound/soc/qcom/lpass-hdmi.h new file +72 -0
+#define LPASS_HDMI_TX_CH_LSB_ADDR(v, port) \
+		(v->hdmi_ch_lsb_addr + \
+		v->ch_stride * (port))
+#define LPASS_HDMI_TX_CH_MSB_ADDR(v, port) \
+		(v->hdmi_ch_msb_addr + \
+		v->ch_stride * (port))
+#define LPASS_HDMI_TX_DMA_ADDR(v, port) \
+		(v->hdmi_dmactl_addr + \
+		v->hdmi_dma_stride * (port))
+#define LPASS_HDMI_TX_PARITY_ADDR(v) \
+		(v->hdmi_parity_addr)
+#define LPASS_HDMI_TX_DP_ADDR(v) \
+		(v->hdmi_DP_addr)
+#define LPASS_HDMI_TX_SSTREAM_ADDR(v) \
+		(v->hdmi_sstream_addr)
+
+struct lpass_sstream_ctl {
+	struct regmap_field *sstream_en;
+	struct regmap_field *dma_sel;
+	struct regmap_field *auto_bbit_en;
+	struct regmap_field *layout;
+	struct regmap_field *layout_sp;
+	struct regmap_field *set_sp_on_en;
+	struct regmap_field *dp_audio;
+	struct regmap_field *dp_staffing_en;
+	struct regmap_field *dp_sp_b_hw_en;
+};
+
+struct lpass_dp_metadata_ctl {
+	struct regmap_field *mute;
+	struct regmap_field *as_sdp_cc;
+	struct regmap_field *as_sdp_ct;
+	struct regmap_field *aif_db4;
+	struct regmap_field *frequency;
+	struct regmap_field *mst_index;
+	struct regmap_field *dptx_index;
+};
+
+struct lpass_hdmi_tx_ctl {
+	struct regmap_field *soft_reset;
+	struct regmap_field *force_reset;
+};
+
+struct lpass_hdmitx_dmactl {
+	struct regmap_field *use_hw_chs;
+	struct regmap_field *use_hw_usr;
+	struct regmap_field *hw_chs_sel;
+	struct regmap_field *hw_usr_sel;
+};
+
+struct lpass_vbit_ctrl {
+		struct regmap_field *replace_vbit;
+		struct regmap_field *vbit_stream;
+};
+
+struct  lpass_hdmitx_legacy {
+	struct regmap_field *legacy_en;
+};
+
+struct  lpass_hdmi_tx_parity {
+	struct regmap_field *calc_en;
+};
+
+struct  lpass_hdmi_tx_ch_lsb {
+	struct regmap_field *lsb_bits;
+};
+
+struct  lpass_hdmi_tx_ch_msb {
+	struct regmap_field *msb_bits;
+};
+
+#endif /* __LPASS_HDMI_H__ */
sound/soc/qcom/lpass-ipq806x.c +2 -2
index 72f09b3..832a916 100644
@@ -96,7 +96,7 @@ static int ipq806x_lpass_exit(struct platform_device *pdev)
  	return 0;
  }
 
-static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
+static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir, unsigned int dai_id)
  {
  	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
  		return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
@@ -104,7 +104,7 @@ static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
  		return -EINVAL;
  }
 
-static int ipq806x_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int ipq806x_lpass_free_dma_channel(struct lpass_data *drvdata, int chan, unsigned int dai_id)
  {
  	return 0;
  }
sound/soc/qcom/lpass-lpaif-reg.h +40 -12
index 5258e60..42aebd4 100644
@@ -9,7 +9,8 @@
  /* LPAIF I2S */
 
  #define LPAIF_I2SCTL_REG_ADDR(v, addr, port) \
-	(v->i2sctrl_reg_base + (addr) + v->i2sctrl_reg_stride * (port))
+	(v->i2sctrl_reg_base + (addr) + \
+	v->i2sctrl_reg_stride * (port))
 
  #define LPAIF_I2SCTL_REG(v, port)	LPAIF_I2SCTL_REG_ADDR(v, 0x0, (port))
 
@@ -70,6 +71,14 @@
  #define LPAIF_IRQSTAT_REG(v, port)	LPAIF_IRQ_REG_ADDR(v, 0x4, (port))
  #define LPAIF_IRQCLEAR_REG(v, port)	LPAIF_IRQ_REG_ADDR(v, 0xC, (port))
 
+
+#define LPASS_HDMITX_APP_IRQ_REG_ADDR(v, addr)  \
+	((v->hdmi_irq_reg_base) + (addr))
+
+#define LPASS_HDMITX_APP_IRQEN_REG(v)			LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x4)
+#define LPASS_HDMITX_APP_IRQSTAT_REG(v)			LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x8)
+#define LPASS_HDMITX_APP_IRQCLEAR_REG(v)		LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0xC)
+
  #define LPAIF_IRQ_BITSTRIDE		3
 
  #define LPAIF_IRQ_PER(chan)		(1 << (LPAIF_IRQ_BITSTRIDE * (chan)))
@@ -77,8 +86,22 @@
  #define LPAIF_IRQ_ERR(chan)		(4 << (LPAIF_IRQ_BITSTRIDE * (chan)))
 
  #define LPAIF_IRQ_ALL(chan)		(7 << (LPAIF_IRQ_BITSTRIDE * (chan)))
+#define LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(chan)	(1 << (14 + chan))
+#define LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(chan)	(1 << (24 + chan))
+#define LPAIF_IRQ_HDMI_METADONE		BIT(23)
 
  /* LPAIF DMA */
+#define LPAIF_HDMI_RDMA_REG_ADDR(v, addr, chan) \
+	(v->hdmi_rdma_reg_base + (addr) + v->hdmi_rdma_reg_stride * (chan))
+
+#define LPAIF_HDMI_RDMACTL_AUDINTF(id)	(id << LPAIF_RDMACTL_AUDINTF_SHIFT)
+
+#define LPAIF_HDMI_RDMACTL_REG(v, chan)		LPAIF_HDMI_RDMA_REG_ADDR(v, 0x00, (chan))
+#define LPAIF_HDMI_RDMABASE_REG(v, chan)	LPAIF_HDMI_RDMA_REG_ADDR(v, 0x04, (chan))
+#define	LPAIF_HDMI_RDMABUFF_REG(v, chan)	LPAIF_HDMI_RDMA_REG_ADDR(v, 0x08, (chan))
+#define LPAIF_HDMI_RDMACURR_REG(v, chan)	LPAIF_HDMI_RDMA_REG_ADDR(v, 0x0C, (chan))
+#define	LPAIF_HDMI_RDMAPER_REG(v, chan)		LPAIF_HDMI_RDMA_REG_ADDR(v, 0x10, (chan))
+#define	LPAIF_HDMI_RDMAPERCNT_REG(v, chan)	LPAIF_HDMI_RDMA_REG_ADDR(v, 0x14, (chan))
 
  #define LPAIF_RDMA_REG_ADDR(v, addr, chan) \
  	(v->rdma_reg_base + (addr) + v->rdma_reg_stride * (chan))
@@ -103,17 +126,22 @@
  #define	LPAIF_WRDMAPER_REG(v, chan)	LPAIF_WRDMA_REG_ADDR(v, 0x10, (chan))
  #define	LPAIF_WRDMAPERCNT_REG(v, chan)	LPAIF_WRDMA_REG_ADDR(v, 0x14, (chan))
 
-#define __LPAIF_DMA_REG(v, chan, dir, reg)  \
-	(dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
-		LPAIF_RDMA##reg##_REG(v, chan) : \
-		LPAIF_WRDMA##reg##_REG(v, chan)
-
-#define LPAIF_DMACTL_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, CTL)
-#define LPAIF_DMABASE_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, BASE)
-#define	LPAIF_DMABUFF_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, BUFF)
-#define LPAIF_DMACURR_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, CURR)
-#define	LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, PER)
-#define	LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, PERCNT)
+#define LPAIF_INTFDMA_REG(v, chan, reg, dai_id)  \
+		((v->dai_driver[dai_id].id ==  LPASS_DP_RX) ? \
+		LPAIF_HDMI_RDMA##reg##_REG(v, chan) : \
+		 LPAIF_RDMA##reg##_REG(v, chan))
+
+#define __LPAIF_DMA_REG(v, chan, dir, reg, dai_id)  \
+	((dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
+		(LPAIF_INTFDMA_REG(v, chan, reg, dai_id)) : \
+		LPAIF_WRDMA##reg##_REG(v, chan))
+
+#define LPAIF_DMACTL_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, CTL, dai_id)
+#define LPAIF_DMABASE_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BASE, dai_id)
+#define	LPAIF_DMABUFF_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BUFF, dai_id)
+#define LPAIF_DMACURR_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, CURR, dai_id)
+#define	LPAIF_DMAPER_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PER, dai_id)
+#define	LPAIF_DMAPERCNT_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PERCNT, dai_id)
 
  #define LPAIF_DMACTL_BURSTEN_SINGLE	0
  #define LPAIF_DMACTL_BURSTEN_INCR4	1
sound/soc/qcom/lpass-platform.c +12 -17
index 35aead1..e2d1f25 100644
@@ -23,7 +23,7 @@ struct lpass_pcm_data {
  	int i2s_port;
  };
 
-#define LPASS_PLATFORM_BUFFER_SIZE	(16 * 1024)
+#define LPASS_PLATFORM_BUFFER_SIZE	(24 *  2 * 1024)
  #define LPASS_PLATFORM_PERIODS		2
 
  static const struct snd_pcm_hardware lpass_platform_pcm_hardware = {
@@ -62,41 +62,79 @@ static int lpass_platform_alloc_dmactl_fields(struct device *dev,
  	if (drvdata->rd_dmactl == NULL)
  		return -ENOMEM;
 
-	drvdata->wr_dmactl = devm_kzalloc(dev, sizeof(struct lpaif_dmactl),
-					  GFP_KERNEL);
-	if (drvdata->wr_dmactl == NULL)
-		return -ENOMEM;
-
  	rd_dmactl = drvdata->rd_dmactl;
-	wr_dmactl = drvdata->wr_dmactl;
 
  	rd_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->rdma_bursten);
  	rd_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->rdma_wpscnt);
  	rd_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->rdma_fifowm);
-	rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf);
  	rd_dmactl->enable = devm_regmap_field_alloc(dev, map, v->rdma_enable);
  	rd_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->rdma_dyncclk);
-
+	rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf);
  	if (IS_ERR(rd_dmactl->bursten) || IS_ERR(rd_dmactl->wpscnt) ||
-	    IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->intf) ||
-	    IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->dyncclk))
+	    IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->dyncclk) ||
+	    IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->intf))
  		return -EINVAL;
+	drvdata->wr_dmactl = devm_kzalloc(dev,
+			sizeof(struct lpaif_dmactl), GFP_KERNEL);
+	if (drvdata->wr_dmactl == NULL)
+		return -ENOMEM;
 
-	wr_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->wrdma_bursten);
-	wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->wrdma_wpscnt);
-	wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->wrdma_fifowm);
-	wr_dmactl->intf = devm_regmap_field_alloc(dev, map, v->wrdma_intf);
-	wr_dmactl->enable = devm_regmap_field_alloc(dev, map, v->wrdma_enable);
-	wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->wrdma_dyncclk);
+	wr_dmactl = drvdata->wr_dmactl;
+
+	wr_dmactl->bursten = devm_regmap_field_alloc(dev, map,
+						v->wrdma_bursten);

sound/soc/qcom/lpass-platform.c static int lpass_platform_alloc_dmactl_fields(struct device *dev, · line 95

Quoted 10 lines
sound/soc/qcom/lpass-platform.c +10 -0
+	wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map,
+						v->wrdma_wpscnt);
+	wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map,
+						v->wrdma_fifowm);
+	wr_dmactl->intf = devm_regmap_field_alloc(dev, map,
+						v->wrdma_intf);
+	wr_dmactl->enable = devm_regmap_field_alloc(dev, map,
+						v->wrdma_enable);
+	wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map,
+						v->wrdma_dyncclk);

Can you explain what is happening here, these chagnes just look like spllitting one line into two!

I should stop reviewing at this point!
As I have seen some general issues with the patch like
- doing un-needed changes!
- removing #ifdef checks.
- try to come up with some api/macro to simplify field allocs as they seems to be all over the place!

thanks,
srini

#13 lore raw

Re: [PATCH v6 4/5] ASoC: qcom: Add support for lpass hdmi driver

Thanks for review Srinivas!!!

Quoted 108 lines · Srinivas Kandagatla
Quoted 89 lines · Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Upadate lpass cpu and platform driver to support audio over dp. Also add lpass-hdmi.c and lpass-hdmi.h.

---
  sound/soc/qcom/Kconfig           |   5 + sound/soc/qcom/Makefile          |   2 + sound/soc/qcom/lpass-apq8016.c   |   4 +- sound/soc/qcom/lpass-cpu.c       |  37 ++- sound/soc/qcom/lpass-hdmi.c      | 596
+++++++++++++++++++++++++++++++++++++++
  sound/soc/qcom/lpass-hdmi.h      | 129 +++++++++ sound/soc/qcom/lpass-ipq806x.c   |   4 +- sound/soc/qcom/lpass-lpaif-reg.h |  52 +++- sound/soc/qcom/lpass-platform.c  | 466 ++++++++++++++++++++++++------ sound/soc/qcom/lpass.h           | 119 +++++++-

  10 files changed, 1294 insertions(+), 120 deletions(-)

  create mode 100644 sound/soc/qcom/lpass-hdmi.c
  create mode 100644 sound/soc/qcom/lpass-hdmi.h

sound/soc/qcom/Kconfig +0 -0
index a607ace..509584c 100644
@@ -12,6 +12,10 @@ config SND_SOC_LPASS_CPU

      tristate
      select REGMAP_MMIO
  +config SND_SOC_LPASS_HDMI
+    tristate
+    select REGMAP_MMIO
+
  config SND_SOC_LPASS_PLATFORM
      tristate
      select REGMAP_MMIO

sound/soc/qcom/Kconfig +0 -0
@@ -30,6 +34,7 @@ config SND_SOC_LPASS_SC7180

      tristate
      select SND_SOC_LPASS_CPU
      select SND_SOC_LPASS_PLATFORM
+    select SND_SOC_LPASS_HDMI
    config SND_SOC_STORM
      tristate "ASoC I2S support for Storm boards"

sound/soc/qcom/Makefile +0 -0
index 7972c94..0bd90d7 100644
@@ -1,12 +1,14 @@
   # SPDX-License-Identifier: GPL-2.0
   # Platform

  snd-soc-lpass-cpu-objs := lpass-cpu.o +snd-soc-lpass-hdmi-objs := lpass-hdmi.o snd-soc-lpass-platform-objs := lpass-platform.o snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o snd-soc-lpass-apq8016-objs := lpass-apq8016.o snd-soc-lpass-sc7180-objs := lpass-sc7180.o obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o +obj-$(CONFIG_SND_SOC_LPASS_HDMI) += snd-soc-lpass-hdmi.o obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o

sound/soc/qcom/lpass-apq8016.c +2 -1
index 5c8ae22..0aedb3a 100644
@@ -125,7 +125,7 @@ static struct snd_soc_dai_driver 
 apq8016_lpass_cpu_dai_driver[] = {
   };
     static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
-                       int direction)
+                       int direction, unsigned int dai_id)
   {
       struct lpass_variant *v = drvdata->variant;
       int chan = 0;
@@ -151,7 +151,7 @@ static int apq8016_lpass_alloc_dma_channel(struct 
 lpass_data *drvdata,
       return chan;
   }
   -static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int apq8016_lpass_free_dma_channel(struct lpass_data 
 *drvdata, int chan, unsigned int dai_id)
   {

      clear_bit(chan, &drvdata->dma_ch_bit_map);
  diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c index 5d84f63..41a2470 100644

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct · line 592

Quoted 5 lines
sound/soc/qcom/lpass-cpu.c +1 -1
@@ -589,13 +589,33 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
 platform_device *pdev)
                           variant->wrdma_channel_start);
         drvdata->lpaif_map = devm_regmap_init_mmio(dev, drvdata->lpaif,
-            &lpass_cpu_regmap_config);
+                                &lpass_cpu_regmap_config);

change not related to this patch!

Have you run checkpatch on this patch

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 598

Quoted 6 lines
sound/soc/qcom/lpass-cpu.c +1 -0
       if (IS_ERR(drvdata->lpaif_map)) {
           dev_err(dev, "error initializing regmap: %ld\n",
               PTR_ERR(drvdata->lpaif_map));
           return PTR_ERR(drvdata->lpaif_map);
       }
+#ifdef CONFIG_SND_SOC_LPASS_SC7180
Quoted 4 lines

No, this is not the right way to do it!
We can not have any ifdef with in the code, we should be able to cope up with finding if hdmiif resources are available or not!

You should proably look at variant data to see which resources are required or not! That should work and also get rid of this #ifdef

Okay. will remove this.

Quoted 37 lines
Quoted 37 lines
sound/soc/qcom/lpass-cpu.c +18 -0
+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
 "lpass-hdmiif");
   +    drvdata->hdmiif = devm_ioremap_resource(dev, res);
+    if (IS_ERR((void const __force *)drvdata->hdmiif)) {
+        dev_err(dev, "error mapping reg resource: %ld\n",
+                PTR_ERR((void const __force *)drvdata->hdmiif));
+        return PTR_ERR((void const __force *)drvdata->hdmiif);
+    }
+
+    lpass_hdmi_regmap_config.max_register = 
 LPAIF_HDMI_RDMAPER_REG(variant,
+                variant->hdmi_rdma_channels);
+    drvdata->hdmiif_map = devm_regmap_init_mmio(dev, drvdata->hdmiif,
+                &lpass_hdmi_regmap_config);
+    if (IS_ERR(drvdata->hdmiif_map)) {
+        dev_err(dev, "error initializing regmap: %ld\n",
+        PTR_ERR(drvdata->hdmiif_map));
+        return PTR_ERR(drvdata->hdmiif_map);
+    }
+
+#endif
       if (variant->init) {

          ret = variant->init(pdev);
          if (ret) {

sound/soc/qcom/lpass-cpu.c +3 -0
@@ -606,6 +626,9 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
 platform_device *pdev)
         for (i = 0; i < variant->num_dai; i++) {
           dai_id = variant->dai_driver[i].id;
+        if (dai_id == LPASS_DP_RX)
+            continue;
+
           drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(dev,
                            variant->dai_osr_clk_names[i]);
           if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
Quoted 8 lines

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct · line 652

Quoted 6 lines
sound/soc/qcom/lpass-cpu.c +1 -0
@@ -625,10 +648,10 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
 platform_device *pdev)
                   "error getting %s: %ld\n",
                   variant->dai_bit_clk_names[i],
                   PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
+

change not related to this patch!

Okay.

Quoted 7 lines

sound/soc/qcom/lpass-cpu.c int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) · line 631

Quoted 5 lines
sound/soc/qcom/lpass-cpu.c +0 -1
               return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
           }
       }
-

change not related to this patch!

Okay.

Quoted 41 lines
Quoted 31 lines
sound/soc/qcom/lpass-cpu.c +0 -0
       /* Allocation for i2sctl regmap fields */
       drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct lpaif_i2sctl),

                      GFP_KERNEL);

sound/soc/qcom/lpass-cpu.c +9 -1
@@ -636,10 +659,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
 platform_device *pdev)
       /* Initialize bitfields for dai I2SCTL register */
       ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
                           drvdata->lpaif_map);
-    if (ret) {
+    if (ret)
           dev_err(dev, "error init i2sctl field: %d\n", ret);
+
+#ifdef CONFIG_SND_SOC_LPASS_SC7180
+    ret = lpass_hdmi_init_bitfields(dev, drvdata->hdmiif_map);
+    if (ret) {
+        dev_err(dev, "%s error  hdmi init failed\n", __func__);
           return ret;
       }
+#else
+    return -EINVAL;
+#endif
         ret = devm_snd_soc_register_component(dev,
                             &lpass_cpu_comp_driver,
sound/soc/qcom/lpass-hdmi.c new file +0 -0
new file mode 100644
index 0000000..7cb2ba3

sound/soc/qcom/lpass-hdmi.c line 8

Quoted 8 lines
sound/soc/qcom/lpass-hdmi.c new file +8 -0
@@ -0,0 +1,596 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ *
+ * lpass-hdmi.c -- ALSA SoC HDMI-CPU DAI driver for QTi LPASS HDMI
+ */
+
+#include <linux/clk.h>

Do you need this header?

Quoted 8 lines

sound/soc/qcom/lpass-hdmi.c line 10

Quoted 2 lines
sound/soc/qcom/lpass-hdmi.c new file +2 -0
+#include <linux/kernel.h>
+#include <linux/module.h>

<<<

Quoted 4 lines
sound/soc/qcom/lpass-hdmi.c new file +3 -0
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
Quoted 1 line
Quoted 1 line

Do you need these headers?

Will remove redundant headers.

Quoted 36 lines
Quoted 36 lines
sound/soc/qcom/lpass-hdmi.c new file +26 -0
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <linux/regmap.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <dt-bindings/sound/sc7180-lpass.h>
+#include "lpass-lpaif-reg.h"
+#include "lpass.h"
+#include "lpass-hdmi.h"
+
+int lpass_hdmi_init_bitfields(struct device *dev, struct regmap *map)
+{
+    struct lpass_data *drvdata = dev_get_drvdata(dev);
+    struct lpass_variant *v = drvdata->variant;
+    unsigned int i;
+    struct lpass_hdmi_tx_ctl *tx_ctl;
+    struct lpass_hdmitx_legacy *legacy;
+    struct lpass_vbit_ctrl *vbit_ctl;
+    struct lpass_hdmi_tx_parity *tx_parity;
+    struct lpass_dp_metadata_ctl *meta_ctl;
+    struct lpass_sstream_ctl *sstream_ctl;
+    struct lpass_hdmi_tx_ch_msb *ch_msb;
+    struct lpass_hdmi_tx_ch_lsb *ch_lsb;
+    struct lpass_hdmitx_dmactl *hdmi_tx_dmactl;
+
+    drvdata->tx_ctl = devm_kzalloc(dev, sizeof(struct 

lpass_hdmi_tx_ctl),
+                    GFP_KERNEL);
+    if (drvdata->tx_ctl == NULL)
+        return -ENOMEM;
+
+    tx_ctl = drvdata->tx_ctl;
+    tx_ctl->soft_reset = devm_regmap_field_alloc(dev, map, +                v->soft_reset);
+    tx_ctl->force_reset = devm_regmap_field_alloc(dev, map,

Quoted 8 lines
Quoted 6 lines

+                v->force_reset);
+    if (IS_ERR(tx_ctl->soft_reset) || IS_ERR(tx_ctl->force_reset)) +        return -EINVAL;
+
+    drvdata->legacy = devm_kzalloc(dev, sizeof(struct lpass_hdmitx_legacy),

consider doing something like this to save uncessary long lines here! which makes code unreadable.

okay.

Quoted 89 lines

legacy = devm_kzalloc(dev, sizeof(*legacy), GFP_KERNEL); if (!legacy)
    return -ENOMEM

drvdata->legacy = legacy;

use sizeof(*legacy) and same comment to other parts of the code!

Quoted 45 lines

+                    GFP_KERNEL);
+    if (drvdata->legacy == NULL)
+        return -ENOMEM;

sound/soc/qcom/lpass-hdmi.c new file +8 -0
+
+    legacy = drvdata->legacy;
+    legacy->legacy_en = devm_regmap_field_alloc(dev, map,
+                v->legacy_en);
+    if (IS_ERR(legacy->legacy_en))
+        return -EINVAL;
+
+    drvdata->vbit_ctl = devm_kzalloc(dev, sizeof(struct 

lpass_vbit_ctrl),
+                    GFP_KERNEL);
+    if (drvdata->vbit_ctl == NULL)
+        return -ENOMEM;
+
+    vbit_ctl = drvdata->vbit_ctl;
+    vbit_ctl->replace_vbit = devm_regmap_field_alloc(dev, map, +                    v->replace_vbit);
+    vbit_ctl->vbit_stream = devm_regmap_field_alloc(dev, map, +                    v->vbit_stream);
+    if (IS_ERR(vbit_ctl->replace_vbit) || IS_ERR(vbit_ctl->vbit_stream))
+        return -EINVAL;

sound/soc/qcom/lpass-hdmi.c new file +21 -0
+
+    drvdata->tx_parity = devm_kzalloc(dev,
+        sizeof(struct  lpass_hdmi_tx_parity), GFP_KERNEL);
+
+    if (drvdata->tx_parity == NULL)
+        return -ENOMEM;
+
+    tx_parity = drvdata->tx_parity;
+    tx_parity->calc_en = devm_regmap_field_alloc(dev, map, v->calc_en);
+
+    if (IS_ERR(tx_parity->calc_en))
+        return -EINVAL;
+
+    drvdata->meta_ctl = devm_kzalloc(dev,
+        sizeof(struct lpass_dp_metadata_ctl), GFP_KERNEL);
+
+    if (drvdata->meta_ctl == NULL)
+        return -ENOMEM;
+
+    meta_ctl = drvdata->meta_ctl;
+    meta_ctl->mute = devm_regmap_field_alloc(dev, map, v->mute);

sound/soc/qcom/lpass-hdmi.c line 105

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+    meta_ctl->as_sdp_cc = devm_regmap_field_alloc(dev, map,
+                        v->as_sdp_cc);
+    meta_ctl->as_sdp_ct = devm_regmap_field_alloc(dev, map,
+                        v->as_sdp_ct);
+    meta_ctl->aif_db4 = devm_regmap_field_alloc(dev, map,
+                        v->aif_db4);
+    meta_ctl->frequency = devm_regmap_field_alloc(dev, map,
+                        v->frequency);
+    meta_ctl->mst_index = devm_regmap_field_alloc(dev, map,
+                        v->mst_index);

<<<<<

Quoted 9 lines

+    meta_ctl->dptx_index = devm_regmap_field_alloc(dev, map, +                        v->dptx_index);
+
+    if (IS_ERR(meta_ctl->mute) || IS_ERR(meta_ctl->as_sdp_cc) ||

+        IS_ERR(meta_ctl->as_sdp_ct) || IS_ERR(meta_ctl->aif_db4) ||
+        IS_ERR(meta_ctl->frequency) || IS_ERR(meta_ctl->mst_index) ||

+        IS_ERR(meta_ctl->dptx_index))
+        return -EINVAL;

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

These type of code is allover the place, why not just add a simple macro to do this.

something like

#define QCOM_REGMAP_FILED_ALLOC(d, m, f)    \
    do { \
        f = devm_regmap_field_alloc(d, m, f);     \
        if (IS_ERR(f))                \
            return -EINVAL;         \
    } while (0)

Quoted 4 lines

and call
    QCOM_REGMAP_FILED_ALLOC(dev, map, meta_ctl->dptx_index);

this will make code much cleaner IMO.

Okay.

Quoted 9 lines

Also we can consider adding some bluk  apis to allocate regmap field
or something like!https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/?h=wip/regmap&id=03551a6e2941d71b99e2e4a5035a9de5f0df6d5d

and call
devm_regmap_field_bulk_alloc(dev, map, &sstream_ctl->sstream_en, &v->sstream_en, 9);

this should work as long as order of reg_field and regmap_field are maintained, which in this case is true!

Okay.

Quoted 297 lines
Quoted 297 lines
sound/soc/qcom/lpass-hdmi.c new file +27 -0
+
+    drvdata->sstream_ctl = devm_kzalloc(dev,
+        sizeof(struct lpass_sstream_ctl), GFP_KERNEL);
+    if (drvdata->sstream_ctl == NULL)
+        return -ENOMEM;
+
+    sstream_ctl = drvdata->sstream_ctl;
+    sstream_ctl->sstream_en = devm_regmap_field_alloc(dev, map,
+                        v->sstream_en);
+    sstream_ctl->dma_sel = devm_regmap_field_alloc(dev, map,
+                        v->dma_sel);
+    sstream_ctl->auto_bbit_en = devm_regmap_field_alloc(dev, map,
+                        v->auto_bbit_en);
+    sstream_ctl->layout = devm_regmap_field_alloc(dev, map,
+                        v->layout);
+    sstream_ctl->layout_sp = devm_regmap_field_alloc(dev, map,
+                        v->layout_sp);
+    sstream_ctl->dp_audio = devm_regmap_field_alloc(dev, map,
+                        v->dp_audio);
+    sstream_ctl->set_sp_on_en = devm_regmap_field_alloc(dev, map,
+                        v->set_sp_on_en);
+    sstream_ctl->dp_staffing_en = devm_regmap_field_alloc(dev, map,
+                        v->dp_staffing_en);
+    sstream_ctl->dp_sp_b_hw_en = devm_regmap_field_alloc(dev, map,
+                        v->dp_sp_b_hw_en);
+
+    if (IS_ERR(sstream_ctl->sstream_en) || 

IS_ERR(sstream_ctl->dma_sel) ||
+        IS_ERR(sstream_ctl->auto_bbit_en) ||
+        IS_ERR(sstream_ctl->layout) ||
IS_ERR(sstream_ctl->layout_sp) ||
+        IS_ERR(sstream_ctl->dp_audio) ||
+        IS_ERR(sstream_ctl->set_sp_on_en) ||
+        IS_ERR(sstream_ctl->dp_staffing_en) ||
+        IS_ERR(sstream_ctl->dp_sp_b_hw_en))
+        return -EINVAL;

sound/soc/qcom/lpass-hdmi.c new file +52 -0
+
+    for (i = 0; i < LPASS_MAX_HDMI_DMA_CHANNELS; i++) {
+        drvdata->ch_msb[i] = devm_kzalloc(dev,
+            sizeof(struct lpass_hdmi_tx_ch_msb), GFP_KERNEL);
+        if (drvdata->ch_msb[i] == NULL)
+            return -ENOMEM;
+
+        ch_msb = drvdata->ch_msb[i];
+
+        ch_msb->msb_bits = devm_regmap_field_alloc(dev, map,
+                            v->msb_bits);
+        if (IS_ERR(ch_msb->msb_bits))
+            return -EINVAL;
+
+        drvdata->ch_lsb[i] = devm_kzalloc(dev,
+            sizeof(struct lpass_hdmi_tx_ch_lsb), GFP_KERNEL);
+        if (drvdata->ch_lsb[i] == NULL)
+            return -ENOMEM;
+
+        ch_lsb = drvdata->ch_lsb[i];
+        ch_lsb->lsb_bits = devm_regmap_field_alloc(dev, map,
+                    v->lsb_bits);
+        if (IS_ERR(ch_lsb->lsb_bits))
+            return -EINVAL;
+
+
+        drvdata->hdmi_tx_dmactl[i] = devm_kzalloc(dev,
+            sizeof(struct lpass_hdmitx_dmactl), GFP_KERNEL);
+        if (drvdata->hdmi_tx_dmactl[i] == NULL)
+            return -ENOMEM;
+
+        hdmi_tx_dmactl = drvdata->hdmi_tx_dmactl[i];
+        hdmi_tx_dmactl->use_hw_chs = devm_regmap_field_alloc(dev, map,
+                        v->use_hw_chs);
+        hdmi_tx_dmactl->use_hw_usr = devm_regmap_field_alloc(dev, map,
+                        v->use_hw_usr);
+        hdmi_tx_dmactl->hw_chs_sel = devm_regmap_field_alloc(dev, map,
+                        v->hw_chs_sel);
+        hdmi_tx_dmactl->hw_usr_sel = devm_regmap_field_alloc(dev, map,
+                        v->hw_usr_sel);
+        if (IS_ERR(hdmi_tx_dmactl->use_hw_chs) ||
+            IS_ERR(hdmi_tx_dmactl->use_hw_usr) ||
+            IS_ERR(hdmi_tx_dmactl->hw_chs_sel) ||
+            IS_ERR(hdmi_tx_dmactl->hw_usr_sel))
+            return -EINVAL;
+    }
+    return 0;
+
+}
+EXPORT_SYMBOL(lpass_hdmi_init_bitfields);
+
+static int lpass_hdmi_daiops_hw_params(struct snd_pcm_substream 

*substream,
+        struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)

sound/soc/qcom/lpass-hdmi.c new file +52 -0
+{
+    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+    snd_pcm_format_t format = params_format(params);
+    unsigned int rate = params_rate(params);
+    unsigned int channels = params_channels(params);
+    unsigned int ret;
+    unsigned int bitwidth;
+    unsigned int word_length;
+    unsigned int ch_sts_buf0;
+    unsigned int ch_sts_buf1;
+    unsigned int data_format;
+    unsigned int sampling_freq;
+    unsigned int ch = 0;
+
+    bitwidth = snd_pcm_format_width(format);
+    if (bitwidth < 0) {
+        dev_err(dai->dev, "%s invalid bit width given : %d\n",
+                    __func__, bitwidth);
+        return bitwidth;
+    }
+
+    switch (bitwidth) {
+    case 16:
+        word_length = LPASS_DP_AUDIO_BITWIDTH16;
+        break;
+    case 24:
+        word_length = LPASS_DP_AUDIO_BITWIDTH24;
+        break;
+    default:
+        dev_err(dai->dev, "%s invalid bit width given : %d\n",
+                    __func__, bitwidth);
+        return -EINVAL;
+    }
+
+    switch (rate) {
+    case 32000:
+        sampling_freq = LPASS_SAMPLING_FREQ32;
+        break;
+    case 44100:
+        sampling_freq = LPASS_SAMPLING_FREQ44;
+        break;
+    case 48000:
+        sampling_freq = LPASS_SAMPLING_FREQ48;
+        break;
+
+    default:
+        dev_err(dai->dev, "%s invalid bit width given : %d\n",
+                    __func__, bitwidth);
+        return -EINVAL;
+    }
+    data_format = LPASS_DATA_FORMAT_LINEAR;
+    ch_sts_buf0 = (((data_format << LPASS_DATA_FORMAT_SHIFT) & 

LPASS_DATA_FORMAT_MASK)
+                | ((sampling_freq << LPASS_FREQ_BIT_SHIFT) & LPASS_FREQ_BIT_MASK));
+    ch_sts_buf1 = (word_length) & LPASS_WORDLENGTH_MASK; +
+    ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_RESET);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_CLEAR);
+    if (ret)
+        return ret;

sound/soc/qcom/lpass-hdmi.c new file +24 -0
+
+    ret = regmap_field_write(drvdata->legacy->legacy_en,
+                LPASS_HDMITX_LEGACY_DISABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->tx_parity->calc_en,
+                HDMITX_PARITY_CALC_EN);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->vbit_ctl->replace_vbit,
+                    REPLACE_VBIT);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->vbit_ctl->vbit_stream,
+                    LINEAR_PCM_DATA);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->ch_msb[0]->msb_bits, 

ch_sts_buf1);
+    if (ret)

sound/soc/qcom/lpass-hdmi.c new file +4 -0
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->ch_lsb[0]->lsb_bits, 

ch_sts_buf0);
+    if (ret)

sound/soc/qcom/lpass-hdmi.c new file +49 -0
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_chs,
+                HW_MODE);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_chs_sel,
+                SW_MODE);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->use_hw_usr,
+                HW_MODE);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->hdmi_tx_dmactl[0]->hw_usr_sel,
+                SW_MODE);
+    if (ret)
+
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->mute,
+                LPASS_MUTE_ENABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->as_sdp_cc,
+                channels - 1);
+    if (ret)
+        return ret;
+
+
+    ret = regmap_field_write(drvdata->meta_ctl->as_sdp_ct,
+                LPASS_META_DEFAULT_VAL);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->aif_db4,
+                LPASS_META_DEFAULT_VAL);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->frequency, 

sampling_freq);
+    if (ret)
+        return ret;

sound/soc/qcom/lpass-hdmi.c new file +56 -0
+
+    ret = regmap_field_write(drvdata->meta_ctl->mst_index,
+                LPASS_META_DEFAULT_VAL);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->dptx_index,
+                LPASS_META_DEFAULT_VAL);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+                LPASS_SSTREAM_DISABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->dma_sel, ch);
+    if (ret)
+        return ret;
+
+
+    ret = regmap_field_write(drvdata->sstream_ctl->auto_bbit_en,
+                LPASS_SSTREAM_DEFAULT_ENABLE);
+    if (ret)
+        return ret;
+
+
+    ret = regmap_field_write(drvdata->sstream_ctl->layout,
+                LPASS_SSTREAM_DEFAULT_DISABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->layout_sp,
+                LPASS_LAYOUT_SP_DEFAULT);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->dp_audio,
+                LPASS_SSTREAM_DEFAULT_ENABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->set_sp_on_en,
+                LPASS_SSTREAM_DEFAULT_ENABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->dp_sp_b_hw_en,
+                LPASS_SSTREAM_DEFAULT_ENABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->dp_staffing_en,
+                LPASS_SSTREAM_DEFAULT_ENABLE);
+    if (ret)
+
Quoted 8 lines

sound/soc/qcom/lpass-hdmi.c line 406

Quoted 7 lines

+        return ret;

sound/soc/qcom/lpass-hdmi.c new file +6 -0
+
+    return ret;
+}
+
+
+

too many empty lines !

Okay. Will remove empty lines!

Quoted 26 lines
Quoted 26 lines

+static int lpass_hdmi_daiops_prepare(struct snd_pcm_substream *substream,
+        struct snd_soc_dai *dai)

sound/soc/qcom/lpass-hdmi.c new file +18 -0
+{
+    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+    int ret;
+
+    ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+                    LPASS_SSTREAM_ENABLE);
+    if (ret)
+        return ret;
+
+    ret = regmap_field_write(drvdata->meta_ctl->mute,
+                    LPASS_MUTE_DISABLE);
+    if (ret)
+        return ret;
+
+    return ret;
+}
+
+static int lpass_hdmi_daiops_trigger(struct snd_pcm_substream 

*substream,
+        int cmd, struct snd_soc_dai *dai)
+{
+    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);

Quoted 8 lines

sound/soc/qcom/lpass-hdmi.c line 436

Quoted 7 lines

+    int ret = -EINVAL;

sound/soc/qcom/lpass-hdmi.c new file +6 -0
+
+    switch (cmd) {
+    case SNDRV_PCM_TRIGGER_START:
+    case SNDRV_PCM_TRIGGER_RESUME:
+    case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+

unnecessary space!

Okay.

Quoted 37 lines
Quoted 37 lines

+        ret = regmap_field_write(drvdata->sstream_ctl->sstream_en, +                    LPASS_SSTREAM_ENABLE);
+        if (ret)
+            return ret;

sound/soc/qcom/lpass-hdmi.c new file +33 -0
+
+        ret = regmap_field_write(drvdata->meta_ctl->mute,
+                    LPASS_MUTE_DISABLE);
+        if (ret)
+            return ret;
+
+        break;
+    case SNDRV_PCM_TRIGGER_STOP:
+    case SNDRV_PCM_TRIGGER_SUSPEND:
+    case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+
+        ret = regmap_field_write(drvdata->sstream_ctl->sstream_en,
+                    LPASS_SSTREAM_DISABLE);
+        if (ret)
+            return ret;
+
+
+        ret = regmap_field_write(drvdata->meta_ctl->mute,
+                    LPASS_MUTE_ENABLE);
+        if (ret)
+            return ret;
+
+        ret = regmap_field_write(drvdata->sstream_ctl->dp_audio, 0);
+        if (ret)
+            return ret;
+
+        break;
+    }
+    return ret;
+}
+
+const struct snd_soc_dai_ops asoc_qcom_lpass_hdmi_dai_ops = {
+    .hw_params    = lpass_hdmi_daiops_hw_params,
Quoted 8 lines

sound/soc/qcom/lpass-hdmi.c line 480

Quoted 7 lines

+    .prepare    = lpass_hdmi_daiops_prepare,
+    .trigger    = lpass_hdmi_daiops_trigger,

sound/soc/qcom/lpass-hdmi.c new file +5 -0
+};
+EXPORT_SYMBOL_GPL(asoc_qcom_lpass_hdmi_dai_ops);
+
+
+

too many empty lines??

Okay.

Quoted 17 lines
Quoted 6 lines

+static bool lpass_hdmi_regmap_writeable(struct device *dev, unsigned int reg)

sound/soc/qcom/lpass-hdmi.c new file +2 -0
+{
+    struct lpass_data *drvdata = dev_get_drvdata(dev);

sound/soc/qcom/lpass-hdmi.c line 493

Quoted 10 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+    struct lpass_variant *v = drvdata->variant;
+    int i;
+
+    if (reg == LPASS_HDMI_TX_CTL_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v))
+        return true;
+

switch case should make this much cleaner.

Okay.

Quoted 43 lines
Quoted 43 lines

+    for (i = 0; i < v->hdmi_rdma_channels; i++) { +        if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i)) +            return true;
+        if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i)) +            return true;
+        if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i)) +            return true;
+    }

sound/soc/qcom/lpass-hdmi.c new file +28 -0
+
+    if (reg == LPASS_HDMI_TX_PARITY_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_DP_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v))
+        return true;
+
+    if (reg == LPASS_HDMITX_APP_IRQEN_REG(v))
+        return true;
+    if (reg == LPASS_HDMITX_APP_IRQCLEAR_REG(v))
+        return true;
+
+    for (i = 0; i < v->rdma_channels; ++i) {
+        if (reg == LPAIF_HDMI_RDMACTL_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMABASE_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMAPER_REG(v, i))
+            return true;
+
+    }
+    return false;
+}
+
+static bool lpass_hdmi_regmap_readable(struct device *dev, unsigned 

int reg)

sound/soc/qcom/lpass-hdmi.c new file +5 -0
+{
+    struct lpass_data *drvdata = dev_get_drvdata(dev);
+    struct lpass_variant *v = drvdata->variant;
+    int i;
+
Quoted 8 lines
Quoted 6 lines

+    if (reg == LPASS_HDMI_TX_CTL_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_LEGACY_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_VBIT_CTL_ADDR(v))
+        return true;

Same here, use switch case

Okay.

Quoted 37 lines
Quoted 37 lines
sound/soc/qcom/lpass-hdmi.c new file +36 -0
+
+    for (i = 0; i < v->hdmi_rdma_channels; i++) {
+        if (reg == LPASS_HDMI_TX_CH_LSB_ADDR(v, i))
+            return true;
+        if (reg == LPASS_HDMI_TX_CH_MSB_ADDR(v, i))
+            return true;
+        if (reg == LPASS_HDMI_TX_DMA_ADDR(v, i))
+            return true;
+    }
 
+
+    if (reg == LPASS_HDMI_TX_PARITY_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_DP_ADDR(v))
+        return true;
+    if (reg == LPASS_HDMI_TX_SSTREAM_ADDR(v))
+        return true;
+
+    if (reg == LPASS_HDMITX_APP_IRQEN_REG(v))
+        return true;
+    if (reg == LPASS_HDMITX_APP_IRQSTAT_REG(v))
+        return true;
+
+    for (i = 0; i < v->rdma_channels; ++i) {
+        if (reg == LPAIF_HDMI_RDMACTL_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMABASE_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMABUFF_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMAPER_REG(v, i))
+            return true;
+        if (reg == LPAIF_HDMI_RDMACURR_REG(v, i))
+            return true;
+    }
+
+    return false;
Quoted 8 lines
Quoted 6 lines
sound/soc/qcom/lpass-hdmi.c new file +3 -0
+}
+
+static bool lpass_hdmi_regmap_volatile(struct device *dev, unsigned 

int reg)
+{
+    return true;

Are all the register range really volatile?

will check this.

Quoted 13 lines

sound/soc/qcom/lpass-hdmi.c line 591

Quoted 11 lines
sound/soc/qcom/lpass-hdmi.c new file +10 -0
+
+}
+struct regmap_config lpass_hdmi_regmap_config = {
+    .reg_bits = 32,
+    .reg_stride = 4,
+    .val_bits = 32,
+    .writeable_reg = lpass_hdmi_regmap_writeable,
+    .readable_reg = lpass_hdmi_regmap_readable,
+    .volatile_reg = lpass_hdmi_regmap_volatile,
+    .cache_type = REGCACHE_FLAT,

Any reason for using FLAT cache type ??

Will check this.

Quoted 29 lines
Quoted 29 lines
sound/soc/qcom/lpass-hdmi.c new file +5 -0
+};
+EXPORT_SYMBOL(lpass_hdmi_regmap_config);
+
+MODULE_DESCRIPTION("QTi LPASS HDMI Driver");
+MODULE_LICENSE("GPL v2");
sound/soc/qcom/lpass-hdmi.h new file +17 -0
new file mode 100644
index 0000000..f91f322
@@ -0,0 +1,129 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ *
+ * lpass_hdmi.h - Definitions for the QTi LPASS HDMI
+ */
+
+#ifndef __LPASS_HDMI_H__
+#define __LPASS_HDMI_H__
+
+#include <linux/clk.h>
+#include <linux/compiler.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define LPASS_HDMITX_LEGACY_DISABLE        0x0
+#define LPASS_HDMITX_LEGACY_ENABLE        0x1
Quoted 8 lines

sound/soc/qcom/lpass-hdmi.h line 23

Quoted 6 lines
sound/soc/qcom/lpass-hdmi.h new file +6 -0
+#define LPASS_DP_AUDIO_BITWIDTH16        0x0
+#define LPASS_DP_AUDIO_BITWIDTH24        0xb
+#define LPASS_DATA_FORMAT_SHIFT            0x1
+#define LPASS_DATA_FORMAT_MASK            0x2
+#define LPASS_FREQ_BIT_SHIFT            24
+#define LPASS_FREQ_BIT_MASK            0xf000000

use GENMASK macro for all such masks.

Okay.

Quoted 28 lines
Quoted 28 lines
sound/soc/qcom/lpass-hdmi.h new file +27 -0
+#define LPASS_DATA_FORMAT_LINEAR 0x0
+#define LPASS_DATA_FORMAT_NON_LINEAR        0x1
+#define LPASS_SAMPLING_FREQ32            0x3
+#define LPASS_SAMPLING_FREQ44            0x0
+#define LPASS_SAMPLING_FREQ48            0x2
+#define LPASS_TX_CTL_RESET            0x1
+#define LPASS_TX_CTL_CLEAR            0x0
+#define LPASS_SSTREAM_ENABLE            1
+#define LPASS_SSTREAM_DISABLE            0
+#define LPASS_LAYOUT_SP_DEFAULT            0xf
+#define LPASS_SSTREAM_DEFAULT_ENABLE        1
+#define LPASS_SSTREAM_DEFAULT_DISABLE        0
+#define LPASS_WORDLENGTH_MASK            0xf
+#define LPASS_MUTE_ENABLE            1
+#define LPASS_MUTE_DISABLE            0
+#define LPASS_META_DEFAULT_VAL            0
+#define HW_MODE                    1
+#define SW_MODE                    0
+#define LEGACY_LPASS_LPAIF            1
+#define LEGACY_LPASS_HDMI            0
+#define REPLACE_VBIT                0x1
+#define LINEAR_PCM_DATA                0x0
+#define NON_LINEAR_PCM_DATA            0x1
+#define HDMITX_PARITY_CALC_EN            0x1
+#define HDMITX_PARITY_CALC_DIS            0x0
+
+
Quoted 8 lines

sound/soc/qcom/lpass-hdmi.h line 57

Quoted 7 lines
sound/soc/qcom/lpass-hdmi.h new file +7 -0
+#define LPASS_HDMI_TX_CTL_ADDR(v) \
+        (v->hdmi_tx_ctl_addr)
+
+#define LPASS_HDMI_TX_LEGACY_ADDR(v) \
+        (v->hdmi_legacy_addr)
+#define LPASS_HDMI_TX_VBIT_CTL_ADDR(v) \
+        (v->hdmi_vbit_addr)

above three macros should be able to go in single lines!! Not sure why you are adding an extra break, which makes it very odd for reading.

Okay.

Quoted 253 lines
Quoted 253 lines
sound/soc/qcom/lpass-hdmi.h new file +72 -0
+#define LPASS_HDMI_TX_CH_LSB_ADDR(v, port) \
+        (v->hdmi_ch_lsb_addr + \
+        v->ch_stride * (port))
+#define LPASS_HDMI_TX_CH_MSB_ADDR(v, port) \
+        (v->hdmi_ch_msb_addr + \
+        v->ch_stride * (port))
+#define LPASS_HDMI_TX_DMA_ADDR(v, port) \
+        (v->hdmi_dmactl_addr + \
+        v->hdmi_dma_stride * (port))
+#define LPASS_HDMI_TX_PARITY_ADDR(v) \
+        (v->hdmi_parity_addr)
+#define LPASS_HDMI_TX_DP_ADDR(v) \
+        (v->hdmi_DP_addr)
+#define LPASS_HDMI_TX_SSTREAM_ADDR(v) \
+        (v->hdmi_sstream_addr)
+
+struct lpass_sstream_ctl {
+    struct regmap_field *sstream_en;
+    struct regmap_field *dma_sel;
+    struct regmap_field *auto_bbit_en;
+    struct regmap_field *layout;
+    struct regmap_field *layout_sp;
+    struct regmap_field *set_sp_on_en;
+    struct regmap_field *dp_audio;
+    struct regmap_field *dp_staffing_en;
+    struct regmap_field *dp_sp_b_hw_en;
+};
+
+struct lpass_dp_metadata_ctl {
+    struct regmap_field *mute;
+    struct regmap_field *as_sdp_cc;
+    struct regmap_field *as_sdp_ct;
+    struct regmap_field *aif_db4;
+    struct regmap_field *frequency;
+    struct regmap_field *mst_index;
+    struct regmap_field *dptx_index;
+};
+
+struct lpass_hdmi_tx_ctl {
+    struct regmap_field *soft_reset;
+    struct regmap_field *force_reset;
+};
+
+struct lpass_hdmitx_dmactl {
+    struct regmap_field *use_hw_chs;
+    struct regmap_field *use_hw_usr;
+    struct regmap_field *hw_chs_sel;
+    struct regmap_field *hw_usr_sel;
+};
+
+struct lpass_vbit_ctrl {
+        struct regmap_field *replace_vbit;
+        struct regmap_field *vbit_stream;
+};
+
+struct  lpass_hdmitx_legacy {
+    struct regmap_field *legacy_en;
+};
+
+struct  lpass_hdmi_tx_parity {
+    struct regmap_field *calc_en;
+};
+
+struct  lpass_hdmi_tx_ch_lsb {
+    struct regmap_field *lsb_bits;
+};
+
+struct  lpass_hdmi_tx_ch_msb {
+    struct regmap_field *msb_bits;
+};
+
+#endif /* __LPASS_HDMI_H__ */
sound/soc/qcom/lpass-ipq806x.c +1 -0
index 72f09b3..832a916 100644
@@ -96,7 +96,7 @@ static int ipq806x_lpass_exit(struct 
 platform_device *pdev)
       return 0;
   }
   -static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
+static int ipq806x_lpass_alloc_dma_channel(struct lpass_data 
 *drvdata, int dir, unsigned int dai_id)
   {

      if (dir == SNDRV_PCM_STREAM_PLAYBACK)
          return IPQ806X_LPAIF_RDMA_CHAN_MI2S;

sound/soc/qcom/lpass-ipq806x.c +1 -0
@@ -104,7 +104,7 @@ static int ipq806x_lpass_alloc_dma_channel(struct 
 lpass_data *drvdata, int dir)
           return -EINVAL;
   }
   -static int ipq806x_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int ipq806x_lpass_free_dma_channel(struct lpass_data 
 *drvdata, int chan, unsigned int dai_id)
   {

      return 0;
  }

sound/soc/qcom/lpass-lpaif-reg.h +2 -1
index 5258e60..42aebd4 100644
@@ -9,7 +9,8 @@
   /* LPAIF I2S */
     #define LPAIF_I2SCTL_REG_ADDR(v, addr, port) \
-    (v->i2sctrl_reg_base + (addr) + v->i2sctrl_reg_stride * (port))
+    (v->i2sctrl_reg_base + (addr) + \
+    v->i2sctrl_reg_stride * (port))
     #define LPAIF_I2SCTL_REG(v, port) LPAIF_I2SCTL_REG_ADDR(v, 0x0, (port))

  @@ -70,6 +71,14 @@
  #define LPAIF_IRQSTAT_REG(v, port)    LPAIF_IRQ_REG_ADDR(v, 0x4, (port)) #define LPAIF_IRQCLEAR_REG(v, port)    LPAIF_IRQ_REG_ADDR(v, 0xC, (port)) +

sound/soc/qcom/lpass-lpaif-reg.h +4 -0
+#define LPASS_HDMITX_APP_IRQ_REG_ADDR(v, addr)  \
+    ((v->hdmi_irq_reg_base) + (addr))
+
+#define LPASS_HDMITX_APP_IRQEN_REG(v) 

LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x4)
+#define LPASS_HDMITX_APP_IRQSTAT_REG(v) LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0x8)
+#define LPASS_HDMITX_APP_IRQCLEAR_REG(v) LPASS_HDMITX_APP_IRQ_REG_ADDR(v, 0xC)
+
  #define LPAIF_IRQ_BITSTRIDE        3
    #define LPAIF_IRQ_PER(chan)        (1 << (LPAIF_IRQ_BITSTRIDE * (chan)))

sound/soc/qcom/lpass-lpaif-reg.h +13 -0
@@ -77,8 +86,22 @@
   #define LPAIF_IRQ_ERR(chan)        (4 << (LPAIF_IRQ_BITSTRIDE * (chan)))
     #define LPAIF_IRQ_ALL(chan)        (7 << (LPAIF_IRQ_BITSTRIDE * (chan)))
+#define LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(chan)    (1 << (14 + chan))
+#define LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(chan)    (1 << (24 + chan))
+#define LPAIF_IRQ_HDMI_METADONE        BIT(23)
     /* LPAIF DMA */
+#define LPAIF_HDMI_RDMA_REG_ADDR(v, addr, chan) \
+    (v->hdmi_rdma_reg_base + (addr) + v->hdmi_rdma_reg_stride * (chan))
+
+#define LPAIF_HDMI_RDMACTL_AUDINTF(id)    (id << 
 LPAIF_RDMACTL_AUDINTF_SHIFT)
+
+#define LPAIF_HDMI_RDMACTL_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, 
 0x00, (chan))
+#define LPAIF_HDMI_RDMABASE_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, 
 0x04, (chan))
+#define    LPAIF_HDMI_RDMABUFF_REG(v, chan) 
 LPAIF_HDMI_RDMA_REG_ADDR(v, 0x08, (chan))
+#define LPAIF_HDMI_RDMACURR_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, 
 0x0C, (chan))
+#define    LPAIF_HDMI_RDMAPER_REG(v, chan) 

LPAIF_HDMI_RDMA_REG_ADDR(v, 0x10, (chan))
+#define    LPAIF_HDMI_RDMAPERCNT_REG(v, chan) LPAIF_HDMI_RDMA_REG_ADDR(v, 0x14, (chan))
    #define LPAIF_RDMA_REG_ADDR(v, addr, chan) \
      (v->rdma_reg_base + (addr) + v->rdma_reg_stride * (chan))

sound/soc/qcom/lpass-lpaif-reg.h +0 -10
@@ -103,17 +126,22 @@
   #define    LPAIF_WRDMAPER_REG(v, chan) LPAIF_WRDMA_REG_ADDR(v, 0x10, (chan))
   #define    LPAIF_WRDMAPERCNT_REG(v, chan) LPAIF_WRDMA_REG_ADDR(v, 0x14, (chan))
   -#define __LPAIF_DMA_REG(v, chan, dir, reg)  \
-    (dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
-        LPAIF_RDMA##reg##_REG(v, chan) : \
-        LPAIF_WRDMA##reg##_REG(v, chan)
-
-#define LPAIF_DMACTL_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, 
 CTL)
-#define LPAIF_DMABASE_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, 
 dir, BASE)
-#define    LPAIF_DMABUFF_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, 
 dir, BUFF)
-#define LPAIF_DMACURR_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, 
 dir, CURR)
-#define    LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, 
 dir, PER)
-#define    LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, 

chan, dir, PERCNT)

sound/soc/qcom/lpass-lpaif-reg.h +11 -0
+#define LPAIF_INTFDMA_REG(v, chan, reg, dai_id)  \
+        ((v->dai_driver[dai_id].id ==  LPASS_DP_RX) ? \
+        LPAIF_HDMI_RDMA##reg##_REG(v, chan) : \
+         LPAIF_RDMA##reg##_REG(v, chan))
+
+#define __LPAIF_DMA_REG(v, chan, dir, reg, dai_id)  \
+    ((dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
+        (LPAIF_INTFDMA_REG(v, chan, reg, dai_id)) : \
+        LPAIF_WRDMA##reg##_REG(v, chan))
+
+#define LPAIF_DMACTL_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, 

chan, dir, CTL, dai_id)
+#define LPAIF_DMABASE_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BASE, dai_id)
+#define    LPAIF_DMABUFF_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BUFF, dai_id)
+#define LPAIF_DMACURR_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, CURR, dai_id)
+#define    LPAIF_DMAPER_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PER, dai_id)
+#define    LPAIF_DMAPERCNT_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PERCNT, dai_id)
    #define LPAIF_DMACTL_BURSTEN_SINGLE    0
  #define LPAIF_DMACTL_BURSTEN_INCR4    1

sound/soc/qcom/lpass-platform.c +2 -7
index 35aead1..e2d1f25 100644
@@ -23,7 +23,7 @@ struct lpass_pcm_data {
       int i2s_port;
   };
   -#define LPASS_PLATFORM_BUFFER_SIZE    (16 * 1024)
+#define LPASS_PLATFORM_BUFFER_SIZE    (24 *  2 * 1024)
   #define LPASS_PLATFORM_PERIODS        2
     static const struct snd_pcm_hardware lpass_platform_pcm_hardware = {
@@ -62,41 +62,79 @@ static int 
 lpass_platform_alloc_dmactl_fields(struct device *dev,
       if (drvdata->rd_dmactl == NULL)
           return -ENOMEM;
   -    drvdata->wr_dmactl = devm_kzalloc(dev, sizeof(struct lpaif_dmactl),
-                      GFP_KERNEL);
-    if (drvdata->wr_dmactl == NULL)
-        return -ENOMEM;
-
       rd_dmactl = drvdata->rd_dmactl;
-    wr_dmactl = drvdata->wr_dmactl;
         rd_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->rdma_bursten);
       rd_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->rdma_wpscnt);
       rd_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->rdma_fifowm);
-    rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf);
       rd_dmactl->enable = devm_regmap_field_alloc(dev, map, v->rdma_enable);
       rd_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->rdma_dyncclk);
-
+    rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf);
      if (IS_ERR(rd_dmactl->bursten) || IS_ERR(rd_dmactl->wpscnt) ||
-        IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->intf) ||
-        IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->dyncclk))
+        IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->dyncclk) ||
+        IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->intf))

          return -EINVAL;
+    drvdata->wr_dmactl = devm_kzalloc(dev,
+            sizeof(struct lpaif_dmactl), GFP_KERNEL);
+    if (drvdata->wr_dmactl == NULL)
+        return -ENOMEM;
  -    wr_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->wrdma_bursten); -    wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->wrdma_wpscnt);
-    wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->wrdma_fifowm);
-    wr_dmactl->intf = devm_regmap_field_alloc(dev, map, v->wrdma_intf); -    wr_dmactl->enable = devm_regmap_field_alloc(dev, map, v->wrdma_enable);
-    wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->wrdma_dyncclk);
+    wr_dmactl = drvdata->wr_dmactl;
+
+    wr_dmactl->bursten = devm_regmap_field_alloc(dev, map,
+                        v->wrdma_bursten);
+    wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map,
+                        v->wrdma_wpscnt);
+    wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map,
+                        v->wrdma_fifowm);

Quoted 8 lines
Quoted 6 lines

+    wr_dmactl->intf = devm_regmap_field_alloc(dev, map, +                        v->wrdma_intf);
+    wr_dmactl->enable = devm_regmap_field_alloc(dev, map, +                        v->wrdma_enable);
+    wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, +                        v->wrdma_dyncclk);

Can you explain what is happening here, these chagnes just look like spllitting one line into two!

Will bring them in same line. As previous code has long lines and not taken care after modification.

Quoted 11 lines

I should stop reviewing at this point!
As I have seen some general issues with the patch like
- doing un-needed changes!
- removing #ifdef checks.
- try to come up with some api/macro to simplify field allocs as they seems to be all over the place!

thanks,
srini

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
#14 lore raw

PATCH v6 5/5 ASoC: qcom: sc7180: Add support for audio over DP

In reply to Srinivasa Rao Mandadapu

From: V Sujith Kumar Reddy <[email protected]>

Add support for audio playback over DP in lpass sc7180 platform driver. Update lpass_variant structure for hdmi data configuaration.

---
 sound/soc/qcom/lpass-sc7180.c | 116 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 102 insertions(+), 14 deletions(-)
sound/soc/qcom/lpass-sc7180.c +102 -14
index 167bf2c..59f115e 100644
@@ -60,38 +60,65 @@ static struct snd_soc_dai_driver sc7180_lpass_cpu_dai_driver[] = {
 		.probe	= &asoc_qcom_lpass_cpu_dai_probe,
 		.ops    = &asoc_qcom_lpass_cpu_dai_ops,
 	},
+	[LPASS_DP_RX] = {
+		.id = LPASS_DP_RX,
+		.name = "Hdmi",
+		.playback = {
+			.stream_name = "Hdmi Playback",
+			.formats	= SNDRV_PCM_FMTBIT_S24,
+			.rates = SNDRV_PCM_RATE_48000,
+			.rate_min	= 48000,
+			.rate_max	= 48000,
+			.channels_min	= 2,
+			.channels_max	= 2,
+		},
+		.ops    = &asoc_qcom_lpass_hdmi_dai_ops,
+	},
 };
 
 static int sc7180_lpass_alloc_dma_channel(struct lpass_data *drvdata,
-					   int direction)
+					   int direction, unsigned int dai_id)
 {
 	struct lpass_variant *v = drvdata->variant;
 	int chan = 0;
 
-	if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
-		chan = find_first_zero_bit(&drvdata->dma_ch_bit_map,
-					v->rdma_channels);
+	if (dai_id == LPASS_DP_RX) {
+		if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
+			chan = find_first_zero_bit(&drvdata->hdmi_dma_ch_bit_map,
+						v->hdmi_rdma_channels);
+
+			if (chan >= v->hdmi_rdma_channels)
+				return -EBUSY;
+		}
+		set_bit(chan, &drvdata->hdmi_dma_ch_bit_map);
+	} else {
+		if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
+			chan = find_first_zero_bit(&drvdata->dma_ch_bit_map,
+						v->rdma_channels);
 
 		if (chan >= v->rdma_channels)
 			return -EBUSY;
-	} else {
-		chan = find_next_zero_bit(&drvdata->dma_ch_bit_map,
+		} else {
+			chan = find_next_zero_bit(&drvdata->dma_ch_bit_map,
 					v->wrdma_channel_start +
 					v->wrdma_channels,
 					v->wrdma_channel_start);
 
-		if (chan >=  v->wrdma_channel_start + v->wrdma_channels)
-			return -EBUSY;
-	}
-
-	set_bit(chan, &drvdata->dma_ch_bit_map);
+			if (chan >=  v->wrdma_channel_start + v->wrdma_channels)
+				return -EBUSY;
+		}
 
+		set_bit(chan, &drvdata->dma_ch_bit_map);
+	}
 	return chan;
 }
 
-static int sc7180_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+static int sc7180_lpass_free_dma_channel(struct lpass_data *drvdata, int chan, unsigned int dai_id)
 {
-	clear_bit(chan, &drvdata->dma_ch_bit_map);
+	if (dai_id == LPASS_DP_RX)
+		clear_bit(chan, &drvdata->hdmi_dma_ch_bit_map);
+	else
+		clear_bit(chan, &drvdata->dma_ch_bit_map);
 
 	return 0;
 }
@@ -144,6 +171,9 @@ static struct lpass_variant sc7180_data = {
 	.rdma_reg_base		= 0xC000,
 	.rdma_reg_stride	= 0x1000,
 	.rdma_channels		= 5,
+	.hdmi_rdma_reg_base		= 0x64000,
+	.hdmi_rdma_reg_stride	= 0x1000,
+	.hdmi_rdma_channels		= 4,
 	.dmactl_audif_start	= 1,
 	.wrdma_reg_base		= 0x18000,
 	.wrdma_reg_stride	= 0x1000,
@@ -163,7 +193,7 @@ static struct lpass_variant sc7180_data = {
 	.rdma_dyncclk		= REG_FIELD_ID(0xC000, 21, 21, 5, 0x1000),
 	.rdma_bursten		= REG_FIELD_ID(0xC000, 20, 20, 5, 0x1000),
 	.rdma_wpscnt		= REG_FIELD_ID(0xC000, 16, 19, 5, 0x1000),
-	.rdma_intf		= REG_FIELD_ID(0xC000, 12, 15, 5, 0x1000),
+	.rdma_intf			= REG_FIELD_ID(0xC000, 12, 15, 5, 0x1000),
 	.rdma_fifowm		= REG_FIELD_ID(0xC000, 1, 5, 5, 0x1000),
 	.rdma_enable		= REG_FIELD_ID(0xC000, 0, 0, 5, 0x1000),
 
@@ -174,6 +204,64 @@ static struct lpass_variant sc7180_data = {
 	.wrdma_fifowm		= REG_FIELD_ID(0x18000, 1, 5, 4, 0x1000),
 	.wrdma_enable		= REG_FIELD_ID(0x18000, 0, 0, 4, 0x1000),
 
+	.hdmi_tx_ctl_addr	= 0x1000,
+	.hdmi_legacy_addr	= 0x1008,
+	.hdmi_vbit_addr		= 0x610c0,
+	.hdmi_ch_lsb_addr	= 0x61048,
+	.hdmi_ch_msb_addr	= 0x6104c,
+	.ch_stride		= 0x8,
+	.hdmi_parity_addr	= 0x61034,
+	.hdmi_dmactl_addr	= 0x61038,
+	.hdmi_dma_stride	= 0x4,
+	.hdmi_DP_addr		= 0x610c8,
+	.hdmi_sstream_addr	= 0x6101c,
+	.hdmi_irq_reg_base		= 0x63000,
+	.hdmi_irq_ports		= 1,
+
+	.hdmi_rdma_dyncclk		= REG_FIELD_ID(0x64000, 14, 14, 4, 0x1000),
+	.hdmi_rdma_bursten		= REG_FIELD_ID(0x64000, 13, 13, 4, 0x1000),
+	.hdmi_rdma_burst8		= REG_FIELD_ID(0x64000, 15, 15, 4, 0x1000),
+	.hdmi_rdma_burst16		= REG_FIELD_ID(0x64000, 16, 16, 4, 0x1000),
+	.hdmi_rdma_dynburst		= REG_FIELD_ID(0x64000, 18, 18, 4, 0x1000),
+	.hdmi_rdma_wpscnt		= REG_FIELD_ID(0x64000, 10, 12, 4, 0x1000),
+	.hdmi_rdma_fifowm		= REG_FIELD_ID(0x64000, 1, 5, 4, 0x1000),
+	.hdmi_rdma_enable		= REG_FIELD_ID(0x64000, 0, 0, 4, 0x1000),
+
+	.sstream_en		= REG_FIELD(0x6101c, 0, 0),
+	.dma_sel			= REG_FIELD(0x6101c, 1, 2),
+	.auto_bbit_en	= REG_FIELD(0x6101c, 3, 3),
+	.layout			= REG_FIELD(0x6101c, 4, 4),
+	.layout_sp		= REG_FIELD(0x6101c, 5, 8),
+	.set_sp_on_en	= REG_FIELD(0x6101c, 10, 10),
+	.dp_audio		= REG_FIELD(0x6101c, 11, 11),
+	.dp_staffing_en	= REG_FIELD(0x6101c, 12, 12),
+	.dp_sp_b_hw_en	= REG_FIELD(0x6101c, 13, 13),
+
+	.mute			= REG_FIELD(0x610c8, 0, 0),
+	.as_sdp_cc		= REG_FIELD(0x610c8, 1, 3),
+	.as_sdp_ct		= REG_FIELD(0x610c8, 4, 7),
+	.aif_db4			= REG_FIELD(0x610c8, 8, 15),
+	.frequency		= REG_FIELD(0x610c8, 16, 21),
+	.mst_index		= REG_FIELD(0x610c8, 28, 29),
+	.dptx_index		= REG_FIELD(0x610c8, 30, 31),
+
+	.soft_reset		= REG_FIELD(0x1000, 31, 31),
+	.force_reset	= REG_FIELD(0x1000, 30, 30),
+
+	.use_hw_chs		= REG_FIELD(0x61038, 0, 0),
+	.use_hw_usr		= REG_FIELD(0x61038, 1, 1),
+	.hw_chs_sel		= REG_FIELD(0x61038, 2, 4),
+	.hw_usr_sel		= REG_FIELD(0x61038, 5, 6),
+
+	.replace_vbit	= REG_FIELD(0x610c0, 0, 0),
+	.vbit_stream	= REG_FIELD(0x610c0, 1, 1),
+
+	.legacy_en		=  REG_FIELD(0x1008, 0, 0),
+	.calc_en		=  REG_FIELD(0x61034, 0, 0),
+	.lsb_bits		=  REG_FIELD(0x61048, 0, 31),
+	.msb_bits		=  REG_FIELD(0x6104c, 0, 31),
+
+
 	.clk_name		= (const char*[]) {
 				   "pcnoc-sway-clk",
 				   "audio-core",
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.