[PATCH v1 0/6] drm/mediatek: Add mt8195 DisplayPort driver

[PATCH v1 0/6] drm/mediatek: Add mt8195 DisplayPort driver

[Markus Schneider-Pargmann]

Hi everyone,

this series is built around the DisplayPort driver. The dpi/dpintf driver and
the added helper functions are required for the DisplayPort driver to work.

It is version 1 of the patch series following the RFC version:
https://lore.kernel.org/linux-mediatek/20210816192523.1739365-1-msp@baylibre.com/

Note: This patch series is currently tested on v5.10 and I am currently working
on testing it on v5.14.

The series is now based on these patch series and its dependencies:
- Add Mediatek Soc DRM (vdosys0) support for mt8195
  https://lore.kernel.org/linux-mediatek/20210825144833.7757-1-jason-jh.lin@mediatek.com/
- Add MediaTek SoC DRM (vdosys1) support for mt8195
  https://lore.kernel.org/linux-mediatek/20210825100531.5653-1-nancy.lin@mediatek.com/

Changes in v1:
- Added DP binding documentation.
- Addressed feedback from the RFC.
- General cleanups in DPI and DP drivers.
- See individual patches for details on the changes done.

Thanks in advance for any feedback and comments.

Best,
Markus


Markus Schneider-Pargmann (6):
  dt-bindings: mediatek,dpi: Add mt8195 dpintf
  dt-bindings: mediatek,dp: Add Display Port binding
  drm/edid: Add cea_sad helpers for freq/length
  video/hdmi: Add audio_infoframe packing for DP
  drm/mediatek: dpi: Add dpintf support
  drm/mediatek: Add mt8195 DisplayPort driver

 .../display/mediatek/mediatek,dp.yaml         |   89 +
 .../display/mediatek/mediatek,dpi.yaml        |   43 +-
 drivers/gpu/drm/drm_edid.c                    |   57 +
 drivers/gpu/drm/mediatek/Kconfig              |    6 +
 drivers/gpu/drm/mediatek/Makefile             |    2 +
 drivers/gpu/drm/mediatek/mtk_dp.c             | 2881 +++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_dp_reg.h         |  580 ++++
 drivers/gpu/drm/mediatek/mtk_dpi.c            |  248 +-
 drivers/gpu/drm/mediatek/mtk_dpi_regs.h       |   12 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |    4 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |    1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |    3 +
 drivers/video/hdmi.c                          |   87 +-
 include/drm/drm_edid.h                        |   18 +-
 include/linux/hdmi.h                          |    4 +
 15 files changed, 3958 insertions(+), 77 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
 create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
 create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h

-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/6] dt-bindings: mediatek,dpi: Add mt8195 dpintf

[Markus Schneider-Pargmann]

DP_INTF is similar to the actual dpi. They differ in some points
regarding registers and what needs to be set but the function blocks
itself are similar in design.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 .../display/mediatek/mediatek,dpi.yaml        | 43 ++++++++++++++++---
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
index dd2896a40ff0..1a158b719ce6 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: mediatek DPI Controller Device Tree Bindings
+title: mediatek DPI/DP_INTF Controller Device Tree Bindings
 
 maintainers:
   - CK Hu <ck.hu@mediatek.com>
@@ -13,7 +13,8 @@ maintainers:
 description: |
   The Mediatek DPI function block is a sink of the display subsystem and
   provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
-  output bus.
+  output bus. The Mediatek DP_INTF is a similar function block that is
+  connected to the (embedded) display port function block.
 
 properties:
   compatible:
@@ -23,6 +24,7 @@ properties:
       - mediatek,mt8173-dpi
       - mediatek,mt8183-dpi
       - mediatek,mt8192-dpi
+      - mediatek,mt8195-dpintf
 
   reg:
     maxItems: 1
@@ -37,10 +39,11 @@ properties:
       - description: DPI PLL
 
   clock-names:
-    items:
-      - const: pixel
-      - const: engine
-      - const: pll
+    description:
+      For dpi clocks pixel, engine and pll are required. For dpintf pixel,
+      hf_fmm and hf_fdp are required.
+    minItems: 3
+    maxItems: 3
 
   pinctrl-0: true
   pinctrl-1: true
@@ -64,6 +67,34 @@ required:
   - clock-names
   - port
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8195-dpintf
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pixel
+            - const: hf_fmm
+            - const: hf_fdp
+    else:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pixel
+            - const: engine
+            - const: pll
+
 additionalProperties: false
 
 examples:
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/6] dt-bindings: mediatek,dpi: Add mt8195 dpintf

[Sam Ravnborg]

Hi Markus,

On Mon, Sep 06, 2021 at 09:35:24PM +0200, Markus Schneider-Pargmann wrote:
> DP_INTF is similar to the actual dpi. They differ in some points
> regarding registers and what needs to be set but the function blocks
> itself are similar in design.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

I fail to see why they share the same dt-schema as the main content in
the schema is the clocks and they differ.

A new mediatek,dpintf schema seems more appropriate.

I recall I though so when reading the RFC variant but failed to comment on it.

	Sam

> ---
>  .../display/mediatek/mediatek,dpi.yaml        | 43 ++++++++++++++++---
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> index dd2896a40ff0..1a158b719ce6 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: mediatek DPI Controller Device Tree Bindings
> +title: mediatek DPI/DP_INTF Controller Device Tree Bindings
>  
>  maintainers:
>    - CK Hu <ck.hu@mediatek.com>
> @@ -13,7 +13,8 @@ maintainers:
>  description: |
>    The Mediatek DPI function block is a sink of the display subsystem and
>    provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
> -  output bus.
> +  output bus. The Mediatek DP_INTF is a similar function block that is
> +  connected to the (embedded) display port function block.
>  
>  properties:
>    compatible:
> @@ -23,6 +24,7 @@ properties:
>        - mediatek,mt8173-dpi
>        - mediatek,mt8183-dpi
>        - mediatek,mt8192-dpi
> +      - mediatek,mt8195-dpintf
>  
>    reg:
>      maxItems: 1
> @@ -37,10 +39,11 @@ properties:
>        - description: DPI PLL
>  
>    clock-names:
> -    items:
> -      - const: pixel
> -      - const: engine
> -      - const: pll
> +    description:
> +      For dpi clocks pixel, engine and pll are required. For dpintf pixel,
> +      hf_fmm and hf_fdp are required.
> +    minItems: 3
> +    maxItems: 3
>  
>    pinctrl-0: true
>    pinctrl-1: true
> @@ -64,6 +67,34 @@ required:
>    - clock-names
>    - port
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8195-dpintf
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pixel
> +            - const: hf_fmm
> +            - const: hf_fdp
> +    else:
> +      properties:
> +        clocks:
> +          minItems: 3
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pixel
> +            - const: engine
> +            - const: pll
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/6] dt-bindings: mediatek,dpi: Add mt8195 dpintf

[Markus Schneider-Pargmann]

Hi Sam,

On Mon, Sep 06, 2021 at 10:14:05PM +0200, Sam Ravnborg wrote:
> Hi Markus,
> 
> On Mon, Sep 06, 2021 at 09:35:24PM +0200, Markus Schneider-Pargmann wrote:
> > DP_INTF is similar to the actual dpi. They differ in some points
> > regarding registers and what needs to be set but the function blocks
> > itself are similar in design.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> 
> I fail to see why they share the same dt-schema as the main content in
> the schema is the clocks and they differ.
> 
> A new mediatek,dpintf schema seems more appropriate.

Good idea, I will do that. Thank you.

Best,
Markus

> 
> I recall I though so when reading the RFC variant but failed to comment on it.
> 
> 	Sam
> 
> > ---
> >  .../display/mediatek/mediatek,dpi.yaml        | 43 ++++++++++++++++---
> >  1 file changed, 37 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > index dd2896a40ff0..1a158b719ce6 100644
> > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> > @@ -4,7 +4,7 @@
> >  $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: mediatek DPI Controller Device Tree Bindings
> > +title: mediatek DPI/DP_INTF Controller Device Tree Bindings
> >  
> >  maintainers:
> >    - CK Hu <ck.hu@mediatek.com>
> > @@ -13,7 +13,8 @@ maintainers:
> >  description: |
> >    The Mediatek DPI function block is a sink of the display subsystem and
> >    provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
> > -  output bus.
> > +  output bus. The Mediatek DP_INTF is a similar function block that is
> > +  connected to the (embedded) display port function block.
> >  
> >  properties:
> >    compatible:
> > @@ -23,6 +24,7 @@ properties:
> >        - mediatek,mt8173-dpi
> >        - mediatek,mt8183-dpi
> >        - mediatek,mt8192-dpi
> > +      - mediatek,mt8195-dpintf
> >  
> >    reg:
> >      maxItems: 1
> > @@ -37,10 +39,11 @@ properties:
> >        - description: DPI PLL
> >  
> >    clock-names:
> > -    items:
> > -      - const: pixel
> > -      - const: engine
> > -      - const: pll
> > +    description:
> > +      For dpi clocks pixel, engine and pll are required. For dpintf pixel,
> > +      hf_fmm and hf_fdp are required.
> > +    minItems: 3
> > +    maxItems: 3
> >  
> >    pinctrl-0: true
> >    pinctrl-1: true
> > @@ -64,6 +67,34 @@ required:
> >    - clock-names
> >    - port
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - mediatek,mt8195-dpintf
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> > +          maxItems: 3
> > +        clock-names:
> > +          items:
> > +            - const: pixel
> > +            - const: hf_fmm
> > +            - const: hf_fdp
> > +    else:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> > +          maxItems: 3
> > +        clock-names:
> > +          items:
> > +            - const: pixel
> > +            - const: engine
> > +            - const: pll
> > +
> >  additionalProperties: false
> >  
> >  examples:
> > -- 
> > 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/6] dt-bindings: mediatek,dp: Add Display Port binding

[Markus Schneider-Pargmann]

This controller is present on different mediatek hardware. Currently
mt8195 and mt8395 have this controller without a functional difference,
so only one compatible is added.

The controller can be in two forms, for a normal display port and for
embedded display port.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    I added the mediatek maintainers in the list of maintainers as I wasn't sure
    whom I should put there. Please let me know if I am supposed to add my mail
    there.

 .../display/mediatek/mediatek,dp.yaml         | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
new file mode 100644
index 000000000000..f7a35962c23b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Display Port Controller
+
+maintainers:
+  - CK Hu <ck.hu@mediatek.com>
+  - Jitao shi <jitao.shi@mediatek.com>
+
+description: |
+  Device tree bindings for the Mediatek (embedded) Display Port controller
+  present on some Mediatek SoCs.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8195-edp_tx
+      - mediatek,mt8195-dp_tx
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: faxi clock
+
+  clock-names:
+    items:
+      - const: faxi
+
+  power-domains:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Input endpoint of the controller, usually dp_intf
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Output endpoint of the controller
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/mt8195-power.h>
+    dp_tx: edp_tx@1c500000 {
+        compatible = "mediatek,mt8195-edp_tx";
+        reg = <0 0x1c500000 0 0x8000>;
+        interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>;
+        power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&edp_pin>;
+        status = "okay";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                edp_in: endpoint {
+                    remote-endpoint = <&dp_intf0_out>;
+                };
+            };
+            port@1 {
+                reg = <1>;
+                edp_out: endpoint {
+                	remote-endpoint = <&panel_in>;
+                };
+            };
+        };
+    };
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/6] drm/edid: Add cea_sad helpers for freq/length

[Markus Schneider-Pargmann]

This patch adds two helper functions that extract the frequency and word
length from a struct cea_sad.

For these helper functions new defines are added that help translate the
'freq' and 'byte2' fields into real numbers.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/gpu/drm/drm_edid.c | 57 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     | 18 ++++++++++--
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 81d5f2524246..2389d34ce10e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4666,6 +4666,63 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
 }
 EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
 
+/**
+ * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad frequency field and returns the sample rate in Hz.
+ *
+ * Return: Sample rate in Hz or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_sample_rate(struct cea_sad *sad)
+{
+	switch (sad->freq) {
+	case CEA_SAD_FREQ_32KHZ:
+		return 32000;
+	case CEA_SAD_FREQ_44KHZ:
+		return 44100;
+	case CEA_SAD_FREQ_48KHZ:
+		return 48000;
+	case CEA_SAD_FREQ_88KHZ:
+		return 88200;
+	case CEA_SAD_FREQ_96KHZ:
+		return 96000;
+	case CEA_SAD_FREQ_176KHZ:
+		return 176400;
+	case CEA_SAD_FREQ_192KHZ:
+		return 192000;
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
+
+/**
+ * drm_cea_sad_get_uncompressed_word_length - Extract word length
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad byte2 field and returns the word length for an
+ * uncompressed stream.
+ *
+ * Note: This function may only be called for uncompressed audio.
+ *
+ * Return: Word length in bits or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad)
+{
+	switch (sad->byte2) {
+	case CEA_SAD_UNCOMPRESSED_WORD_16BIT:
+		return 16;
+	case CEA_SAD_UNCOMPRESSED_WORD_20BIT:
+		return 20;
+	case CEA_SAD_UNCOMPRESSED_WORD_24BIT:
+		return 24;
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length);
+
 /**
  * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
  * @connector: connector associated with the HDMI/DP sink
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 759328a5eeb2..bed091a749ef 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -361,12 +361,24 @@ struct edid {
 
 /* Short Audio Descriptor */
 struct cea_sad {
-	u8 format;
+	u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */
 	u8 channels; /* max number of channels - 1 */
-	u8 freq;
+	u8 freq; /* See CEA_SAD_FREQ_* */
 	u8 byte2; /* meaning depends on format */
 };
 
+#define CEA_SAD_FREQ_32KHZ  BIT(0)
+#define CEA_SAD_FREQ_44KHZ  BIT(1)
+#define CEA_SAD_FREQ_48KHZ  BIT(2)
+#define CEA_SAD_FREQ_88KHZ  BIT(3)
+#define CEA_SAD_FREQ_96KHZ  BIT(4)
+#define CEA_SAD_FREQ_176KHZ BIT(5)
+#define CEA_SAD_FREQ_192KHZ BIT(6)
+
+#define CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0)
+#define CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1)
+#define CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2)
+
 struct drm_encoder;
 struct drm_connector;
 struct drm_connector_state;
@@ -374,6 +386,8 @@ struct drm_display_mode;
 
 int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
 int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
+int drm_cea_sad_get_sample_rate(struct cea_sad *sad);
+int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad);
 int drm_av_sync_delay(struct drm_connector *connector,
 		      const struct drm_display_mode *mode);
 
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/6] drm/edid: Add cea_sad helpers for freq/length

[Sam Ravnborg]

Hi Markus,

On Mon, Sep 06, 2021 at 09:35:26PM +0200, Markus Schneider-Pargmann wrote:
> This patch adds two helper functions that extract the frequency and word
> length from a struct cea_sad.
> 
> For these helper functions new defines are added that help translate the
> 'freq' and 'byte2' fields into real numbers.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 57 ++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     | 18 ++++++++++--
>  2 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 81d5f2524246..2389d34ce10e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4666,6 +4666,63 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
>  }
>  EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
>  
> +/**
> + * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
> + * @sad: Pointer to the cea_sad struct
> + *
> + * Extracts the cea_sad frequency field and returns the sample rate in Hz.
> + *
> + * Return: Sample rate in Hz or a negative errno if parsing failed.
> + */
> +int drm_cea_sad_get_sample_rate(struct cea_sad *sad)

It would be nice to use const struct cea_sad *sad here.

> +{
> +	switch (sad->freq) {
> +	case CEA_SAD_FREQ_32KHZ:
> +		return 32000;
> +	case CEA_SAD_FREQ_44KHZ:
> +		return 44100;
> +	case CEA_SAD_FREQ_48KHZ:
> +		return 48000;
> +	case CEA_SAD_FREQ_88KHZ:
> +		return 88200;
> +	case CEA_SAD_FREQ_96KHZ:
> +		return 96000;
> +	case CEA_SAD_FREQ_176KHZ:
> +		return 176400;
> +	case CEA_SAD_FREQ_192KHZ:
> +		return 192000;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
> +
> +/**
> + * drm_cea_sad_get_uncompressed_word_length - Extract word length
> + * @sad: Pointer to the cea_sad struct
> + *
> + * Extracts the cea_sad byte2 field and returns the word length for an
> + * uncompressed stream.
> + *
> + * Note: This function may only be called for uncompressed audio.
Can you check or this and WARN (or drm_WARN) if this is not the case?

> + *
> + * Return: Word length in bits or a negative errno if parsing failed.
> + */
> +int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad)
Again, consider to use const.

	Sam

> +{
> +	switch (sad->byte2) {
> +	case CEA_SAD_UNCOMPRESSED_WORD_16BIT:
> +		return 16;
> +	case CEA_SAD_UNCOMPRESSED_WORD_20BIT:
> +		return 20;
> +	case CEA_SAD_UNCOMPRESSED_WORD_24BIT:
> +		return 24;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length);
> +
>  /**
>   * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
>   * @connector: connector associated with the HDMI/DP sink
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 759328a5eeb2..bed091a749ef 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -361,12 +361,24 @@ struct edid {
>  
>  /* Short Audio Descriptor */
>  struct cea_sad {
> -	u8 format;
> +	u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */
>  	u8 channels; /* max number of channels - 1 */
> -	u8 freq;
> +	u8 freq; /* See CEA_SAD_FREQ_* */
>  	u8 byte2; /* meaning depends on format */
>  };
>  
> +#define CEA_SAD_FREQ_32KHZ  BIT(0)
> +#define CEA_SAD_FREQ_44KHZ  BIT(1)
> +#define CEA_SAD_FREQ_48KHZ  BIT(2)
> +#define CEA_SAD_FREQ_88KHZ  BIT(3)
> +#define CEA_SAD_FREQ_96KHZ  BIT(4)
> +#define CEA_SAD_FREQ_176KHZ BIT(5)
> +#define CEA_SAD_FREQ_192KHZ BIT(6)
> +
> +#define CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0)
> +#define CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1)
> +#define CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2)
> +
>  struct drm_encoder;
>  struct drm_connector;
>  struct drm_connector_state;
> @@ -374,6 +386,8 @@ struct drm_display_mode;
>  
>  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
>  int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
> +int drm_cea_sad_get_sample_rate(struct cea_sad *sad);
> +int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad);
>  int drm_av_sync_delay(struct drm_connector *connector,
>  		      const struct drm_display_mode *mode);
>  
> -- 
> 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/6] drm/edid: Add cea_sad helpers for freq/length

[Markus Schneider-Pargmann]

Hi Sam,

On Mon, Sep 06, 2021 at 10:19:00PM +0200, Sam Ravnborg wrote:
> Hi Markus,
> 
> On Mon, Sep 06, 2021 at 09:35:26PM +0200, Markus Schneider-Pargmann wrote:
> > This patch adds two helper functions that extract the frequency and word
> > length from a struct cea_sad.
> > 
> > For these helper functions new defines are added that help translate the
> > 'freq' and 'byte2' fields into real numbers.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c | 57 ++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_edid.h     | 18 ++++++++++--
> >  2 files changed, 73 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 81d5f2524246..2389d34ce10e 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4666,6 +4666,63 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
> >  }
> >  EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
> >  
> > +/**
> > + * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
> > + * @sad: Pointer to the cea_sad struct
> > + *
> > + * Extracts the cea_sad frequency field and returns the sample rate in Hz.
> > + *
> > + * Return: Sample rate in Hz or a negative errno if parsing failed.
> > + */
> > +int drm_cea_sad_get_sample_rate(struct cea_sad *sad)
> 
> It would be nice to use const struct cea_sad *sad here.

Thanks, fixing it for the next version.

> 
> > +{
> > +	switch (sad->freq) {
> > +	case CEA_SAD_FREQ_32KHZ:
> > +		return 32000;
> > +	case CEA_SAD_FREQ_44KHZ:
> > +		return 44100;
> > +	case CEA_SAD_FREQ_48KHZ:
> > +		return 48000;
> > +	case CEA_SAD_FREQ_88KHZ:
> > +		return 88200;
> > +	case CEA_SAD_FREQ_96KHZ:
> > +		return 96000;
> > +	case CEA_SAD_FREQ_176KHZ:
> > +		return 176400;
> > +	case CEA_SAD_FREQ_192KHZ:
> > +		return 192000;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
> > +
> > +/**
> > + * drm_cea_sad_get_uncompressed_word_length - Extract word length
> > + * @sad: Pointer to the cea_sad struct
> > + *
> > + * Extracts the cea_sad byte2 field and returns the word length for an
> > + * uncompressed stream.
> > + *
> > + * Note: This function may only be called for uncompressed audio.
> Can you check or this and WARN (or drm_WARN) if this is not the case?

Good idea, I added a check, a DRM_WARN and return -EINVAL now.

Thanks,
Markus

> 
> > + *
> > + * Return: Word length in bits or a negative errno if parsing failed.
> > + */
> > +int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad)
> Again, consider to use const.
> 
> 	Sam
> 
> > +{
> > +	switch (sad->byte2) {
> > +	case CEA_SAD_UNCOMPRESSED_WORD_16BIT:
> > +		return 16;
> > +	case CEA_SAD_UNCOMPRESSED_WORD_20BIT:
> > +		return 20;
> > +	case CEA_SAD_UNCOMPRESSED_WORD_24BIT:
> > +		return 24;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length);
> > +
> >  /**
> >   * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
> >   * @connector: connector associated with the HDMI/DP sink
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 759328a5eeb2..bed091a749ef 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -361,12 +361,24 @@ struct edid {
> >  
> >  /* Short Audio Descriptor */
> >  struct cea_sad {
> > -	u8 format;
> > +	u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */
> >  	u8 channels; /* max number of channels - 1 */
> > -	u8 freq;
> > +	u8 freq; /* See CEA_SAD_FREQ_* */
> >  	u8 byte2; /* meaning depends on format */
> >  };
> >  
> > +#define CEA_SAD_FREQ_32KHZ  BIT(0)
> > +#define CEA_SAD_FREQ_44KHZ  BIT(1)
> > +#define CEA_SAD_FREQ_48KHZ  BIT(2)
> > +#define CEA_SAD_FREQ_88KHZ  BIT(3)
> > +#define CEA_SAD_FREQ_96KHZ  BIT(4)
> > +#define CEA_SAD_FREQ_176KHZ BIT(5)
> > +#define CEA_SAD_FREQ_192KHZ BIT(6)
> > +
> > +#define CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0)
> > +#define CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1)
> > +#define CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2)
> > +
> >  struct drm_encoder;
> >  struct drm_connector;
> >  struct drm_connector_state;
> > @@ -374,6 +386,8 @@ struct drm_display_mode;
> >  
> >  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
> >  int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
> > +int drm_cea_sad_get_sample_rate(struct cea_sad *sad);
> > +int drm_cea_sad_get_uncompressed_word_length(struct cea_sad *sad);
> >  int drm_av_sync_delay(struct drm_connector *connector,
> >  		      const struct drm_display_mode *mode);
> >  
> > -- 
> > 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/6] video/hdmi: Add audio_infoframe packing for DP

[Markus Schneider-Pargmann]

Similar to HDMI, DP uses audio infoframes as well which are structured
very similar to the HDMI ones.

This patch adds a helper function to pack the HDMI audio infoframe for
DP, called hdmi_audio_infoframe_pack_for_dp().
hdmi_audio_infoframe_pack_only() is split into two parts. One of them
packs the payload only and can be used for HDMI and DP.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/video/hdmi.c | 87 +++++++++++++++++++++++++++++++++++---------
 include/linux/hdmi.h |  4 ++
 2 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 947be761dfa4..59c4341549e4 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -387,6 +387,28 @@ int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
 }
 EXPORT_SYMBOL(hdmi_audio_infoframe_check);
 
+static void
+hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
+				  u8 *buffer)
+{
+	u8 channels;
+
+	if (frame->channels >= 2)
+		channels = frame->channels - 1;
+	else
+		channels = 0;
+
+	buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
+	buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
+		 (frame->sample_size & 0x3);
+	buffer[2] = frame->coding_type_ext & 0x1f;
+	buffer[3] = frame->channel_allocation;
+	buffer[4] = (frame->level_shift_value & 0xf) << 3;
+
+	if (frame->downmix_inhibit)
+		buffer[4] |= BIT(7);
+}
+
 /**
  * hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary buffer
  * @frame: HDMI audio infoframe
@@ -404,7 +426,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
 ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
 				       void *buffer, size_t size)
 {
-	unsigned char channels;
 	u8 *ptr = buffer;
 	size_t length;
 	int ret;
@@ -420,28 +441,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
 
 	memset(buffer, 0, size);
 
-	if (frame->channels >= 2)
-		channels = frame->channels - 1;
-	else
-		channels = 0;
-
 	ptr[0] = frame->type;
 	ptr[1] = frame->version;
 	ptr[2] = frame->length;
 	ptr[3] = 0; /* checksum */
 
-	/* start infoframe payload */
-	ptr += HDMI_INFOFRAME_HEADER_SIZE;
-
-	ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
-	ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
-		 (frame->sample_size & 0x3);
-	ptr[2] = frame->coding_type_ext & 0x1f;
-	ptr[3] = frame->channel_allocation;
-	ptr[4] = (frame->level_shift_value & 0xf) << 3;
-
-	if (frame->downmix_inhibit)
-		ptr[4] |= BIT(7);
+	hdmi_audio_infoframe_pack_payload(frame,
+					  ptr + HDMI_INFOFRAME_HEADER_SIZE);
 
 	hdmi_infoframe_set_checksum(buffer, length);
 
@@ -479,6 +485,51 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
 }
 EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
 
+/**
+ * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
+ *                                    displayport
+ *
+ * @frame HDMI Audio infoframe
+ * @header Header buffer to be used
+ * @header_size Size of header buffer
+ * @data Data buffer to be used
+ * @data_size Size of data buffer
+ * @dp_version Display Port version to be encoded in the header
+ *
+ * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
+ * fills both header and data buffer with the required data.
+ *
+ * Return: Number of total written bytes or a negative errno on failure.
+ */
+ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
+					 void *header, size_t header_size,
+					 void *data, size_t data_size,
+					 u8 dp_version)
+{
+	int ret;
+	u8 *hdr_ptr = header;
+
+	ret = hdmi_audio_infoframe_check(frame);
+	if (ret)
+		return ret;
+
+	if (header_size < 4 || data_size < frame->length)
+		return -ENOSPC;
+
+	memset(header, 0, header_size);
+	memset(data, 0, data_size);
+
+	// Secondary-data packet header
+	hdr_ptr[1] = frame->type;
+	hdr_ptr[2] = 0x1B;  // As documented by DP spec for Secondary-data Packets
+	hdr_ptr[3] = (dp_version & 0x3f) << 2;
+
+	hdmi_audio_infoframe_pack_payload(frame, data);
+
+	return frame->length + 4;
+}
+EXPORT_SYMBOL(hdmi_audio_infoframe_pack_for_dp);
+
 /**
  * hdmi_vendor_infoframe_init() - initialize an HDMI vendor infoframe
  * @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index c8ec982ff498..f576a0b08c85 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -334,6 +334,10 @@ struct hdmi_audio_infoframe {
 int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
 ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
 				  void *buffer, size_t size);
+ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
+					 void *header, size_t header_size,
+					 void *data, size_t data_size,
+					 u8 dp_version);
 ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
 				       void *buffer, size_t size);
 int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame);
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/6] video/hdmi: Add audio_infoframe packing for DP

[Sam Ravnborg]

Hi Markus,

On Mon, Sep 06, 2021 at 09:35:27PM +0200, Markus Schneider-Pargmann wrote:
> Similar to HDMI, DP uses audio infoframes as well which are structured
> very similar to the HDMI ones.
> 
> This patch adds a helper function to pack the HDMI audio infoframe for
> DP, called hdmi_audio_infoframe_pack_for_dp().
> hdmi_audio_infoframe_pack_only() is split into two parts. One of them
> packs the payload only and can be used for HDMI and DP.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/video/hdmi.c | 87 +++++++++++++++++++++++++++++++++++---------
>  include/linux/hdmi.h |  4 ++
>  2 files changed, 73 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 947be761dfa4..59c4341549e4 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -387,6 +387,28 @@ int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
>  }
>  EXPORT_SYMBOL(hdmi_audio_infoframe_check);
>  
> +static void
> +hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
> +				  u8 *buffer)
> +{
> +	u8 channels;
> +
> +	if (frame->channels >= 2)
> +		channels = frame->channels - 1;
> +	else
> +		channels = 0;
> +
> +	buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
> +	buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
> +		 (frame->sample_size & 0x3);
> +	buffer[2] = frame->coding_type_ext & 0x1f;
> +	buffer[3] = frame->channel_allocation;
> +	buffer[4] = (frame->level_shift_value & 0xf) << 3;
> +
> +	if (frame->downmix_inhibit)
> +		buffer[4] |= BIT(7);
> +}
> +
>  /**
>   * hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary buffer
>   * @frame: HDMI audio infoframe
> @@ -404,7 +426,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
>  ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
>  				       void *buffer, size_t size)
>  {
> -	unsigned char channels;
>  	u8 *ptr = buffer;
>  	size_t length;
>  	int ret;
> @@ -420,28 +441,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
>  
>  	memset(buffer, 0, size);
>  
> -	if (frame->channels >= 2)
> -		channels = frame->channels - 1;
> -	else
> -		channels = 0;
> -
>  	ptr[0] = frame->type;
>  	ptr[1] = frame->version;
>  	ptr[2] = frame->length;
>  	ptr[3] = 0; /* checksum */
>  
> -	/* start infoframe payload */
> -	ptr += HDMI_INFOFRAME_HEADER_SIZE;
> -
> -	ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
> -	ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
> -		 (frame->sample_size & 0x3);
> -	ptr[2] = frame->coding_type_ext & 0x1f;
> -	ptr[3] = frame->channel_allocation;
> -	ptr[4] = (frame->level_shift_value & 0xf) << 3;
> -
> -	if (frame->downmix_inhibit)
> -		ptr[4] |= BIT(7);
> +	hdmi_audio_infoframe_pack_payload(frame,
> +					  ptr + HDMI_INFOFRAME_HEADER_SIZE);
>  
>  	hdmi_infoframe_set_checksum(buffer, length);
>  
> @@ -479,6 +485,51 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  }
>  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
>  
> +/**
> + * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
> + *                                    displayport
> + *
> + * @frame HDMI Audio infoframe
> + * @header Header buffer to be used
> + * @header_size Size of header buffer
> + * @data Data buffer to be used
> + * @data_size Size of data buffer
> + * @dp_version Display Port version to be encoded in the header
> + *
> + * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
> + * fills both header and data buffer with the required data.
> + *
> + * Return: Number of total written bytes or a negative errno on failure.
> + */
> +ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
> +					 void *header, size_t header_size,
> +					 void *data, size_t data_size,
> +					 u8 dp_version)
> +{
> +	int ret;
> +	u8 *hdr_ptr = header;
> +
> +	ret = hdmi_audio_infoframe_check(frame);
> +	if (ret)
> +		return ret;
> +
> +	if (header_size < 4 || data_size < frame->length)
> +		return -ENOSPC;
> +
> +	memset(header, 0, header_size);
> +	memset(data, 0, data_size);
> +
> +	// Secondary-data packet header
> +	hdr_ptr[1] = frame->type;
> +	hdr_ptr[2] = 0x1B;  // As documented by DP spec for Secondary-data Packets
Any constant we could use or define and use here?
Hard coding 0x1b is the less desireable option.

	Sam

> +	hdr_ptr[3] = (dp_version & 0x3f) << 2;
> +
> +	hdmi_audio_infoframe_pack_payload(frame, data);
> +
> +	return frame->length + 4;
> +}
> +EXPORT_SYMBOL(hdmi_audio_infoframe_pack_for_dp);
> +
>  /**
>   * hdmi_vendor_infoframe_init() - initialize an HDMI vendor infoframe
>   * @frame: HDMI vendor infoframe
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index c8ec982ff498..f576a0b08c85 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -334,6 +334,10 @@ struct hdmi_audio_infoframe {
>  int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
>  ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  				  void *buffer, size_t size);
> +ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
> +					 void *header, size_t header_size,
> +					 void *data, size_t data_size,
> +					 u8 dp_version);
>  ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
>  				       void *buffer, size_t size);
>  int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame);
> -- 
> 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 4/6] video/hdmi: Add audio_infoframe packing for DP

[Markus Schneider-Pargmann]

Hi Sam,

On Mon, Sep 06, 2021 at 10:30:26PM +0200, Sam Ravnborg wrote:
> Hi Markus,
> 
> On Mon, Sep 06, 2021 at 09:35:27PM +0200, Markus Schneider-Pargmann wrote:
> > Similar to HDMI, DP uses audio infoframes as well which are structured
> > very similar to the HDMI ones.
> > 
> > This patch adds a helper function to pack the HDMI audio infoframe for
> > DP, called hdmi_audio_infoframe_pack_for_dp().
> > hdmi_audio_infoframe_pack_only() is split into two parts. One of them
> > packs the payload only and can be used for HDMI and DP.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >  drivers/video/hdmi.c | 87 +++++++++++++++++++++++++++++++++++---------
> >  include/linux/hdmi.h |  4 ++
> >  2 files changed, 73 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index 947be761dfa4..59c4341549e4 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -387,6 +387,28 @@ int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
> >  }
> >  EXPORT_SYMBOL(hdmi_audio_infoframe_check);
> >  
> > +static void
> > +hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
> > +				  u8 *buffer)
> > +{
> > +	u8 channels;
> > +
> > +	if (frame->channels >= 2)
> > +		channels = frame->channels - 1;
> > +	else
> > +		channels = 0;
> > +
> > +	buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
> > +	buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
> > +		 (frame->sample_size & 0x3);
> > +	buffer[2] = frame->coding_type_ext & 0x1f;
> > +	buffer[3] = frame->channel_allocation;
> > +	buffer[4] = (frame->level_shift_value & 0xf) << 3;
> > +
> > +	if (frame->downmix_inhibit)
> > +		buffer[4] |= BIT(7);
> > +}
> > +
> >  /**
> >   * hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary buffer
> >   * @frame: HDMI audio infoframe
> > @@ -404,7 +426,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
> >  ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
> >  				       void *buffer, size_t size)
> >  {
> > -	unsigned char channels;
> >  	u8 *ptr = buffer;
> >  	size_t length;
> >  	int ret;
> > @@ -420,28 +441,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
> >  
> >  	memset(buffer, 0, size);
> >  
> > -	if (frame->channels >= 2)
> > -		channels = frame->channels - 1;
> > -	else
> > -		channels = 0;
> > -
> >  	ptr[0] = frame->type;
> >  	ptr[1] = frame->version;
> >  	ptr[2] = frame->length;
> >  	ptr[3] = 0; /* checksum */
> >  
> > -	/* start infoframe payload */
> > -	ptr += HDMI_INFOFRAME_HEADER_SIZE;
> > -
> > -	ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
> > -	ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
> > -		 (frame->sample_size & 0x3);
> > -	ptr[2] = frame->coding_type_ext & 0x1f;
> > -	ptr[3] = frame->channel_allocation;
> > -	ptr[4] = (frame->level_shift_value & 0xf) << 3;
> > -
> > -	if (frame->downmix_inhibit)
> > -		ptr[4] |= BIT(7);
> > +	hdmi_audio_infoframe_pack_payload(frame,
> > +					  ptr + HDMI_INFOFRAME_HEADER_SIZE);
> >  
> >  	hdmi_infoframe_set_checksum(buffer, length);
> >  
> > @@ -479,6 +485,51 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
> >  }
> >  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
> >  
> > +/**
> > + * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
> > + *                                    displayport
> > + *
> > + * @frame HDMI Audio infoframe
> > + * @header Header buffer to be used
> > + * @header_size Size of header buffer
> > + * @data Data buffer to be used
> > + * @data_size Size of data buffer
> > + * @dp_version Display Port version to be encoded in the header
> > + *
> > + * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
> > + * fills both header and data buffer with the required data.
> > + *
> > + * Return: Number of total written bytes or a negative errno on failure.
> > + */
> > +ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
> > +					 void *header, size_t header_size,
> > +					 void *data, size_t data_size,
> > +					 u8 dp_version)
> > +{
> > +	int ret;
> > +	u8 *hdr_ptr = header;
> > +
> > +	ret = hdmi_audio_infoframe_check(frame);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (header_size < 4 || data_size < frame->length)
> > +		return -ENOSPC;
> > +
> > +	memset(header, 0, header_size);
> > +	memset(data, 0, data_size);
> > +
> > +	// Secondary-data packet header
> > +	hdr_ptr[1] = frame->type;
> > +	hdr_ptr[2] = 0x1B;  // As documented by DP spec for Secondary-data Packets
> Any constant we could use or define and use here?
> Hard coding 0x1b is the less desireable option.

I couldn't find a constant for this, but I will add one.

Also thanks to looking for these defines, I found a struct dp_sdp that
could be used instead of the header and data pointers which I would
prefer, so I will fix that for the next version.

Thanks for your feedback.

Best,
Markus

> 
> 	Sam
> 
> > +	hdr_ptr[3] = (dp_version & 0x3f) << 2;
> > +
> > +	hdmi_audio_infoframe_pack_payload(frame, data);
> > +
> > +	return frame->length + 4;
> > +}
> > +EXPORT_SYMBOL(hdmi_audio_infoframe_pack_for_dp);
> > +
> >  /**
> >   * hdmi_vendor_infoframe_init() - initialize an HDMI vendor infoframe
> >   * @frame: HDMI vendor infoframe
> > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> > index c8ec982ff498..f576a0b08c85 100644
> > --- a/include/linux/hdmi.h
> > +++ b/include/linux/hdmi.h
> > @@ -334,6 +334,10 @@ struct hdmi_audio_infoframe {
> >  int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
> >  ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
> >  				  void *buffer, size_t size);
> > +ssize_t hdmi_audio_infoframe_pack_for_dp(struct hdmi_audio_infoframe *frame,
> > +					 void *header, size_t header_size,
> > +					 void *data, size_t data_size,
> > +					 u8 dp_version);
> >  ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame,
> >  				       void *buffer, size_t size);
> >  int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame);
> > -- 
> > 2.33.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 5/6] drm/mediatek: dpi: Add dpintf support

[Markus Schneider-Pargmann]

dpintf is the displayport interface hardware unit. This unit is similar
to dpi and can reuse most of the code.

This patch adds support for mt8195-dpintf to this dpi driver. Main
differences are:
 - Some features/functional components are not available for dpintf
   which are now excluded from code execution once is_dpintf is set
 - dpintf can and needs to choose between different clockdividers based
   on the clockspeed. This is done by choosing a different clock parent.
 - There are two additional clocks that need to be managed. These are
   only set for dpintf and will be set to NULL if not supplied. The
   clk_* calls handle these as normal clocks then.
 - Some register contents differ slightly between the two components. To
   work around this I added register bits/masks with a DPINTF_ prefix
   and use them where different.

Based on a separate driver for dpintf created by
Jason-JH.Lin <jason-jh.lin@mediatek.com>.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes RFC -> v1:
    - Remove setting parents and fully rely on the clock tree instead which already
      models a mux at the important place.
    - Integrated mtk_dpi dpintf changes into the mediatek drm driver.

 drivers/gpu/drm/mediatek/mtk_dpi.c          | 248 ++++++++++++++++----
 drivers/gpu/drm/mediatek/mtk_dpi_regs.h     |  12 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   4 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   3 +
 5 files changed, 217 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index e94738fe4db8..986c7f72483f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -71,6 +71,8 @@ struct mtk_dpi {
 	void __iomem *regs;
 	struct device *dev;
 	struct clk *engine_clk;
+	struct clk *hf_fmm_clk;
+	struct clk *hf_fdp_clk;
 	struct clk *pixel_clk;
 	struct clk *tvd_clk;
 	int irq;
@@ -125,6 +127,7 @@ struct mtk_dpi_conf {
 	bool edge_sel_en;
 	const u32 *output_fmts;
 	u32 num_output_fmts;
+	bool is_dpintf;
 };
 
 static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
@@ -153,30 +156,52 @@ static void mtk_dpi_disable(struct mtk_dpi *dpi)
 static void mtk_dpi_config_hsync(struct mtk_dpi *dpi,
 				 struct mtk_dpi_sync_param *sync)
 {
-	mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
-		     sync->sync_width << HPW, HPW_MASK);
-	mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
-		     sync->back_porch << HBP, HBP_MASK);
-	mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
-		     HFP_MASK);
+	if (dpi->conf->is_dpintf) {
+		mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
+			     sync->sync_width << HPW, DPINTF_HPW_MASK);
+		mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
+			     sync->back_porch << HBP, DPINTF_HBP_MASK);
+		mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
+			     DPINTF_HFP_MASK);
+	} else {
+		mtk_dpi_mask(dpi, DPI_TGEN_HWIDTH,
+			     sync->sync_width << HPW, HPW_MASK);
+		mtk_dpi_mask(dpi, DPI_TGEN_HPORCH,
+			     sync->back_porch << HBP, HBP_MASK);
+		mtk_dpi_mask(dpi, DPI_TGEN_HPORCH, sync->front_porch << HFP,
+			     HFP_MASK);
+	}
 }
 
 static void mtk_dpi_config_vsync(struct mtk_dpi *dpi,
 				 struct mtk_dpi_sync_param *sync,
 				 u32 width_addr, u32 porch_addr)
 {
-	mtk_dpi_mask(dpi, width_addr,
-		     sync->sync_width << VSYNC_WIDTH_SHIFT,
-		     VSYNC_WIDTH_MASK);
 	mtk_dpi_mask(dpi, width_addr,
 		     sync->shift_half_line << VSYNC_HALF_LINE_SHIFT,
 		     VSYNC_HALF_LINE_MASK);
-	mtk_dpi_mask(dpi, porch_addr,
-		     sync->back_porch << VSYNC_BACK_PORCH_SHIFT,
-		     VSYNC_BACK_PORCH_MASK);
-	mtk_dpi_mask(dpi, porch_addr,
-		     sync->front_porch << VSYNC_FRONT_PORCH_SHIFT,
-		     VSYNC_FRONT_PORCH_MASK);
+
+	if (dpi->conf->is_dpintf) {
+		mtk_dpi_mask(dpi, width_addr,
+			     sync->sync_width << VSYNC_WIDTH_SHIFT,
+			     DPINTF_VSYNC_WIDTH_MASK);
+		mtk_dpi_mask(dpi, porch_addr,
+			     sync->back_porch << VSYNC_BACK_PORCH_SHIFT,
+			     DPINTF_VSYNC_BACK_PORCH_MASK);
+		mtk_dpi_mask(dpi, porch_addr,
+			     sync->front_porch << VSYNC_FRONT_PORCH_SHIFT,
+			     DPINTF_VSYNC_FRONT_PORCH_MASK);
+	} else {
+		mtk_dpi_mask(dpi, width_addr,
+			     sync->sync_width << VSYNC_WIDTH_SHIFT,
+			     VSYNC_WIDTH_MASK);
+		mtk_dpi_mask(dpi, porch_addr,
+			     sync->back_porch << VSYNC_BACK_PORCH_SHIFT,
+			     VSYNC_BACK_PORCH_MASK);
+		mtk_dpi_mask(dpi, porch_addr,
+			     sync->front_porch << VSYNC_FRONT_PORCH_SHIFT,
+			     VSYNC_FRONT_PORCH_MASK);
+	}
 }
 
 static void mtk_dpi_config_vsync_lodd(struct mtk_dpi *dpi,
@@ -210,13 +235,20 @@ static void mtk_dpi_config_pol(struct mtk_dpi *dpi,
 			       struct mtk_dpi_polarities *dpi_pol)
 {
 	unsigned int pol;
+	unsigned int mask;
 
-	pol = (dpi_pol->ck_pol == MTK_DPI_POLARITY_RISING ? 0 : CK_POL) |
-	      (dpi_pol->de_pol == MTK_DPI_POLARITY_RISING ? 0 : DE_POL) |
-	      (dpi_pol->hsync_pol == MTK_DPI_POLARITY_RISING ? 0 : HSYNC_POL) |
+	mask = HSYNC_POL | VSYNC_POL;
+	pol = (dpi_pol->hsync_pol == MTK_DPI_POLARITY_RISING ? 0 : HSYNC_POL) |
 	      (dpi_pol->vsync_pol == MTK_DPI_POLARITY_RISING ? 0 : VSYNC_POL);
-	mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, pol,
-		     CK_POL | DE_POL | HSYNC_POL | VSYNC_POL);
+	if (!dpi->conf->is_dpintf) {
+		mask |= CK_POL | DE_POL;
+		pol |= (dpi_pol->ck_pol == MTK_DPI_POLARITY_RISING ?
+			0 : CK_POL) |
+		       (dpi_pol->de_pol == MTK_DPI_POLARITY_RISING ?
+			0 : DE_POL);
+	}
+
+	mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, pol, mask);
 }
 
 static void mtk_dpi_config_3d(struct mtk_dpi *dpi, bool en_3d)
@@ -231,8 +263,13 @@ static void mtk_dpi_config_interface(struct mtk_dpi *dpi, bool inter)
 
 static void mtk_dpi_config_fb_size(struct mtk_dpi *dpi, u32 width, u32 height)
 {
-	mtk_dpi_mask(dpi, DPI_SIZE, width << HSIZE, HSIZE_MASK);
-	mtk_dpi_mask(dpi, DPI_SIZE, height << VSIZE, VSIZE_MASK);
+	if (dpi->conf->is_dpintf) {
+		mtk_dpi_mask(dpi, DPI_SIZE, width << HSIZE, DPINTF_HSIZE_MASK);
+		mtk_dpi_mask(dpi, DPI_SIZE, height << VSIZE, DPINTF_VSIZE_MASK);
+	} else {
+		mtk_dpi_mask(dpi, DPI_SIZE, width << HSIZE, HSIZE_MASK);
+		mtk_dpi_mask(dpi, DPI_SIZE, height << VSIZE, VSIZE_MASK);
+	}
 }
 
 static void mtk_dpi_config_channel_limit(struct mtk_dpi *dpi,
@@ -332,12 +369,21 @@ static void mtk_dpi_config_channel_swap(struct mtk_dpi *dpi,
 		break;
 	}
 
-	mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP, CH_SWAP_MASK);
+	if (dpi->conf->is_dpintf)
+		mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << DPINTF_CH_SWAP,
+			     DPINTF_CH_SWAP_MASK);
+	else
+		mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP,
+			     CH_SWAP_MASK);
 }
 
 static void mtk_dpi_config_yuv422_enable(struct mtk_dpi *dpi, bool enable)
 {
-	mtk_dpi_mask(dpi, DPI_CON, enable ? YUV422_EN : 0, YUV422_EN);
+	if (dpi->conf->is_dpintf)
+		mtk_dpi_mask(dpi, DPI_CON, enable ? DPINTF_YUV422_EN : 0,
+			     DPINTF_YUV422_EN);
+	else
+		mtk_dpi_mask(dpi, DPI_CON, enable ? YUV422_EN : 0, YUV422_EN);
 }
 
 static void mtk_dpi_config_csc_enable(struct mtk_dpi *dpi, bool enable)
@@ -367,19 +413,25 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
 	if ((format == MTK_DPI_COLOR_FORMAT_YCBCR_444) ||
 	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
 		mtk_dpi_config_yuv422_enable(dpi, false);
-		mtk_dpi_config_csc_enable(dpi, true);
-		mtk_dpi_config_swap_input(dpi, false);
+		if (!dpi->conf->is_dpintf) {
+			mtk_dpi_config_csc_enable(dpi, true);
+			mtk_dpi_config_swap_input(dpi, false);
+		}
 		mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR);
 	} else if ((format == MTK_DPI_COLOR_FORMAT_YCBCR_422) ||
 		   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
 		mtk_dpi_config_yuv422_enable(dpi, true);
-		mtk_dpi_config_csc_enable(dpi, true);
-		mtk_dpi_config_swap_input(dpi, true);
+		if (!dpi->conf->is_dpintf) {
+			mtk_dpi_config_csc_enable(dpi, true);
+			mtk_dpi_config_swap_input(dpi, true);
+		}
 		mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB);
 	} else {
 		mtk_dpi_config_yuv422_enable(dpi, false);
-		mtk_dpi_config_csc_enable(dpi, false);
-		mtk_dpi_config_swap_input(dpi, false);
+		if (!dpi->conf->is_dpintf) {
+			mtk_dpi_config_csc_enable(dpi, false);
+			mtk_dpi_config_swap_input(dpi, false);
+		}
 		mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB);
 	}
 }
@@ -410,6 +462,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi)
 		pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio);
 
 	mtk_dpi_disable(dpi);
+	clk_disable_unprepare(dpi->hf_fdp_clk);
+	clk_disable_unprepare(dpi->hf_fmm_clk);
 	clk_disable_unprepare(dpi->pixel_clk);
 	clk_disable_unprepare(dpi->engine_clk);
 }
@@ -433,12 +487,28 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
 		goto err_pixel;
 	}
 
+	ret = clk_prepare_enable(dpi->hf_fmm_clk);
+	if (ret) {
+		dev_err(dpi->dev, "Failed to enable hf_fmm clock: %d\n", ret);
+		goto err_hf_fmm;
+	}
+
+	ret = clk_prepare_enable(dpi->hf_fdp_clk);
+	if (ret) {
+		dev_err(dpi->dev, "Failed to enable hf_fdp clock: %d\n", ret);
+		goto err_hf_fdp;
+	}
+
 	if (dpi->pinctrl && dpi->pins_dpi)
 		pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi);
 
 	mtk_dpi_enable(dpi);
 	return 0;
 
+err_hf_fdp:
+	clk_disable_unprepare(dpi->hf_fmm_clk);
+err_hf_fmm:
+	clk_disable_unprepare(dpi->pixel_clk);
 err_pixel:
 	clk_disable_unprepare(dpi->engine_clk);
 err_refcount:
@@ -472,22 +542,30 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
 	pll_rate = clk_get_rate(dpi->tvd_clk);
 
 	vm.pixelclock = pll_rate / factor;
-	if ((dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_LE) ||
-	    (dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_BE))
+	if (dpi->conf->is_dpintf)
+		clk_set_rate(dpi->pixel_clk, vm.pixelclock / 4);
+	else if ((dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_LE) ||
+		 (dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_BE))
 		clk_set_rate(dpi->pixel_clk, vm.pixelclock * 2);
 	else
 		clk_set_rate(dpi->pixel_clk, vm.pixelclock);
 
-
 	vm.pixelclock = clk_get_rate(dpi->pixel_clk);
 
 	dev_dbg(dpi->dev, "Got  PLL %lu Hz, pixel clock %lu Hz\n",
 		pll_rate, vm.pixelclock);
 
-	limit.c_bottom = 0x0010;
-	limit.c_top = 0x0FE0;
-	limit.y_bottom = 0x0010;
-	limit.y_top = 0x0FE0;
+	if (dpi->conf->is_dpintf) {
+		limit.c_bottom = 0x0000;
+		limit.c_top = 0xFFF;
+		limit.y_bottom = 0x0000;
+		limit.y_top = 0xFFF;
+	} else {
+		limit.c_bottom = 0x0010;
+		limit.c_top = 0x0FE0;
+		limit.y_bottom = 0x0010;
+		limit.y_top = 0x0FE0;
+	}
 
 	dpi_pol.ck_pol = MTK_DPI_POLARITY_FALLING;
 	dpi_pol.de_pol = MTK_DPI_POLARITY_RISING;
@@ -495,9 +573,15 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
 			    MTK_DPI_POLARITY_FALLING : MTK_DPI_POLARITY_RISING;
 	dpi_pol.vsync_pol = vm.flags & DISPLAY_FLAGS_VSYNC_HIGH ?
 			    MTK_DPI_POLARITY_FALLING : MTK_DPI_POLARITY_RISING;
-	hsync.sync_width = vm.hsync_len;
-	hsync.back_porch = vm.hback_porch;
-	hsync.front_porch = vm.hfront_porch;
+	if (dpi->conf->is_dpintf) {
+		hsync.sync_width = vm.hsync_len / 4;
+		hsync.back_porch = vm.hback_porch / 4;
+		hsync.front_porch = vm.hfront_porch / 4;
+	} else {
+		hsync.sync_width = vm.hsync_len;
+		hsync.back_porch = vm.hback_porch;
+		hsync.front_porch = vm.hfront_porch;
+	}
 	hsync.shift_half_line = false;
 	vsync_lodd.sync_width = vm.vsync_len;
 	vsync_lodd.back_porch = vm.vback_porch;
@@ -539,11 +623,16 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
 	mtk_dpi_config_channel_limit(dpi, &limit);
 	mtk_dpi_config_bit_num(dpi, dpi->bit_num);
 	mtk_dpi_config_channel_swap(dpi, dpi->channel_swap);
-	mtk_dpi_config_yc_map(dpi, dpi->yc_map);
 	mtk_dpi_config_color_format(dpi, dpi->color_format);
-	mtk_dpi_config_2n_h_fre(dpi);
-	mtk_dpi_dual_edge(dpi);
-	mtk_dpi_config_disable_edge(dpi);
+	if (dpi->conf->is_dpintf) {
+		mtk_dpi_mask(dpi, DPI_CON, DPINTF_INPUT_2P_EN,
+			     DPINTF_INPUT_2P_EN);
+	} else {
+		mtk_dpi_config_yc_map(dpi, dpi->yc_map);
+		mtk_dpi_config_2n_h_fre(dpi);
+		mtk_dpi_dual_edge(dpi);
+		mtk_dpi_config_disable_edge(dpi);
+	}
 	mtk_dpi_sw_reset(dpi, false);
 
 	return 0;
@@ -687,6 +776,17 @@ static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
+static const struct drm_bridge_funcs mtk_dpintf_bridge_funcs = {
+	.attach = mtk_dpi_bridge_attach,
+	.mode_set = mtk_dpi_bridge_mode_set,
+	.disable = mtk_dpi_bridge_disable,
+	.enable = mtk_dpi_bridge_enable,
+	.atomic_check = mtk_dpi_bridge_atomic_check,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+};
+
 void mtk_dpi_start(struct device *dev)
 {
 	struct mtk_dpi *dpi = dev_get_drvdata(dev);
@@ -783,6 +883,16 @@ static unsigned int mt8183_calculate_factor(int clock)
 		return 2;
 }
 
+static unsigned int mt8195_dpintf_calculate_factor(int clock)
+{
+	if (clock < 70000)
+		return 4;
+	else if (clock < 200000)
+		return 2;
+	else
+		return 1;
+}
+
 static const u32 mt8173_output_fmts[] = {
 	MEDIA_BUS_FMT_RGB888_1X24,
 };
@@ -798,6 +908,7 @@ static const struct mtk_dpi_conf mt8173_conf = {
 	.max_clock_khz = 300000,
 	.output_fmts = mt8173_output_fmts,
 	.num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
+	.is_dpintf = false,
 };
 
 static const struct mtk_dpi_conf mt2701_conf = {
@@ -807,6 +918,7 @@ static const struct mtk_dpi_conf mt2701_conf = {
 	.max_clock_khz = 150000,
 	.output_fmts = mt8173_output_fmts,
 	.num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
+	.is_dpintf = false,
 };
 
 static const struct mtk_dpi_conf mt8183_conf = {
@@ -815,6 +927,7 @@ static const struct mtk_dpi_conf mt8183_conf = {
 	.max_clock_khz = 100000,
 	.output_fmts = mt8183_output_fmts,
 	.num_output_fmts = ARRAY_SIZE(mt8183_output_fmts),
+	.is_dpintf = false,
 };
 
 static const struct mtk_dpi_conf mt8192_conf = {
@@ -823,6 +936,12 @@ static const struct mtk_dpi_conf mt8192_conf = {
 	.max_clock_khz = 150000,
 	.output_fmts = mt8173_output_fmts,
 	.num_output_fmts = ARRAY_SIZE(mt8173_output_fmts),
+	.is_dpintf = false,
+};
+
+static const struct mtk_dpi_conf mt8195_dpintf_conf = {
+	.cal_factor = mt8195_dpintf_calculate_factor,
+	.is_dpintf = true,
 };
 
 static int mtk_dpi_probe(struct platform_device *pdev)
@@ -868,13 +987,16 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dpi->engine_clk = devm_clk_get(dev, "engine");
-	if (IS_ERR(dpi->engine_clk)) {
-		ret = PTR_ERR(dpi->engine_clk);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get engine clock: %d\n", ret);
+	if (!dpi->conf->is_dpintf) {
+		dpi->engine_clk = devm_clk_get(dev, "engine");
+		if (IS_ERR(dpi->engine_clk)) {
+			ret = PTR_ERR(dpi->engine_clk);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get engine clock: %d\n",
+					ret);
 
-		return ret;
+			return ret;
+		}
 	}
 
 	dpi->pixel_clk = devm_clk_get(dev, "pixel");
@@ -895,6 +1017,24 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dpi->hf_fmm_clk = devm_clk_get_optional(dev, "hf_fmm");
+	if (IS_ERR(dpi->hf_fmm_clk)) {
+		ret = PTR_ERR(dpi->hf_fmm_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get hf_fmm clock: %d\n", ret);
+
+		return ret;
+	}
+
+	dpi->hf_fdp_clk = devm_clk_get_optional(dev, "hf_fdp");
+	if (IS_ERR(dpi->hf_fdp_clk)) {
+		ret = PTR_ERR(dpi->hf_fdp_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get hf_fdp clock: %d\n", ret);
+
+		return ret;
+	}
+
 	dpi->irq = platform_get_irq(pdev, 0);
 	if (dpi->irq <= 0)
 		return -EINVAL;
@@ -908,7 +1048,10 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, dpi);
 
-	dpi->bridge.funcs = &mtk_dpi_bridge_funcs;
+	if (dpi->conf->is_dpintf)
+		dpi->bridge.funcs = &mtk_dpintf_bridge_funcs;
+	else
+		dpi->bridge.funcs = &mtk_dpi_bridge_funcs;
 	dpi->bridge.of_node = dev->of_node;
 	dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
 
@@ -947,6 +1090,9 @@ static const struct of_device_id mtk_dpi_of_ids[] = {
 	{ .compatible = "mediatek,mt8192-dpi",
 	  .data = &mt8192_conf,
 	},
+	{ .compatible = "mediatek,mt8195-dpintf",
+	  .data = &mt8195_dpintf_conf,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, mtk_dpi_of_ids);
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
index 3a02fabe1662..72efe6ee2584 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
+++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
@@ -40,10 +40,14 @@
 #define FAKE_DE_LEVEN			BIT(21)
 #define FAKE_DE_RODD			BIT(22)
 #define FAKE_DE_REVEN			BIT(23)
+#define DPINTF_YUV422_EN		BIT(24)
+#define DPINTF_INPUT_2P_EN		BIT(29)
 
 #define DPI_OUTPUT_SETTING	0x14
 #define CH_SWAP				0
+#define DPINTF_CH_SWAP			BIT(1)
 #define CH_SWAP_MASK			(0x7 << 0)
+#define DPINTF_CH_SWAP_MASK		(0x7 << 1)
 #define SWAP_RGB			0x00
 #define SWAP_GBR			0x01
 #define SWAP_BRG			0x02
@@ -80,8 +84,10 @@
 #define DPI_SIZE		0x18
 #define HSIZE				0
 #define HSIZE_MASK			(0x1FFF << 0)
+#define DPINTF_HSIZE_MASK		(0xFFFF << 0)
 #define VSIZE				16
 #define VSIZE_MASK			(0x1FFF << 16)
+#define DPINTF_VSIZE_MASK		(0xFFFF << 16)
 
 #define DPI_DDR_SETTING		0x1C
 #define DDR_EN				BIT(0)
@@ -93,24 +99,30 @@
 #define DPI_TGEN_HWIDTH		0x20
 #define HPW				0
 #define HPW_MASK			(0xFFF << 0)
+#define DPINTF_HPW_MASK			(0xFFFF << 0)
 
 #define DPI_TGEN_HPORCH		0x24
 #define HBP				0
 #define HBP_MASK			(0xFFF << 0)
+#define DPINTF_HBP_MASK			(0xFFFF << 0)
 #define HFP				16
 #define HFP_MASK			(0xFFF << 16)
+#define DPINTF_HFP_MASK			(0xFFFF << 16)
 
 #define DPI_TGEN_VWIDTH		0x28
 #define DPI_TGEN_VPORCH		0x2C
 
 #define VSYNC_WIDTH_SHIFT		0
 #define VSYNC_WIDTH_MASK		(0xFFF << 0)
+#define DPINTF_VSYNC_WIDTH_MASK		(0xFFFF << 0)
 #define VSYNC_HALF_LINE_SHIFT		16
 #define VSYNC_HALF_LINE_MASK		BIT(16)
 #define VSYNC_BACK_PORCH_SHIFT		0
 #define VSYNC_BACK_PORCH_MASK		(0xFFF << 0)
+#define DPINTF_VSYNC_BACK_PORCH_MASK	(0xFFFF << 0)
 #define VSYNC_FRONT_PORCH_SHIFT		16
 #define VSYNC_FRONT_PORCH_MASK		(0xFFF << 16)
+#define DPINTF_VSYNC_FRONT_PORCH_MASK	(0xFFFF << 16)
 
 #define DPI_BG_HCNTL		0x30
 #define BG_RIGHT			(0x1FFF << 0)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 900f9c9f61f3..871800b4b6ca 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -376,6 +376,7 @@ static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = {
 	[MTK_DISP_UFOE] = "ufoe",
 	[MTK_DISP_WDMA] = "wdma",
 	[MTK_DPI] = "dpi",
+	[MTK_DP_INTF] = "dp-intf",
 	[MTK_DSI] = "dsi",
 };
 
@@ -395,6 +396,8 @@ static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_DITHER]	= { MTK_DISP_DITHER,	0, &ddp_dither },
 	[DDP_COMPONENT_DPI0]	= { MTK_DPI,		0, &ddp_dpi },
 	[DDP_COMPONENT_DPI1]	= { MTK_DPI,		1, &ddp_dpi },
+	[DDP_COMPONENT_DP_INTF0]= { MTK_DP_INTF,	0, &ddp_dpi },
+	[DDP_COMPONENT_DP_INTF1]= { MTK_DP_INTF,	1, &ddp_dpi },
 	[DDP_COMPONENT_DSC0]	= { MTK_DISP_DSC,	0, &ddp_dsc },
 	[DDP_COMPONENT_DSC1]	= { MTK_DISP_DSC,	1, &ddp_dsc },
 	[DDP_COMPONENT_DSI0]	= { MTK_DSI,		0, &ddp_dsi },
@@ -547,6 +550,7 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
 	    type == MTK_DISP_PWM ||
 	    type == MTK_DISP_RDMA ||
 	    type == MTK_DPI ||
+	    type == MTK_DP_INTF ||
 	    type == MTK_DSI)
 		return 0;
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index a8e5e770ce2f..9f47e883cc91 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -36,6 +36,7 @@ enum mtk_ddp_comp_type {
 	MTK_DISP_UFOE,
 	MTK_DISP_WDMA,
 	MTK_DPI,
+	MTK_DP_INTF,
 	MTK_DSI,
 	MTK_DDP_COMP_TYPE_MAX,
 };
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index e831175a66f3..a8c6b9708e34 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -728,6 +728,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
 	  .data = (void *)MTK_DPI },
 	{ .compatible = "mediatek,mt8183-dpi",
 	  .data = (void *)MTK_DPI },
+	{ .compatible = "mediatek,mt8195-dpintf",
+	  .data = (void *)MTK_DP_INTF },
 	{ .compatible = "mediatek,mt2701-dsi",
 	  .data = (void *)MTK_DSI },
 	{ .compatible = "mediatek,mt8173-dsi",
@@ -827,6 +829,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
 		    comp_type == MTK_DISP_OVL_ADAPTOR ||
 		    comp_type == MTK_DISP_RDMA ||
 		    comp_type == MTK_DPI ||
+		    comp_type == MTK_DP_INTF ||
 		    comp_type == MTK_DSI) {
 			dev_info(dev, "Adding component match for %pOF\n",
 				 node);
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Sam Ravnborg]

Hi Markus,

On Mon, Sep 06, 2021 at 09:35:29PM +0200, Markus Schneider-Pargmann wrote:
> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> 
> It supports both functional units on the mt8195, the embedded
> DisplayPort as well as the external DisplayPort units. It offers
> hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> to 4 lanes.
> 
> This driver is based on an initial version by
> Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>     Changes RFC -> v1:
>     - Removed unused register definitions.
>     - Replaced workqueue with threaded irq.
>     - Removed connector code.
>     - Move to atomic_* drm functions.
>     - General cleanups of the code.
>     - Remove unused select GENERIC_PHY.
> 
>  drivers/gpu/drm/mediatek/Kconfig      |    6 +
>  drivers/gpu/drm/mediatek/Makefile     |    2 +
>  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
>  4 files changed, 3469 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> 


> +
> +static const struct drm_bridge_funcs mtk_dp_bridge_funcs = {
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.attach = mtk_dp_bridge_attach,
> +	.detach = mtk_dp_bridge_detach,
> +	.pre_enable = mtk_dp_bridge_pre_enable,
Use the atomic variant here as pre_enable is deprecated.

> +	.atomic_enable = mtk_dp_bridge_atomic_enable,
> +	.atomic_disable = mtk_dp_bridge_atomic_disable,
> +	.post_disable = mtk_dp_bridge_post_disable,
Use the atomic variant here as .post_disable is deprecated.

> +	.get_edid = mtk_dp_get_edid,
> +	.detect = mtk_dp_bdg_detect,
> +};

Everything else I skimmed looked fine. But it was a quick skim so..

	Sam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Markus Schneider-Pargmann]

Hi Sam,

On Mon, Sep 06, 2021 at 10:39:21PM +0200, Sam Ravnborg wrote:
> Hi Markus,
> 
> On Mon, Sep 06, 2021 at 09:35:29PM +0200, Markus Schneider-Pargmann wrote:
> > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> > 
> > It supports both functional units on the mt8195, the embedded
> > DisplayPort as well as the external DisplayPort units. It offers
> > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > to 4 lanes.
> > 
> > This driver is based on an initial version by
> > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> > 
> > Notes:
> >     Changes RFC -> v1:
> >     - Removed unused register definitions.
> >     - Replaced workqueue with threaded irq.
> >     - Removed connector code.
> >     - Move to atomic_* drm functions.
> >     - General cleanups of the code.
> >     - Remove unused select GENERIC_PHY.
> > 
> >  drivers/gpu/drm/mediatek/Kconfig      |    6 +
> >  drivers/gpu/drm/mediatek/Makefile     |    2 +
> >  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
> >  4 files changed, 3469 insertions(+)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> > 
> 
> 
> > +
> > +static const struct drm_bridge_funcs mtk_dp_bridge_funcs = {
> > +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> > +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> > +	.atomic_reset = drm_atomic_helper_bridge_reset,
> > +	.attach = mtk_dp_bridge_attach,
> > +	.detach = mtk_dp_bridge_detach,
> > +	.pre_enable = mtk_dp_bridge_pre_enable,
> Use the atomic variant here as pre_enable is deprecated.
> 
> > +	.atomic_enable = mtk_dp_bridge_atomic_enable,
> > +	.atomic_disable = mtk_dp_bridge_atomic_disable,
> > +	.post_disable = mtk_dp_bridge_post_disable,
> Use the atomic variant here as .post_disable is deprecated.

Thanks, I got rid of both pre_enable and post_disable as well now.

> 
> > +	.get_edid = mtk_dp_get_edid,
> > +	.detect = mtk_dp_bdg_detect,
> > +};
> 
> Everything else I skimmed looked fine. But it was a quick skim so..

Thanks for taking the time to have a look.

Best,
Markus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Philipp Zabel]

Hi Markus,

On Mon, 2021-09-06 at 21:35 +0200, Markus Schneider-Pargmann wrote:
> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> 
> It supports both functional units on the mt8195, the embedded
> DisplayPort as well as the external DisplayPort units. It offers
> hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> to 4 lanes.
> 
> This driver is based on an initial version by
> Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
[...]
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> new file mode 100644
> index 000000000000..1bd07c8d2f69
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -0,0 +1,2881 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Copyright (c) 2021 BayLibre
> + */
> +
[...]
> +#include <linux/component.h>
[...]
> +#include <linux/extcon.h>
> +#include <linux/extcon-provider.h>
[...]
> +#include <linux/kthread.h>
> +#include <linux/mfd/syscon.h>
[...]
> +#include <linux/of_gpio.h>
> +#include <linux/of_graph.h>
[...]
> +#include <linux/phy/phy.h>

The list of included headers could be pruned a bit, from a quick look it
seems like neither of these are actually used.

[...]
> +static void mtk_dp_ssc_enable(struct mtk_dp *mtk_dp, bool enable)
> +{
> +	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, DP_PWR_STATE_BANDGAP,
> +			   DP_PWR_STATE_MASK);
> +	mtk_dp_update_bits(mtk_dp, DP_PHY_DIG_PLL_CTL_1,
> +			   enable ? TPLL_SSC_EN : 0, TPLL_SSC_EN);
> +	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> +			   DP_PWR_STATE_BANDGAP_TPLL_LANE, DP_PWR_STATE_MASK);
> +
> +	udelay(50);

Can usleep_range() be used here? Same for the other delays.

[...]
> +static void mtk_dp_hpd_sink_event(struct mtk_dp *mtk_dp)
> +{
[...]
> +
> +	if (DP_GET_SINK_COUNT(sink_count) &&
> +	    (link_status[2] & DP_DOWNSTREAM_PORT_STATUS_CHANGED)) {
> +		mtk_dp->train_info.check_cap_count = 0;
> +		kfree(mtk_dp->edid);
> +		mtk_dp->edid = NULL;

Should this be protect by a lock? This looks like it could race with the
access in mtk_dp_edid_parse_audio_capabilities() or mtk_dp_get_edid()

[...]
> +static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)
> +{
> +	int ret = 0;
> +
> +	ret = mtk_dp_train_hpd_handle(mtk_dp);
> +
> +	if (!mtk_dp->train_info.cable_plugged_in)
> +		return -ENODEV;
> +
> +	if (mtk_dp->train_state == MTK_DP_TRAIN_STATE_NORMAL)
> +		return ret;
> +
> +	switch (mtk_dp->train_state) {
[...]
> +	case MTK_DP_TRAIN_STATE_TRAINING:
> +		ret = mtk_dp_train_start(mtk_dp);
> +		if (!ret) {
> +			mtk_dp_video_mute(mtk_dp, true);
> +			mtk_dp_audio_mute(mtk_dp, true);
> +			mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKTIMING;
> +			mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);
> +		}  else if (ret != -EAGAIN)
> +			mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;

A small whitespace issue and missing braces.

Consider running this through checkpatch.pl --strict once for style
issues.

[...]
> +static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev)
> +{
> +	struct mtk_dp *mtk_dp = dev;
> +	uint32_t irq_status;
> +
> +	irq_status = mtk_dp_read(mtk_dp, MTK_DP_TOP_IRQ_STATUS);
> +
> +	if (!irq_status)
> +		return IRQ_HANDLED;

This check seems superfluous given that only the
RGS_IRQ_STATUS_TRANSMITTER bit is checked right below:

> +	if (irq_status & RGS_IRQ_STATUS_TRANSMITTER)
> +		return mtk_dp_hpd_isr_handler(mtk_dp);
> +
> +	return IRQ_HANDLED;
> +}
[...]
> +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
> +				    struct drm_connector *connector)
> +{
> +	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> +	bool pre_enabled = mtk_dp->pre_enabled;
> +
> +	if (mtk_dp->edid)
> +		kfree(mtk_dp->edid);

Unnecessary check, kfree() accepts NULL.

regards
Philipp

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Markus Schneider-Pargmann]

Hi Philipp,

On Tue, Sep 07, 2021 at 10:47:41AM +0200, Philipp Zabel wrote:
> Hi Markus,
> 
> On Mon, 2021-09-06 at 21:35 +0200, Markus Schneider-Pargmann wrote:
> > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> > 
> > It supports both functional units on the mt8195, the embedded
> > DisplayPort as well as the external DisplayPort units. It offers
> > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > to 4 lanes.
> > 
> > This driver is based on an initial version by
> > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> [...]
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> > new file mode 100644
> > index 000000000000..1bd07c8d2f69
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -0,0 +1,2881 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2019 MediaTek Inc.
> > + * Copyright (c) 2021 BayLibre
> > + */
> > +
> [...]
> > +#include <linux/component.h>
> [...]
> > +#include <linux/extcon.h>
> > +#include <linux/extcon-provider.h>
> [...]
> > +#include <linux/kthread.h>
> > +#include <linux/mfd/syscon.h>
> [...]
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_graph.h>
> [...]
> > +#include <linux/phy/phy.h>
> 
> The list of included headers could be pruned a bit, from a quick look it
> seems like neither of these are actually used.

Thank you. I fixed the includes for the next version.

> 
> [...]
> > +static void mtk_dp_ssc_enable(struct mtk_dp *mtk_dp, bool enable)
> > +{
> > +	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, DP_PWR_STATE_BANDGAP,
> > +			   DP_PWR_STATE_MASK);
> > +	mtk_dp_update_bits(mtk_dp, DP_PHY_DIG_PLL_CTL_1,
> > +			   enable ? TPLL_SSC_EN : 0, TPLL_SSC_EN);
> > +	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > +			   DP_PWR_STATE_BANDGAP_TPLL_LANE, DP_PWR_STATE_MASK);
> > +
> > +	udelay(50);
> 
> Can usleep_range() be used here? Same for the other delays.

Yes, thanks, I replaced it here and everywhere else.

> 
> [...]
> > +static void mtk_dp_hpd_sink_event(struct mtk_dp *mtk_dp)
> > +{
> [...]
> > +
> > +	if (DP_GET_SINK_COUNT(sink_count) &&
> > +	    (link_status[2] & DP_DOWNSTREAM_PORT_STATUS_CHANGED)) {
> > +		mtk_dp->train_info.check_cap_count = 0;
> > +		kfree(mtk_dp->edid);
> > +		mtk_dp->edid = NULL;
> 
> Should this be protect by a lock? This looks like it could race with the
> access in mtk_dp_edid_parse_audio_capabilities() or mtk_dp_get_edid()

Completely right, I guarded all edid accesses with a mutex now. Thanks.

> 
> [...]
> > +static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)
> > +{
> > +	int ret = 0;
> > +
> > +	ret = mtk_dp_train_hpd_handle(mtk_dp);
> > +
> > +	if (!mtk_dp->train_info.cable_plugged_in)
> > +		return -ENODEV;
> > +
> > +	if (mtk_dp->train_state == MTK_DP_TRAIN_STATE_NORMAL)
> > +		return ret;
> > +
> > +	switch (mtk_dp->train_state) {
> [...]
> > +	case MTK_DP_TRAIN_STATE_TRAINING:
> > +		ret = mtk_dp_train_start(mtk_dp);
> > +		if (!ret) {
> > +			mtk_dp_video_mute(mtk_dp, true);
> > +			mtk_dp_audio_mute(mtk_dp, true);
> > +			mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKTIMING;
> > +			mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);
> > +		}  else if (ret != -EAGAIN)
> > +			mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;
> 
> A small whitespace issue and missing braces.

Thanks for spotting, fixed.

> 
> Consider running this through checkpatch.pl --strict once for style
> issues.

Thanks for the tip, I didn't know about --strict. I now added it to my
editor tooling. Interesting thing: It picked up the missing braces as
well as all the udelays, but not the extra space before else.

> 
> [...]
> > +static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev)
> > +{
> > +	struct mtk_dp *mtk_dp = dev;
> > +	uint32_t irq_status;
> > +
> > +	irq_status = mtk_dp_read(mtk_dp, MTK_DP_TOP_IRQ_STATUS);
> > +
> > +	if (!irq_status)
> > +		return IRQ_HANDLED;
> 
> This check seems superfluous given that only the
> RGS_IRQ_STATUS_TRANSMITTER bit is checked right below:

Thanks, I removed it.

> 
> > +	if (irq_status & RGS_IRQ_STATUS_TRANSMITTER)
> > +		return mtk_dp_hpd_isr_handler(mtk_dp);
> > +
> > +	return IRQ_HANDLED;
> > +}
> [...]
> > +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
> > +				    struct drm_connector *connector)
> > +{
> > +	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> > +	bool pre_enabled = mtk_dp->pre_enabled;
> > +
> > +	if (mtk_dp->edid)
> > +		kfree(mtk_dp->edid);
> 
> Unnecessary check, kfree() accepts NULL.

Fixed.

Thank you Philipp for the review.

Best,
Markus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Chun-Kuang Hu]

Hi, Markus:

Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月7日 週二 上午3:37寫道：
>
> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>
> It supports both functional units on the mt8195, the embedded
> DisplayPort as well as the external DisplayPort units. It offers
> hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> to 4 lanes.
>
> This driver is based on an initial version by
> Jason-JH.Lin <jason-jh.lin@mediatek.com>.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>
> Notes:
>     Changes RFC -> v1:
>     - Removed unused register definitions.
>     - Replaced workqueue with threaded irq.
>     - Removed connector code.
>     - Move to atomic_* drm functions.
>     - General cleanups of the code.
>     - Remove unused select GENERIC_PHY.
>
>  drivers/gpu/drm/mediatek/Kconfig      |    6 +
>  drivers/gpu/drm/mediatek/Makefile     |    2 +
>  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
>  4 files changed, 3469 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> new file mode 100644
> index 000000000000..26ed3b66da91
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -0,0 +1,580 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Copyright (c) 2021 BayLibre
> + */
> +#ifndef _MTK_DP_REG_H_
> +#define _MTK_DP_REG_H_
> +
> +#define MTK_DP_SIP_CONTROL_AARCH32 0x82000523
> +# define MTK_DP_SIP_ATF_VIDEO_UNMUTE 0x20
> +# define MTK_DP_SIP_ATF_EDP_VIDEO_UNMUTE 0x21
> +
> +#define TOP_OFFSET             0x2000
> +#define ENC0_OFFSET            0x3000
> +#define ENC1_OFFSET            0x3200
> +#define TRANS_OFFSET           0x3400
> +#define AUX_OFFSET             0x3600
> +#define SEC_OFFSET             0x4000
> +
> +#define MTK_DP_HPD_DISCONNECT  BIT(1)
> +#define MTK_DP_HPD_CONNECT     BIT(2)
> +#define MTK_DP_HPD_INTERRUPT   BIT(3)
> +
> +#define MTK_DP_ENC0_P0_3000              (ENC0_OFFSET + 0x000)
> +# define LANE_NUM_DP_ENC0_P0_MASK                                      0x3
> +# define VIDEO_MUTE_SW_DP_ENC0_P0_MASK                                 0x4
> +# define VIDEO_MUTE_SW_DP_ENC0_P0_SHIFT                                2
> +# define VIDEO_MUTE_SEL_DP_ENC0_P0_MASK                                0x8
> +# define VIDEO_MUTE_SEL_DP_ENC0_P0_SHIFT                               3
> +# define ENHANCED_FRAME_EN_DP_ENC0_P0_MASK                             0x10
> +# define ENHANCED_FRAME_EN_DP_ENC0_P0_SHIFT                            4
> +
> +#define MTK_DP_ENC0_P0_3004              (ENC0_OFFSET + 0x004)
> +# define VIDEO_M_CODE_SEL_DP_ENC0_P0_MASK                              0x100
> +# define VIDEO_M_CODE_SEL_DP_ENC0_P0_SHIFT                             8
> +# define DP_TX_ENCODER_4P_RESET_SW_DP_ENC0_P0_MASK                     0x200
> +# define DP_TX_ENCODER_4P_RESET_SW_DP_ENC0_P0_SHIFT                    9
> +
> +#define MTK_DP_ENC0_P0_3008              (ENC0_OFFSET + 0x008)
> +# define VIDEO_M_CODE_SW_0_DP_ENC0_P0_MASK                             0xffff
> +
> +#define MTK_DP_ENC0_P0_300C              (ENC0_OFFSET + 0x00C)
> +# define VIDEO_M_CODE_SW_1_DP_ENC0_P0_MASK                             0xff
> +
> +#define MTK_DP_ENC0_P0_3010              (ENC0_OFFSET + 0x010)
> +# define HTOTAL_SW_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3014              (ENC0_OFFSET + 0x014)
> +# define VTOTAL_SW_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3018              (ENC0_OFFSET + 0x018)
> +# define HSTART_SW_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_301C              (ENC0_OFFSET + 0x01C)
> +# define VSTART_SW_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3020              (ENC0_OFFSET + 0x020)
> +# define HWIDTH_SW_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3024              (ENC0_OFFSET + 0x024)
> +# define VHEIGHT_SW_DP_ENC0_P0_MASK                                    0xffff
> +
> +#define MTK_DP_ENC0_P0_3028              (ENC0_OFFSET + 0x028)
> +# define HSW_SW_DP_ENC0_P0_MASK                                        0x7fff
> +# define HSW_SW_DP_ENC0_P0_SHIFT                                       0
> +# define HSP_SW_DP_ENC0_P0_MASK                                        0x8000
> +
> +#define MTK_DP_ENC0_P0_302C              (ENC0_OFFSET + 0x02C)
> +# define VSW_SW_DP_ENC0_P0_MASK                                        0x7fff
> +# define VSW_SW_DP_ENC0_P0_SHIFT                                       0
> +# define VSP_SW_DP_ENC0_P0_MASK                                        0x8000
> +
> +#define MTK_DP_ENC0_P0_3030              (ENC0_OFFSET + 0x030)
> +# define HTOTAL_SEL_DP_ENC0_P0_SHIFT                                   0
> +# define VTOTAL_SEL_DP_ENC0_P0_SHIFT                                   1
> +# define HSTART_SEL_DP_ENC0_P0_SHIFT                                   2
> +# define VSTART_SEL_DP_ENC0_P0_SHIFT                                   3
> +# define HWIDTH_SEL_DP_ENC0_P0_SHIFT                                   4
> +# define VHEIGHT_SEL_DP_ENC0_P0_SHIFT                                  5
> +# define HSP_SEL_DP_ENC0_P0_SHIFT                                      6
> +# define HSW_SEL_DP_ENC0_P0_SHIFT                                      7
> +# define VSP_SEL_DP_ENC0_P0_SHIFT                                      8
> +# define VSW_SEL_DP_ENC0_P0_SHIFT                                      9
> +# define VBID_AUDIO_MUTE_FLAG_SW_DP_ENC0_P0_MASK                       0x800
> +# define VBID_AUDIO_MUTE_SW_DP_ENC0_P0_SHIFT                           11
> +# define VBID_AUDIO_MUTE_FLAG_SEL_DP_ENC0_P0_MASK                      0x1000
> +# define VBID_AUDIO_MUTE_SEL_DP_ENC0_P0_SHIFT                          12
> +
> +#define MTK_DP_ENC0_P0_3034              (ENC0_OFFSET + 0x034)
> +
> +#define MTK_DP_ENC0_P0_3038              (ENC0_OFFSET + 0x038)
> +# define VIDEO_SOURCE_SEL_DP_ENC0_P0_MASK                              0x800
> +# define VIDEO_SOURCE_SEL_DP_ENC0_P0_SHIFT                             11
> +
> +#define MTK_DP_ENC0_P0_303C              (ENC0_OFFSET + 0x03C)
> +# define SRAM_START_READ_THRD_DP_ENC0_P0_MASK                          0x3f
> +# define SRAM_START_READ_THRD_DP_ENC0_P0_SHIFT                         0
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_MASK                             0x700
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT                            8
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_16BIT                            (0 << VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT)
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_12BIT                            (1 << VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT)
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_10BIT                            (2 << VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT)
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_8BIT                             (3 << VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT)
> +# define VIDEO_COLOR_DEPTH_DP_ENC0_P0_6BIT                             (4 << VIDEO_COLOR_DEPTH_DP_ENC0_P0_SHIFT)
> +# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK                           0x7000
> +# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_SHIFT                          12
> +# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_RGB                            (0 << PIXEL_ENCODE_FORMAT_DP_ENC0_P0_SHIFT)
> +# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR422                       (1 << PIXEL_ENCODE_FORMAT_DP_ENC0_P0_SHIFT)
> +# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR420                       (2 << PIXEL_ENCODE_FORMAT_DP_ENC0_P0_SHIFT)
> +# define VIDEO_MN_GEN_EN_DP_ENC0_P0_MASK                               0x8000
> +# define VIDEO_MN_GEN_EN_DP_ENC0_P0_SHIFT                              15
> +
> +#define MTK_DP_ENC0_P0_3040              (ENC0_OFFSET + 0x040)
> +# define SDP_DOWN_CNT_INIT_DP_ENC0_P0_MASK                             0xfff
> +# define SDP_DOWN_CNT_INIT_DP_ENC0_P0_SHIFT                            0
> +
> +#define MTK_DP_ENC0_P0_3044              (ENC0_OFFSET + 0x044)
> +# define VIDEO_N_CODE_0_DP_ENC0_P0_MASK                                0xffff
> +
> +#define MTK_DP_ENC0_P0_3048              (ENC0_OFFSET + 0x048)
> +# define VIDEO_N_CODE_1_DP_ENC0_P0_MASK                                0xff
> +
> +#define MTK_DP_ENC0_P0_304C              (ENC0_OFFSET + 0x04C)
> +# define VBID_VIDEO_MUTE_DP_ENC0_P0_MASK                                 0x4
> +# define SDP_VSYNC_RISING_MASK_DP_ENC0_P0_MASK                           0x100
> +
> +#define MTK_DP_ENC0_P0_3050              (ENC0_OFFSET + 0x050)
> +# define VIDEO_N_CODE_MN_GEN_0_DP_ENC0_P0_MASK                           0xffff
> +
> +#define MTK_DP_ENC0_P0_3054              (ENC0_OFFSET + 0x054)
> +# define VIDEO_N_CODE_MN_GEN_1_DP_ENC0_P0_MASK                           0xff
> +
> +#define MTK_DP_ENC0_P0_3064              (ENC0_OFFSET + 0x064)
> +# define HDE_NUM_LAST_DP_ENC0_P0_MASK                                    0xffff
> +
> +#define MTK_DP_ENC0_P0_3088              (ENC0_OFFSET + 0x088)
> +# define AU_EN_DP_ENC0_P0_MASK                                           0x40
> +# define AU_EN_DP_ENC0_P0_SHIFT                                          6
> +# define AUDIO_8CH_EN_DP_ENC0_P0_MASK                                    0x80
> +# define AUDIO_8CH_SEL_DP_ENC0_P0_MASK                                   0x100
> +# define AUDIO_2CH_EN_DP_ENC0_P0_MASK                                    0x4000
> +# define AUDIO_2CH_SEL_DP_ENC0_P0_MASK                                   0x8000
> +
> +#define MTK_DP_ENC0_P0_308C              (ENC0_OFFSET + 0x08C)
> +# define CH_STATUS_0_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3090              (ENC0_OFFSET + 0x090)
> +# define CH_STATUS_1_DP_ENC0_P0_MASK                                     0xffff
> +
> +#define MTK_DP_ENC0_P0_3094              (ENC0_OFFSET + 0x094)
> +# define CH_STATUS_2_DP_ENC0_P0_MASK                                     0xff
> +
> +#define MTK_DP_ENC0_P0_30A0              (ENC0_OFFSET + 0x0A0)
> +
> +#define MTK_DP_ENC0_P0_30A4              (ENC0_OFFSET + 0x0A4)
> +# define AU_TS_CFG_DP_ENC0_P0_MASK                                       0xff
> +
> +#define MTK_DP_ENC0_P0_30A8              (ENC0_OFFSET + 0x0A8)
> +
> +#define MTK_DP_ENC0_P0_30AC              (ENC0_OFFSET + 0x0AC)
> +
> +#define MTK_DP_ENC0_P0_30B0              (ENC0_OFFSET + 0x0B0)
> +
> +#define MTK_DP_ENC0_P0_30B4              (ENC0_OFFSET + 0x0B4)
> +# define ISRC_CFG_DP_ENC0_P0_MASK                                        0xff00
> +# define ISRC_CFG_DP_ENC0_P0_SHIFT                                       8
> +
> +#define MTK_DP_ENC0_P0_30B8              (ENC0_OFFSET + 0x0B8)
> +
> +#define MTK_DP_ENC0_P0_30BC              (ENC0_OFFSET + 0x0BC)
> +# define ISRC_CONT_DP_ENC0_P0_MASK                                       0x1
> +# define ISRC_CONT_DP_ENC0_P0_SHIFT                                      0
> +# define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_MASK                       0x700
> +# define AUDIO_M_CODE_MULT_DIV_SEL_DP_ENC0_P0_SHIFT                      8
> +
> +#define MTK_DP_ENC0_P0_30D8              (ENC0_OFFSET + 0x0D8)
> +
> +#define MTK_DP_ENC0_P0_312C              (ENC0_OFFSET + 0x12C)
> +# define ASP_HB2_DP_ENC0_P0_MASK                                         0xff
> +# define ASP_HB3_DP_ENC0_P0_MASK                                         0xff00
> +# define ASP_HB3_DP_ENC0_P0_SHIFT                                        8
> +
> +#define MTK_DP_ENC0_P0_3130              (ENC0_OFFSET + 0x130)
> +
> +#define MTK_DP_ENC0_P0_3138              (ENC0_OFFSET + 0x138)
> +
> +#define MTK_DP_ENC0_P0_3154              (ENC0_OFFSET + 0x154)
> +# define PGEN_HTOTAL_DP_ENC0_P0_MASK                                     0x3fff
> +
> +#define MTK_DP_ENC0_P0_3158              (ENC0_OFFSET + 0x158)
> +# define PGEN_HSYNC_RISING_DP_ENC0_P0_MASK                               0x3fff
> +
> +#define MTK_DP_ENC0_P0_315C              (ENC0_OFFSET + 0x15C)
> +# define PGEN_HSYNC_PULSE_WIDTH_DP_ENC0_P0_MASK                          0x3fff
> +
> +#define MTK_DP_ENC0_P0_3160              (ENC0_OFFSET + 0x160)
> +# define PGEN_HFDE_START_DP_ENC0_P0_MASK                                 0x3fff
> +
> +#define MTK_DP_ENC0_P0_3164              (ENC0_OFFSET + 0x164)
> +# define PGEN_HFDE_ACTIVE_WIDTH_DP_ENC0_P0_MASK                          0x3fff
> +
> +#define MTK_DP_ENC0_P0_3168              (ENC0_OFFSET + 0x168)
> +# define PGEN_VTOTAL_DP_ENC0_P0_MASK                                     0x1fff
> +
> +#define MTK_DP_ENC0_P0_316C              (ENC0_OFFSET + 0x16C)
> +# define PGEN_VSYNC_RISING_DP_ENC0_P0_MASK                               0x1fff
> +
> +#define MTK_DP_ENC0_P0_3170              (ENC0_OFFSET + 0x170)
> +# define PGEN_VSYNC_PULSE_WIDTH_DP_ENC0_P0_MASK                          0x1fff
> +
> +#define MTK_DP_ENC0_P0_3174              (ENC0_OFFSET + 0x174)
> +# define PGEN_VFDE_START_DP_ENC0_P0_MASK                                 0x1fff
> +
> +#define MTK_DP_ENC0_P0_3178              (ENC0_OFFSET + 0x178)
> +# define PGEN_VFDE_ACTIVE_WIDTH_DP_ENC0_P0_MASK                          0x1fff
> +
> +#define MTK_DP_ENC0_P0_31B0              (ENC0_OFFSET + 0x1B0)
> +
> +#define MTK_DP_ENC0_P0_31C8              (ENC0_OFFSET + 0x1C8)
> +# define VSC_EXT_VESA_HB0_DP_ENC0_P0_MASK                                0xff
> +# define VSC_EXT_VESA_HB1_DP_ENC0_P0_MASK                                0xff00
> +# define VSC_EXT_VESA_HB1_DP_ENC0_P0_SHIFT                               8
> +
> +#define MTK_DP_ENC0_P0_31CC              (ENC0_OFFSET + 0x1CC)
> +# define VSC_EXT_VESA_HB2_DP_ENC0_P0_MASK                                0xff
> +# define VSC_EXT_VESA_HB2_DP_ENC0_P0_SHIFT                               0
> +# define VSC_EXT_VESA_HB3_DP_ENC0_P0_MASK                                0xff00
> +
> +#define MTK_DP_ENC0_P0_31D0              (ENC0_OFFSET + 0x1D0)
> +# define VSC_EXT_CEA_HB0_DP_ENC0_P0_MASK                                 0xff
> +# define VSC_EXT_CEA_HB1_DP_ENC0_P0_MASK                                 0xff00
> +# define VSC_EXT_CEA_HB1_DP_ENC0_P0_SHIFT                                8
> +
> +#define MTK_DP_ENC0_P0_31D4              (ENC0_OFFSET + 0x1D4)
> +# define VSC_EXT_CEA_HB2_DP_ENC0_P0_MASK                                 0xff
> +# define VSC_EXT_CEA_HB2_DP_ENC0_P0_SHIFT                                0
> +# define VSC_EXT_CEA_HB3_DP_ENC0_P0_MASK                                 0xff00
> +
> +#define MTK_DP_ENC0_P0_31D8              (ENC0_OFFSET + 0x1D8)
> +# define VSC_EXT_VESA_NUM_DP_ENC0_P0_MASK                                0x3f
> +# define VSC_EXT_VESA_NUM_DP_ENC0_P0_SHIFT                               0
> +# define VSC_EXT_CEA_NUM_DP_ENC0_P0_MASK                                 0x3f00
> +# define VSC_EXT_CEA_NUM_DP_ENC0_P0_SHIFT                                8
> +
> +#define MTK_DP_ENC0_P0_31DC              (ENC0_OFFSET + 0x1DC)
> +# define HDR0_CFG_DP_ENC0_P0_MASK                                        0xff
> +# define HDR0_CFG_DP_ENC0_P0_SHIFT                                       0
> +
> +#define MTK_DP_ENC0_P0_31E8              (ENC0_OFFSET + 0x1E8)
> +
> +#define MTK_DP_ENC0_P0_31EC              (ENC0_OFFSET + 0x1EC)
> +# define AUDIO_CH_SRC_SEL_DP_ENC0_P0_MASK                                0x10
> +# define AUDIO_CH_SRC_SEL_DP_ENC0_P0_SHIFT                               4
> +# define ISRC1_HB3_DP_ENC0_P0_MASK                                       0xff00
> +# define ISRC1_HB3_DP_ENC0_P0_SHIFT                                      8
> +
> +#define MTK_DP_ENC1_P0_3200              (ENC1_OFFSET + 0x000)
> +
> +#define MTK_DP_ENC1_P0_3280              (ENC1_OFFSET + 0x080)
> +# define SDP_PACKET_TYPE_DP_ENC1_P0_MASK                                 0x1f
> +# define SDP_PACKET_W_DP_ENC1_P0                                         0x20
> +# define SDP_PACKET_W_DP_ENC1_P0_MASK                                    0x20
> +# define SDP_PACKET_W_DP_ENC1_P0_SHIFT                                   5
> +
> +#define MTK_DP_ENC1_P0_328C              (ENC1_OFFSET + 0x08C)
> +
> +#define MTK_DP_ENC1_P0_3290              (ENC1_OFFSET + 0x090)
> +
> +#define MTK_DP_ENC1_P0_32A0              (ENC1_OFFSET + 0x0A0)
> +
> +#define MTK_DP_ENC1_P0_32A4              (ENC1_OFFSET + 0x0A4)
> +
> +#define MTK_DP_ENC1_P0_3300              (ENC1_OFFSET + 0x100)
> +# define VIDEO_AFIFO_RDY_SEL_DP_ENC1_P0_MASK                             0x300
> +# define VIDEO_AFIFO_RDY_SEL_DP_ENC1_P0_SHIFT                            8
> +
> +#define MTK_DP_ENC1_P0_3304              (ENC1_OFFSET + 0x104)
> +# define AU_PRTY_REGEN_DP_ENC1_P0_MASK                                   0x100
> +# define AU_CH_STS_REGEN_DP_ENC1_P0_MASK                                 0x200
> +# define AUDIO_SAMPLE_PRSENT_REGEN_DP_ENC1_P0_MASK                       0x1000
> +
> +#define MTK_DP_ENC1_P0_3324              (ENC1_OFFSET + 0x124)
> +# define AUDIO_SOURCE_MUX_DP_ENC1_P0_MASK                                0x300
> +# define AUDIO_SOURCE_MUX_DP_ENC1_P0_SHIFT                               8
> +# define AUDIO_SOURCE_MUX_DP_ENC1_P0_DPRX                                (0 << AUDIO_SOURCE_MUX_DP_ENC1_P0_SHIFT)
> +
> +#define MTK_DP_ENC1_P0_3364              (ENC1_OFFSET + 0x164)
> +# define SDP_DOWN_CNT_INIT_IN_HBLANK_DP_ENC1_P0_MASK                     0xfff
> +# define SDP_DOWN_CNT_INIT_IN_HBLANK_DP_ENC1_P0_SHIFT                    0
> +# define FIFO_READ_START_POINT_DP_ENC1_P0_MASK                           0xf000
> +# define FIFO_READ_START_POINT_DP_ENC1_P0_SHIFT                          12
> +
> +#define MTK_DP_ENC1_P0_3368              (ENC1_OFFSET + 0x168)
> +# define VIDEO_SRAM_FIFO_CNT_RESET_SEL_DP_ENC1_P0_SHIFT                  0
> +# define VIDEO_STABLE_CNT_THRD_DP_ENC1_P0_SHIFT                          4
> +# define SDP_DP13_EN_DP_ENC1_P0_SHIFT                                    8
> +# define BS2BS_MODE_DP_ENC1_P0_MASK                                      0x3000
> +# define BS2BS_MODE_DP_ENC1_P0_SHIFT                                     12
> +
> +#define MTK_DP_ENC1_P0_33F4              (ENC1_OFFSET + 0x1F4)
> +
> +#define MTK_DP_TRANS_P0_3400              (TRANS_OFFSET + 0x000)
> +# define PATTERN1_EN_DP_TRANS_P0_MASK                                        0x1000
> +# define PATTERN1_EN_DP_TRANS_P0_SHIFT                                       12
> +# define PATTERN2_EN_DP_TRANS_P0_MASK                                        0x2000
> +# define PATTERN3_EN_DP_TRANS_P0_MASK                                        0x4000
> +# define PATTERN4_EN_DP_TRANS_P0_MASK                                        0x8000
> +
> +#define MTK_DP_TRANS_P0_3404              (TRANS_OFFSET + 0x004)
> +# define DP_SCR_EN_DP_TRANS_P0_MASK                                          0x1
> +
> +#define MTK_DP_TRANS_P0_340C              (TRANS_OFFSET + 0x00C)
> +# define DP_TX_TRANSMITTER_4P_RESET_SW_DP_TRANS_P0_MASK                      0x2000
> +# define DP_TX_TRANSMITTER_4P_RESET_SW_DP_TRANS_P0_SHIFT                     13
> +
> +#define MTK_DP_TRANS_P0_3410              (TRANS_OFFSET + 0x010)
> +# define HPD_DEB_THD_DP_TRANS_P0_MASK                                        0xf
> +# define HPD_DEB_THD_DP_TRANS_P0_SHIFT                                       0
> +# define HPD_INT_THD_DP_TRANS_P0_MASK                                        0xf0
> +# define HPD_INT_THD_DP_TRANS_P0_SHIFT                                       4
> +# define HPD_INT_THD_DP_TRANS_P0_LOWER_500US                                 (2 << HPD_INT_THD_DP_TRANS_P0_SHIFT)
> +# define HPD_INT_THD_DP_TRANS_P0_UPPER_1100US                                (2 << (HPD_INT_THD_DP_TRANS_P0_SHIFT + 2))
> +# define HPD_DISC_THD_DP_TRANS_P0_MASK                                       0xf00
> +# define HPD_DISC_THD_DP_TRANS_P0_SHIFT                                      8
> +# define HPD_CONN_THD_DP_TRANS_P0_MASK                                       0xf000
> +# define HPD_CONN_THD_DP_TRANS_P0_SHIFT                                      12
> +
> +#define MTK_DP_TRANS_P0_3414              (TRANS_OFFSET + 0x014)
> +# define HPD_DB_DP_TRANS_P0_MASK                                             0x4
> +
> +#define MTK_DP_TRANS_P0_3418              (TRANS_OFFSET + 0x018)
> +# define IRQ_CLR_DP_TRANS_P0_MASK                                            0xf
> +# define IRQ_MASK_DP_TRANS_P0_MASK                                           0xf0
> +# define IRQ_MASK_DP_TRANS_P0_SHIFT                                          4
> +# define IRQ_MASK_DP_TRANS_P0_DISC_IRQ                                       (BIT(1) << IRQ_MASK_DP_TRANS_P0_SHIFT)
> +# define IRQ_MASK_DP_TRANS_P0_CONN_IRQ                                       (BIT(2) << IRQ_MASK_DP_TRANS_P0_SHIFT)
> +# define IRQ_MASK_DP_TRANS_P0_INT_IRQ                                        (BIT(3) << IRQ_MASK_DP_TRANS_P0_SHIFT)
> +# define IRQ_STATUS_DP_TRANS_P0_MASK                                         0xf000
> +# define IRQ_STATUS_DP_TRANS_P0_SHIFT                                        12
> +
> +#define MTK_DP_TRANS_P0_342C              (TRANS_OFFSET + 0x02C)
> +# define XTAL_FREQ_DP_TRANS_P0_DEFAULT                                       0x69
> +# define XTAL_FREQ_DP_TRANS_P0_MASK                                          0xff
> +
> +#define MTK_DP_TRANS_P0_3430              (TRANS_OFFSET + 0x030)
> +# define HPD_INT_THD_ECO_DP_TRANS_P0_MASK                                    0x3
> +# define HPD_INT_THD_ECO_DP_TRANS_P0_HIGH_BOUND_EXT                          BIT(1)
> +
> +#define MTK_DP_TRANS_P0_34A4              (TRANS_OFFSET + 0x0A4)
> +# define LANE_NUM_DP_TRANS_P0_MASK                                           0xc
> +# define LANE_NUM_DP_TRANS_P0_SHIFT                                          2
> +
> +#define MTK_DP_TRANS_P0_3540              (TRANS_OFFSET + 0x140)
> +# define FEC_EN_DP_TRANS_P0_MASK                                             0x1
> +# define FEC_EN_DP_TRANS_P0_SHIFT                                            0
> +# define FEC_CLOCK_EN_MODE_DP_TRANS_P0_MASK                                  0x8
> +# define FEC_CLOCK_EN_MODE_DP_TRANS_P0_SHIFT                                 3
> +
> +#define MTK_DP_TRANS_P0_3580              (TRANS_OFFSET + 0x180)
> +# define POST_MISC_DATA_LANE0_OV_DP_TRANS_P0_MASK                            0x100
> +# define POST_MISC_DATA_LANE1_OV_DP_TRANS_P0_MASK                            0x200
> +# define POST_MISC_DATA_LANE2_OV_DP_TRANS_P0_MASK                            0x400
> +# define POST_MISC_DATA_LANE3_OV_DP_TRANS_P0_MASK                            0x800
> +
> +#define MTK_DP_TRANS_P0_35C4              (TRANS_OFFSET + 0x1C4)
> +# define SW_IRQ_MASK_DP_TRANS_P0_MASK                                        0xffff
> +
> +#define MTK_DP_TRANS_P0_35C8              (TRANS_OFFSET + 0x1C8)
> +# define SW_IRQ_CLR_DP_TRANS_P0_MASK                                         0xffff
> +
> +# define SW_IRQ_STATUS_DP_TRANS_P0_MASK                                      0xffff
> +# define SW_IRQ_STATUS_DP_TRANS_P0_SHIFT                                     0
> +
> +#define MTK_DP_TRANS_P0_35D0              (TRANS_OFFSET + 0x1D0)
> +# define SW_IRQ_FINAL_STATUS_DP_TRANS_P0_MASK                                0xffff
> +
> +#define MTK_DP_TRANS_P0_35F0              (TRANS_OFFSET + 0x1F0)
> +
> +#define MTK_DP_AUX_P0_360C              (AUX_OFFSET + 0x00C)
> +# define AUX_TIMEOUT_THR_AUX_TX_P0_MASK                                      0x1fff
> +
> +#define MTK_DP_AUX_P0_3614              (AUX_OFFSET + 0x014)
> +# define AUX_RX_UI_CNT_THR_AUX_TX_P0_MASK                                    0x7f
> +# define AUX_RX_UI_CNT_THR_AUX_TX_P0_SHIFT                                   0
> +
> +#define MTK_DP_AUX_P0_3618              (AUX_OFFSET + 0x018)
> +# define AUX_RX_FIFO_FULL_AUX_TX_P0_MASK                                     0x200
> +# define AUX_RX_FIFO_WRITE_POINTER_AUX_TX_P0_MASK                            0xf
> +
> +#define MTK_DP_AUX_P0_3620              (AUX_OFFSET + 0x020)
> +# define AUX_RD_MODE_AUX_TX_P0_MASK                                          0x200
> +# define AUX_RX_FIFO_READ_PULSE_TX_P0_MASK                                   0x100
> +# define AUX_RX_FIFO_R_PULSE_TX_P0_SHIFT                                     8
> +# define AUX_RX_FIFO_READ_DATA_AUX_TX_P0_MASK                                0xff
> +# define AUX_RX_FIFO_READ_DATA_AUX_TX_P0_SHIFT                               0
> +
> +#define MTK_DP_AUX_P0_3624              (AUX_OFFSET + 0x024)
> +# define AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK                                 0xf
> +
> +#define MTK_DP_AUX_P0_3628              (AUX_OFFSET + 0x028)
> +# define AUX_RX_PHY_STATE_AUX_TX_P0_MASK                                     0x3ff
> +# define AUX_RX_PHY_STATE_AUX_TX_P0_SHIFT                                    0
> +# define AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE                                  (BIT(1) << AUX_RX_PHY_STATE_AUX_TX_P0_SHIFT)
> +
> +#define MTK_DP_AUX_P0_362C              (AUX_OFFSET + 0x02C)
> +# define AUX_NO_LENGTH_AUX_TX_P0_MASK                                        0x1
> +# define AUX_NO_LENGTH_AUX_TX_P0_SHIFT                                       0
> +# define AUX_TX_AUXTX_OV_EN_AUX_TX_P0_MASK                                   0x2
> +# define AUX_RESERVED_RW_0_AUX_TX_P0_MASK                                    0xfffc
> +
> +#define MTK_DP_AUX_P0_3630              (AUX_OFFSET + 0x030)
> +# define AUX_TX_REQUEST_READY_AUX_TX_P0_MASK                                 0x8
> +# define AUX_TX_REQUEST_READY_AUX_TX_P0_SHIFT                                3
> +
> +#define MTK_DP_AUX_P0_3634              (AUX_OFFSET + 0x034)
> +# define AUX_TX_OVER_SAMPLE_RATE_AUX_TX_P0_MASK                              0xff00
> +# define AUX_TX_OVER_SAMPLE_RATE_AUX_TX_P0_SHIFT                             8
> +
> +#define MTK_DP_AUX_P0_3640              (AUX_OFFSET + 0x040)
> +# define AUX_RX_RECV_COMPLETE_IRQ_TX_P0_MASK                                 0x40
> +# define AUX_RX_AUX_RECV_COMPLETE_IRQ_AUX_TX_P0_SHIFT                        6
> +# define AUX_RX_EDID_RECV_COMPLETE_IRQ_AUX_TX_P0_SHIFT                       5
> +# define AUX_RX_MCCS_RECV_COMPLETE_IRQ_AUX_TX_P0_SHIFT                       4
> +# define AUX_RX_CMD_RECV_IRQ_AUX_TX_P0_SHIFT                                 3
> +# define AUX_RX_ADDR_RECV_IRQ_AUX_TX_P0_SHIFT                                2
> +# define AUX_RX_DATA_RECV_IRQ_AUX_TX_P0_SHIFT                                1
> +# define AUX_400US_TIMEOUT_IRQ_AUX_TX_P0_MASK                                0x1
> +# define AUX_400US_TIMEOUT_IRQ_AUX_TX_P0_SHIFT                               0
> +
> +#define MTK_DP_AUX_P0_3644              (AUX_OFFSET + 0x044)
> +# define MCU_REQUEST_COMMAND_AUX_TX_P0_MASK                                  0xf
> +
> +#define MTK_DP_AUX_P0_3648              (AUX_OFFSET + 0x048)
> +# define MCU_REQUEST_ADDRESS_LSB_AUX_TX_P0_MASK                              0xffff
> +
> +#define MTK_DP_AUX_P0_364C              (AUX_OFFSET + 0x04C)
> +# define MCU_REQUEST_ADDRESS_MSB_AUX_TX_P0_MASK                              0xf
> +
> +#define MTK_DP_AUX_P0_3650              (AUX_OFFSET + 0x050)
> +# define MCU_REQ_DATA_NUM_AUX_TX_P0_MASK                                     0xf000
> +# define MCU_REQ_DATA_NUM_AUX_TX_P0_SHIFT                                    12
> +# define PHY_FIFO_RST_AUX_TX_P0_MASK                                         0x200
> +# define MCU_ACK_TRAN_COMPLETE_AUX_TX_P0_MASK                                0x100
> +# define MCU_ACK_TRAN_COMPLETE_AUX_TX_P0_SHIFT                               8
> +
> +#define MTK_DP_AUX_P0_3658              (AUX_OFFSET + 0x058)
> +# define AUX_TX_OV_EN_AUX_TX_P0_MASK                                         0x1
> +
> +#define MTK_DP_AUX_P0_3690              (AUX_OFFSET + 0x090)
> +# define RX_REPLY_COMPLETE_MODE_AUX_TX_P0_MASK                               0x100
> +# define RX_REPLY_COMPLETE_MODE_AUX_TX_P0_SHIFT                              8
> +
> +#define MTK_DP_AUX_P0_3704              (AUX_OFFSET + 0x104)
> +# define AUX_TX_FIFO_WRITE_DATA_NEW_MODE_TOGGLE_AUX_TX_P0_MASK               0x2
> +# define AUX_TX_FIFO_NEW_MODE_EN_AUX_TX_P0_MASK                              0x4
> +# define AUX_TX_FIFO_NEW_MODE_EN_AUX_TX_P0_SHIFT                             2
> +
> +#define MTK_DP_AUX_P0_3708              (AUX_OFFSET + 0x108)
> +
> +#define MTK_DP_AUX_P0_37C8              (AUX_OFFSET + 0x1C8)
> +# define MTK_ATOP_EN_AUX_TX_P0_MASK                                          0x1
> +# define MTK_ATOP_EN_AUX_TX_P0_SHIFT                                         0
> +
> +#define MTK_DP_TOP_PWR_STATE              (TOP_OFFSET + 0x00)
> +# define DP_PWR_STATE_MASK                                                   0x3
> +# define DP_PWR_STATE_BANDGAP                                                1
> +# define DP_PWR_STATE_BANDGAP_TPLL                                           2
> +# define DP_PWR_STATE_BANDGAP_TPLL_LANE                                      3
> +
> +#define MTK_DP_TOP_SWING_EMP              (TOP_OFFSET + 0x04)
> +# define DP_TX0_VOLT_SWING_MASK                                              0x3
> +# define DP_TX0_VOLT_SWING_SHIFT                                             0
> +# define DP_TX0_PRE_EMPH_MASK                                                0xc
> +# define DP_TX0_PRE_EMPH_SHIFT                                               2
> +# define DP_TX1_VOLT_SWING_MASK                                              0x300
> +# define DP_TX1_VOLT_SWING_SHIFT                                             8
> +# define DP_TX1_PRE_EMPH_MASK                                                0xc00
> +# define DP_TX2_VOLT_SWING_MASK                                              0x30000
> +# define DP_TX2_PRE_EMPH_MASK                                                0xc0000
> +# define DP_TX3_VOLT_SWING_MASK                                              0x3000000
> +# define DP_TX3_PRE_EMPH_MASK                                                0xc000000
> +
> +#define MTK_DP_TOP_RESET_AND_PROBE              (TOP_OFFSET + 0x20)
> +# define SW_RST_B_SHIFT                                           0
> +# define SW_RST_B_PHYD                                            (BIT(4) << SW_RST_B_SHIFT)
> +
> +#define MTK_DP_TOP_IRQ_STATUS              (TOP_OFFSET + 0x28)
> +# define RGS_IRQ_STATUS_SHIFT                                     0
> +# define RGS_IRQ_STATUS_TRANSMITTER                               (BIT(1) << RGS_IRQ_STATUS_SHIFT)
> +
> +#define MTK_DP_TOP_IRQ_MASK              (TOP_OFFSET + 0x2C)
> +# define IRQ_MASK_AUX_TOP_IRQ                                     BIT(2)
> +
> +#define MTK_DP_TOP_MEM_PD              (TOP_OFFSET + 0x38)
> +# define MEM_ISO_EN_SHIFT                                         0
> +# define FUSE_SEL_SHIFT                                           2
> +
> +#define DP_PHY_DIG_PLL_CTL_1                                   0x1014
> +# define TPLL_SSC_EN                                           BIT(3)

It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~
0x4fff is to control non-phy part. For mipi and hdmi, the phy part is
an independent device [1] and the phy driver is independent [2] , so I
would like this phy to be an independent device.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14

Regards,
Chun-Kuang.

> +
> +#define DP_PHY_DIG_BIT_RATE                                    0x103C
> +# define BIT_RATE_RBR                                          0
> +# define BIT_RATE_HBR                                          1
> +# define BIT_RATE_HBR2                                         2
> +# define BIT_RATE_HBR3                                         3
> +
> +#define DP_PHY_DIG_SW_RST                                      0x1038
> +# define DP_GLB_SW_RST_PHYD                                    BIT(0)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_3                           0x1138
> +#define MTK_DP_LANE1_DRIVING_PARAM_3                           0x1238
> +#define MTK_DP_LANE2_DRIVING_PARAM_3                           0x1338
> +#define MTK_DP_LANE3_DRIVING_PARAM_3                           0x1438
> +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT                     0x10
> +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT                     (0x14 << 8)
> +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT                     (0x18 << 16)
> +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT                     (0x20 << 24)
> +# define DRIVING_PARAM_3_DEFAULT                               (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_4                           0x113C
> +#define MTK_DP_LANE1_DRIVING_PARAM_4                           0x123C
> +#define MTK_DP_LANE2_DRIVING_PARAM_4                           0x133C
> +#define MTK_DP_LANE3_DRIVING_PARAM_4                           0x143C
> +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT                     0x18
> +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT                     (0x1e << 8)
> +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT                     (0x24 << 16)
> +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT                     (0x20 << 24)
> +# define DRIVING_PARAM_4_DEFAULT                               (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_5                           0x1140
> +#define MTK_DP_LANE1_DRIVING_PARAM_5                           0x1240
> +#define MTK_DP_LANE2_DRIVING_PARAM_5                           0x1340
> +#define MTK_DP_LANE3_DRIVING_PARAM_5                           0x1440
> +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT                     0x28
> +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT                     (0x30 << 8)
> +# define DRIVING_PARAM_5_DEFAULT                               (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_6                           0x1144
> +#define MTK_DP_LANE1_DRIVING_PARAM_6                           0x1244
> +#define MTK_DP_LANE2_DRIVING_PARAM_6                           0x1344
> +#define MTK_DP_LANE3_DRIVING_PARAM_6                           0x1444
> +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT                    0x00
> +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT                    (0x04 << 8)
> +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT                    (0x08 << 16)
> +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT                    (0x10 << 24)
> +# define DRIVING_PARAM_6_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_7                           0x1148
> +#define MTK_DP_LANE1_DRIVING_PARAM_7                           0x1248
> +#define MTK_DP_LANE2_DRIVING_PARAM_7                           0x1348
> +#define MTK_DP_LANE3_DRIVING_PARAM_7                           0x1448
> +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT                    0x00
> +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT                    (0x06 << 8)
> +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT                    (0x0c << 16)
> +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT                    (0x00 << 24)
> +# define DRIVING_PARAM_7_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
> +
> +#define MTK_DP_LANE0_DRIVING_PARAM_8                           0x114C
> +#define MTK_DP_LANE1_DRIVING_PARAM_8                           0x124C
> +#define MTK_DP_LANE2_DRIVING_PARAM_8                           0x134C
> +#define MTK_DP_LANE3_DRIVING_PARAM_8                           0x144C
> +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT                    0x08
> +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT                    (0x00 << 8)
> +# define DRIVING_PARAM_8_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
> +                                                                XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
> +
> +#endif /*_MTK_DP_REG_H_*/
> --
> 2.33.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Markus Schneider-Pargmann]

Hi Chun-Kuang,

On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:
> Hi, Markus:
> 
> Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月7日 週二 上午3:37寫道：
> >
> > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> >
> > It supports both functional units on the mt8195, the embedded
> > DisplayPort as well as the external DisplayPort units. It offers
> > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > to 4 lanes.
> >
> > This driver is based on an initial version by
> > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> >
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >
> > Notes:
> >     Changes RFC -> v1:
> >     - Removed unused register definitions.
> >     - Replaced workqueue with threaded irq.
> >     - Removed connector code.
> >     - Move to atomic_* drm functions.
> >     - General cleanups of the code.
> >     - Remove unused select GENERIC_PHY.
> >
> >  drivers/gpu/drm/mediatek/Kconfig      |    6 +
> >  drivers/gpu/drm/mediatek/Makefile     |    2 +
> >  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
> >  4 files changed, 3469 insertions(+)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> >

...

> > +#define TOP_OFFSET             0x2000
> > +#define ENC0_OFFSET            0x3000
> > +#define ENC1_OFFSET            0x3200
> > +#define TRANS_OFFSET           0x3400
> > +#define AUX_OFFSET             0x3600
> > +#define SEC_OFFSET             0x4000

...

> > +
> > +#define DP_PHY_DIG_PLL_CTL_1                                   0x1014
> > +# define TPLL_SSC_EN                                           BIT(3)
> 
> It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~
> 0x4fff is to control non-phy part. For mipi and hdmi, the phy part is
> an independent device [1] and the phy driver is independent [2] , so I
> would like this phy to be an independent device.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14

Thanks for your feedback. I looked into both mipi and hdmi phy drivers
that you referenced. It looks like both are really separate units in
their SoCs having their own registerspaces located at a completely
different range than the units using the phy.

For this displayport driver, the phy registers are listed as part of the
(e)DP_TX unit in the datasheet. Next to the phy registers all the other
parts are listed as well in the same overall register ranges (see
above), e.g. TOP_OFFSET, ENC_OFFSET or SEC_OFFSET. Also I would like to
avoid splitting it up into a separate unit in the devicetree as the
datasheet handles it as a single unit (including the phy registers).

From a practical perspective there is also not much to these PHY
registers. The only things that would be done in the driver are:
- initializing the lane driving parameters with static values
- setup the bitrate
- enable/disable SSC
- do a reset
Exporting these four used functions over a driver boundary wouldn't help
clarity I think and the code probably can't be reused by any other
component anyways.

So I personally would prefer keeping it as part of the whole driver
because of the above mentioned reasons. What do you think?

Thanks,
Markus

> 
> Regards,
> Chun-Kuang.
> 
> > +
> > +#define DP_PHY_DIG_BIT_RATE                                    0x103C
> > +# define BIT_RATE_RBR                                          0
> > +# define BIT_RATE_HBR                                          1
> > +# define BIT_RATE_HBR2                                         2
> > +# define BIT_RATE_HBR3                                         3
> > +
> > +#define DP_PHY_DIG_SW_RST                                      0x1038
> > +# define DP_GLB_SW_RST_PHYD                                    BIT(0)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_3                           0x1138
> > +#define MTK_DP_LANE1_DRIVING_PARAM_3                           0x1238
> > +#define MTK_DP_LANE2_DRIVING_PARAM_3                           0x1338
> > +#define MTK_DP_LANE3_DRIVING_PARAM_3                           0x1438
> > +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT                     0x10
> > +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT                     (0x14 << 8)
> > +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT                     (0x18 << 16)
> > +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT                     (0x20 << 24)
> > +# define DRIVING_PARAM_3_DEFAULT                               (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_4                           0x113C
> > +#define MTK_DP_LANE1_DRIVING_PARAM_4                           0x123C
> > +#define MTK_DP_LANE2_DRIVING_PARAM_4                           0x133C
> > +#define MTK_DP_LANE3_DRIVING_PARAM_4                           0x143C
> > +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT                     0x18
> > +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT                     (0x1e << 8)
> > +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT                     (0x24 << 16)
> > +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT                     (0x20 << 24)
> > +# define DRIVING_PARAM_4_DEFAULT                               (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_5                           0x1140
> > +#define MTK_DP_LANE1_DRIVING_PARAM_5                           0x1240
> > +#define MTK_DP_LANE2_DRIVING_PARAM_5                           0x1340
> > +#define MTK_DP_LANE3_DRIVING_PARAM_5                           0x1440
> > +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT                     0x28
> > +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT                     (0x30 << 8)
> > +# define DRIVING_PARAM_5_DEFAULT                               (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_6                           0x1144
> > +#define MTK_DP_LANE1_DRIVING_PARAM_6                           0x1244
> > +#define MTK_DP_LANE2_DRIVING_PARAM_6                           0x1344
> > +#define MTK_DP_LANE3_DRIVING_PARAM_6                           0x1444
> > +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT                    0x00
> > +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT                    (0x04 << 8)
> > +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT                    (0x08 << 16)
> > +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT                    (0x10 << 24)
> > +# define DRIVING_PARAM_6_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_7                           0x1148
> > +#define MTK_DP_LANE1_DRIVING_PARAM_7                           0x1248
> > +#define MTK_DP_LANE2_DRIVING_PARAM_7                           0x1348
> > +#define MTK_DP_LANE3_DRIVING_PARAM_7                           0x1448
> > +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT                    0x00
> > +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT                    (0x06 << 8)
> > +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT                    (0x0c << 16)
> > +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT                    (0x00 << 24)
> > +# define DRIVING_PARAM_7_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
> > +
> > +#define MTK_DP_LANE0_DRIVING_PARAM_8                           0x114C
> > +#define MTK_DP_LANE1_DRIVING_PARAM_8                           0x124C
> > +#define MTK_DP_LANE2_DRIVING_PARAM_8                           0x134C
> > +#define MTK_DP_LANE3_DRIVING_PARAM_8                           0x144C
> > +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT                    0x08
> > +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT                    (0x00 << 8)
> > +# define DRIVING_PARAM_8_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
> > +                                                                XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
> > +
> > +#endif /*_MTK_DP_REG_H_*/
> > --
> > 2.33.0
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Chun-Kuang Hu]

Hi, Markus:

Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月10日 週五 下午1:36寫道：
>
> Hi Chun-Kuang,
>
> On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:
> > Hi, Markus:
> >
> > Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月7日 週二 上午3:37寫道：
> > >
> > > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> > >
> > > It supports both functional units on the mt8195, the embedded
> > > DisplayPort as well as the external DisplayPort units. It offers
> > > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > > to 4 lanes.
> > >
> > > This driver is based on an initial version by
> > > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> > >
> > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > > ---
> > >
> > > Notes:
> > >     Changes RFC -> v1:
> > >     - Removed unused register definitions.
> > >     - Replaced workqueue with threaded irq.
> > >     - Removed connector code.
> > >     - Move to atomic_* drm functions.
> > >     - General cleanups of the code.
> > >     - Remove unused select GENERIC_PHY.
> > >
> > >  drivers/gpu/drm/mediatek/Kconfig      |    6 +
> > >  drivers/gpu/drm/mediatek/Makefile     |    2 +
> > >  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
> > >  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
> > >  4 files changed, 3469 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> > >
>
> ...
>
> > > +#define TOP_OFFSET             0x2000
> > > +#define ENC0_OFFSET            0x3000
> > > +#define ENC1_OFFSET            0x3200
> > > +#define TRANS_OFFSET           0x3400
> > > +#define AUX_OFFSET             0x3600
> > > +#define SEC_OFFSET             0x4000
>
> ...
>
> > > +
> > > +#define DP_PHY_DIG_PLL_CTL_1                                   0x1014
> > > +# define TPLL_SSC_EN                                           BIT(3)
> >
> > It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~
> > 0x4fff is to control non-phy part. For mipi and hdmi, the phy part is
> > an independent device [1] and the phy driver is independent [2] , so I
> > would like this phy to be an independent device.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14
>
> Thanks for your feedback. I looked into both mipi and hdmi phy drivers
> that you referenced. It looks like both are really separate units in
> their SoCs having their own registerspaces located at a completely
> different range than the units using the phy.
>
> For this displayport driver, the phy registers are listed as part of the
> (e)DP_TX unit in the datasheet. Next to the phy registers all the other
> parts are listed as well in the same overall register ranges (see
> above), e.g. TOP_OFFSET, ENC_OFFSET or SEC_OFFSET. Also I would like to
> avoid splitting it up into a separate unit in the devicetree as the
> datasheet handles it as a single unit (including the phy registers).

OK, according to the datasheet, let it to be a single device.

>
> From a practical perspective there is also not much to these PHY
> registers. The only things that would be done in the driver are:
> - initializing the lane driving parameters with static values
> - setup the bitrate
> - enable/disable SSC
> - do a reset
> Exporting these four used functions over a driver boundary wouldn't help
> clarity I think and the code probably can't be reused by any other
> component anyways.

Use mmsys device [1] as an example. mmsys has both clock control
function and other function including routing function. The main
driver [2] is placed in soc folder, and the clock control part [3] is
separated to clk folder but the clock control part just simply control
clock gating.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.15-rc1#n992
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/mediatek/mtk-mmsys.c?h=v5.15-rc1
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.15-rc1

I think many phy driver could be used only by single device driver, so
we don't need to consider the reusability.

Regards,
Chun-Kuang.

>
> So I personally would prefer keeping it as part of the whole driver
> because of the above mentioned reasons. What do you think?
>
> Thanks,
> Markus
>
> >
> > Regards,
> > Chun-Kuang.
> >
> > > +
> > > +#define DP_PHY_DIG_BIT_RATE                                    0x103C
> > > +# define BIT_RATE_RBR                                          0
> > > +# define BIT_RATE_HBR                                          1
> > > +# define BIT_RATE_HBR2                                         2
> > > +# define BIT_RATE_HBR3                                         3
> > > +
> > > +#define DP_PHY_DIG_SW_RST                                      0x1038
> > > +# define DP_GLB_SW_RST_PHYD                                    BIT(0)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_3                           0x1138
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_3                           0x1238
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_3                           0x1338
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_3                           0x1438
> > > +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT                     0x10
> > > +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT                     (0x14 << 8)
> > > +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT                     (0x18 << 16)
> > > +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT                     (0x20 << 24)
> > > +# define DRIVING_PARAM_3_DEFAULT                               (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_4                           0x113C
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_4                           0x123C
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_4                           0x133C
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_4                           0x143C
> > > +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT                     0x18
> > > +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT                     (0x1e << 8)
> > > +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT                     (0x24 << 16)
> > > +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT                     (0x20 << 24)
> > > +# define DRIVING_PARAM_4_DEFAULT                               (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_5                           0x1140
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_5                           0x1240
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_5                           0x1340
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_5                           0x1440
> > > +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT                     0x28
> > > +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT                     (0x30 << 8)
> > > +# define DRIVING_PARAM_5_DEFAULT                               (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_6                           0x1144
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_6                           0x1244
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_6                           0x1344
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_6                           0x1444
> > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT                    0x00
> > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT                    (0x04 << 8)
> > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT                    (0x08 << 16)
> > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT                    (0x10 << 24)
> > > +# define DRIVING_PARAM_6_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_7                           0x1148
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_7                           0x1248
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_7                           0x1348
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_7                           0x1448
> > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT                    0x00
> > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT                    (0x06 << 8)
> > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT                    (0x0c << 16)
> > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT                    (0x00 << 24)
> > > +# define DRIVING_PARAM_7_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
> > > +
> > > +#define MTK_DP_LANE0_DRIVING_PARAM_8                           0x114C
> > > +#define MTK_DP_LANE1_DRIVING_PARAM_8                           0x124C
> > > +#define MTK_DP_LANE2_DRIVING_PARAM_8                           0x134C
> > > +#define MTK_DP_LANE3_DRIVING_PARAM_8                           0x144C
> > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT                    0x08
> > > +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT                    (0x00 << 8)
> > > +# define DRIVING_PARAM_8_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
> > > +                                                                XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
> > > +
> > > +#endif /*_MTK_DP_REG_H_*/
> > > --
> > > 2.33.0
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Markus Schneider-Pargmann]

Hi Chun-Kuang,

On Tue, Sep 14, 2021 at 07:25:48AM +0800, Chun-Kuang Hu wrote:
> Hi, Markus:
> 
> Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月10日 週五 下午1:36寫道：
> >
> > Hi Chun-Kuang,
> >
> > On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:
> > > Hi, Markus:
> > >
> > > Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月7日 週二 上午3:37寫道：
> > > >
> > > > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> > > >
> > > > It supports both functional units on the mt8195, the embedded
> > > > DisplayPort as well as the external DisplayPort units. It offers
> > > > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > > > to 4 lanes.
> > > >
> > > > This driver is based on an initial version by
> > > > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> > > >
> > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > > > ---
> > > >
> > > > Notes:
> > > >     Changes RFC -> v1:
> > > >     - Removed unused register definitions.
> > > >     - Replaced workqueue with threaded irq.
> > > >     - Removed connector code.
> > > >     - Move to atomic_* drm functions.
> > > >     - General cleanups of the code.
> > > >     - Remove unused select GENERIC_PHY.
> > > >
> > > >  drivers/gpu/drm/mediatek/Kconfig      |    6 +
> > > >  drivers/gpu/drm/mediatek/Makefile     |    2 +
> > > >  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
> > > >  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
> > > >  4 files changed, 3469 insertions(+)
> > > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> > > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> > > >
> >
> > ...
> >
> > > > +#define TOP_OFFSET             0x2000
> > > > +#define ENC0_OFFSET            0x3000
> > > > +#define ENC1_OFFSET            0x3200
> > > > +#define TRANS_OFFSET           0x3400
> > > > +#define AUX_OFFSET             0x3600
> > > > +#define SEC_OFFSET             0x4000
> >
> > ...
> >
> > > > +
> > > > +#define DP_PHY_DIG_PLL_CTL_1                                   0x1014
> > > > +# define TPLL_SSC_EN                                           BIT(3)
> > >
> > > It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~
> > > 0x4fff is to control non-phy part. For mipi and hdmi, the phy part is
> > > an independent device [1] and the phy driver is independent [2] , so I
> > > would like this phy to be an independent device.
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14
> >
> > Thanks for your feedback. I looked into both mipi and hdmi phy drivers
> > that you referenced. It looks like both are really separate units in
> > their SoCs having their own registerspaces located at a completely
> > different range than the units using the phy.
> >
> > For this displayport driver, the phy registers are listed as part of the
> > (e)DP_TX unit in the datasheet. Next to the phy registers all the other
> > parts are listed as well in the same overall register ranges (see
> > above), e.g. TOP_OFFSET, ENC_OFFSET or SEC_OFFSET. Also I would like to
> > avoid splitting it up into a separate unit in the devicetree as the
> > datasheet handles it as a single unit (including the phy registers).
> 
> OK, according to the datasheet, let it to be a single device.
> 
> >
> > From a practical perspective there is also not much to these PHY
> > registers. The only things that would be done in the driver are:
> > - initializing the lane driving parameters with static values
> > - setup the bitrate
> > - enable/disable SSC
> > - do a reset
> > Exporting these four used functions over a driver boundary wouldn't help
> > clarity I think and the code probably can't be reused by any other
> > component anyways.
> 
> Use mmsys device [1] as an example. mmsys has both clock control
> function and other function including routing function. The main
> driver [2] is placed in soc folder, and the clock control part [3] is
> separated to clk folder but the clock control part just simply control
> clock gating.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.15-rc1#n992
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/mediatek/mtk-mmsys.c?h=v5.15-rc1
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.15-rc1
> 
> I think many phy driver could be used only by single device driver, so
> we don't need to consider the reusability.

Thank you, I have split of a display port phy similar to the patches you
cited. I will send the next version soon.

Another short question. This series has a functional dependency on the
rest of the video pipeline, so mostly the patch series vdosys0 and
vdosys1. So I suppose the theoretical order for merging (once this
series is ready for merge) is first vdosys0/1 and dependencies and then
this displayport series, is that correct?

Thanks,
Markus

> 
> Regards,
> Chun-Kuang.
> 
> >
> > So I personally would prefer keeping it as part of the whole driver
> > because of the above mentioned reasons. What do you think?
> >
> > Thanks,
> > Markus
> >
> > >
> > > Regards,
> > > Chun-Kuang.
> > >
> > > > +
> > > > +#define DP_PHY_DIG_BIT_RATE                                    0x103C
> > > > +# define BIT_RATE_RBR                                          0
> > > > +# define BIT_RATE_HBR                                          1
> > > > +# define BIT_RATE_HBR2                                         2
> > > > +# define BIT_RATE_HBR3                                         3
> > > > +
> > > > +#define DP_PHY_DIG_SW_RST                                      0x1038
> > > > +# define DP_GLB_SW_RST_PHYD                                    BIT(0)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_3                           0x1138
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_3                           0x1238
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_3                           0x1338
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_3                           0x1438
> > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT                     0x10
> > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT                     (0x14 << 8)
> > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT                     (0x18 << 16)
> > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT                     (0x20 << 24)
> > > > +# define DRIVING_PARAM_3_DEFAULT                               (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_4                           0x113C
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_4                           0x123C
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_4                           0x133C
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_4                           0x143C
> > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT                     0x18
> > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT                     (0x1e << 8)
> > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT                     (0x24 << 16)
> > > > +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT                     (0x20 << 24)
> > > > +# define DRIVING_PARAM_4_DEFAULT                               (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_5                           0x1140
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_5                           0x1240
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_5                           0x1340
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_5                           0x1440
> > > > +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT                     0x28
> > > > +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT                     (0x30 << 8)
> > > > +# define DRIVING_PARAM_5_DEFAULT                               (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_6                           0x1144
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_6                           0x1244
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_6                           0x1344
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_6                           0x1444
> > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT                    0x00
> > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT                    (0x04 << 8)
> > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT                    (0x08 << 16)
> > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT                    (0x10 << 24)
> > > > +# define DRIVING_PARAM_6_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_7                           0x1148
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_7                           0x1248
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_7                           0x1348
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_7                           0x1448
> > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT                    0x00
> > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT                    (0x06 << 8)
> > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT                    (0x0c << 16)
> > > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT                    (0x00 << 24)
> > > > +# define DRIVING_PARAM_7_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
> > > > +
> > > > +#define MTK_DP_LANE0_DRIVING_PARAM_8                           0x114C
> > > > +#define MTK_DP_LANE1_DRIVING_PARAM_8                           0x124C
> > > > +#define MTK_DP_LANE2_DRIVING_PARAM_8                           0x134C
> > > > +#define MTK_DP_LANE3_DRIVING_PARAM_8                           0x144C
> > > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT                    0x08
> > > > +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT                    (0x00 << 8)
> > > > +# define DRIVING_PARAM_8_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
> > > > +                                                                XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
> > > > +
> > > > +#endif /*_MTK_DP_REG_H_*/
> > > > --
> > > > 2.33.0
> > > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver

[Chun-Kuang Hu]

Hi, Markus:

Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月17日 週五 下午9:33寫道：
>
> Hi Chun-Kuang,
>
> On Tue, Sep 14, 2021 at 07:25:48AM +0800, Chun-Kuang Hu wrote:
> > Hi, Markus:
> >
> > Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月10日 週五 下午1:36寫道：
> > >
> > > Hi Chun-Kuang,
> > >
> > > On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:
> > > > Hi, Markus:
> > > >
> > > > Markus Schneider-Pargmann <msp@baylibre.com> 於 2021年9月7日 週二 上午3:37寫道：
> > > > >
> > > > > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
> > > > >
> > > > > It supports both functional units on the mt8195, the embedded
> > > > > DisplayPort as well as the external DisplayPort units. It offers
> > > > > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > > > > to 4 lanes.
> > > > >
> > > > > This driver is based on an initial version by
> > > > > Jason-JH.Lin <jason-jh.lin@mediatek.com>.
> > > > >
> > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > > > > ---
> > > > >
> > > > > Notes:
> > > > >     Changes RFC -> v1:
> > > > >     - Removed unused register definitions.
> > > > >     - Replaced workqueue with threaded irq.
> > > > >     - Removed connector code.
> > > > >     - Move to atomic_* drm functions.
> > > > >     - General cleanups of the code.
> > > > >     - Remove unused select GENERIC_PHY.
> > > > >
> > > > >  drivers/gpu/drm/mediatek/Kconfig      |    6 +
> > > > >  drivers/gpu/drm/mediatek/Makefile     |    2 +
> > > > >  drivers/gpu/drm/mediatek/mtk_dp.c     | 2881 +++++++++++++++++++++++++
> > > > >  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  580 +++++
> > > > >  4 files changed, 3469 insertions(+)
> > > > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> > > > >  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> > > > >
> > >
> > > ...
> > >
> > > > > +#define TOP_OFFSET             0x2000
> > > > > +#define ENC0_OFFSET            0x3000
> > > > > +#define ENC1_OFFSET            0x3200
> > > > > +#define TRANS_OFFSET           0x3400
> > > > > +#define AUX_OFFSET             0x3600
> > > > > +#define SEC_OFFSET             0x4000
> > >
> > > ...
> > >
> > > > > +
> > > > > +#define DP_PHY_DIG_PLL_CTL_1                                   0x1014
> > > > > +# define TPLL_SSC_EN                                           BIT(3)
> > > >
> > > > It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~
> > > > 0x4fff is to control non-phy part. For mipi and hdmi, the phy part is
> > > > an independent device [1] and the phy driver is independent [2] , so I
> > > > would like this phy to be an independent device.
> > > >
> > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14
> > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14
> > >
> > > Thanks for your feedback. I looked into both mipi and hdmi phy drivers
> > > that you referenced. It looks like both are really separate units in
> > > their SoCs having their own registerspaces located at a completely
> > > different range than the units using the phy.
> > >
> > > For this displayport driver, the phy registers are listed as part of the
> > > (e)DP_TX unit in the datasheet. Next to the phy registers all the other
> > > parts are listed as well in the same overall register ranges (see
> > > above), e.g. TOP_OFFSET, ENC_OFFSET or SEC_OFFSET. Also I would like to
> > > avoid splitting it up into a separate unit in the devicetree as the
> > > datasheet handles it as a single unit (including the phy registers).
> >
> > OK, according to the datasheet, let it to be a single device.
> >
> > >
> > > From a practical perspective there is also not much to these PHY
> > > registers. The only things that would be done in the driver are:
> > > - initializing the lane driving parameters with static values
> > > - setup the bitrate
> > > - enable/disable SSC
> > > - do a reset
> > > Exporting these four used functions over a driver boundary wouldn't help
> > > clarity I think and the code probably can't be reused by any other
> > > component anyways.
> >
> > Use mmsys device [1] as an example. mmsys has both clock control
> > function and other function including routing function. The main
> > driver [2] is placed in soc folder, and the clock control part [3] is
> > separated to clk folder but the clock control part just simply control
> > clock gating.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.15-rc1#n992
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/mediatek/mtk-mmsys.c?h=v5.15-rc1
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.15-rc1
> >
> > I think many phy driver could be used only by single device driver, so
> > we don't need to consider the reusability.
>
> Thank you, I have split of a display port phy similar to the patches you
> cited. I will send the next version soon.
>
> Another short question. This series has a functional dependency on the
> rest of the video pipeline, so mostly the patch series vdosys0 and
> vdosys1. So I suppose the theoretical order for merging (once this
> series is ready for merge) is first vdosys0/1 and dependencies and then
> this displayport series, is that correct?

Sometimes I would apply partial reviewed patches in a series. Even
though these partial patches would not functionally work without other
patches, they are so independent and I would like to apply them to
prevent conflicts in the future. But these patches should build pass
and not break other workable function.

Regards,
Chun-Kuang.

>
> Thanks,
> Markus
>
> >
> > Regards,
> > Chun-Kuang.
> >
> > >
> > > So I personally would prefer keeping it as part of the whole driver
> > > because of the above mentioned reasons. What do you think?
> > >
> > > Thanks,
> > > Markus
> > >
> > > >
> > > > Regards,
> > > > Chun-Kuang.
> > > >
> > > > > +
> > > > > +#define DP_PHY_DIG_BIT_RATE                                    0x103C
> > > > > +# define BIT_RATE_RBR                                          0
> > > > > +# define BIT_RATE_HBR                                          1
> > > > > +# define BIT_RATE_HBR2                                         2
> > > > > +# define BIT_RATE_HBR3                                         3
> > > > > +
> > > > > +#define DP_PHY_DIG_SW_RST                                      0x1038
> > > > > +# define DP_GLB_SW_RST_PHYD                                    BIT(0)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_3                           0x1138
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_3                           0x1238
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_3                           0x1338
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_3                           0x1438
> > > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT                     0x10
> > > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT                     (0x14 << 8)
> > > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT                     (0x18 << 16)
> > > > > +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT                     (0x20 << 24)
> > > > > +# define DRIVING_PARAM_3_DEFAULT                               (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_4                           0x113C
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_4                           0x123C
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_4                           0x133C
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_4                           0x143C
> > > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT                     0x18
> > > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT                     (0x1e << 8)
> > > > > +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT                     (0x24 << 16)
> > > > > +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT                     (0x20 << 24)
> > > > > +# define DRIVING_PARAM_4_DEFAULT                               (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_5                           0x1140
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_5                           0x1240
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_5                           0x1340
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_5                           0x1440
> > > > > +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT                     0x28
> > > > > +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT                     (0x30 << 8)
> > > > > +# define DRIVING_PARAM_5_DEFAULT                               (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_6                           0x1144
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_6                           0x1244
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_6                           0x1344
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_6                           0x1444
> > > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT                    0x00
> > > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT                    (0x04 << 8)
> > > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT                    (0x08 << 16)
> > > > > +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT                    (0x10 << 24)
> > > > > +# define DRIVING_PARAM_6_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_7                           0x1148
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_7                           0x1248
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_7                           0x1348
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_7                           0x1448
> > > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT                    0x00
> > > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT                    (0x06 << 8)
> > > > > +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT                    (0x0c << 16)
> > > > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT                    (0x00 << 24)
> > > > > +# define DRIVING_PARAM_7_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
> > > > > +
> > > > > +#define MTK_DP_LANE0_DRIVING_PARAM_8                           0x114C
> > > > > +#define MTK_DP_LANE1_DRIVING_PARAM_8                           0x124C
> > > > > +#define MTK_DP_LANE2_DRIVING_PARAM_8                           0x134C
> > > > > +#define MTK_DP_LANE3_DRIVING_PARAM_8                           0x144C
> > > > > +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT                    0x08
> > > > > +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT                    (0x00 << 8)
> > > > > +# define DRIVING_PARAM_8_DEFAULT                               (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
> > > > > +                                                                XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
> > > > > +
> > > > > +#endif /*_MTK_DP_REG_H_*/
> > > > > --
> > > > > 2.33.0
> > > > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel