[EXT] Re: [PATCH v6 2/8] dt-bindings: display: bridge: Add Cadence MHDP8501 HDMI and DP

Sandor Yu sandor.yu at nxp.com
Tue Jun 20 19:23:16 PDT 2023


Hi Rob,

Thanks for your comments,

> -----Original Message-----
> From: Rob Herring <robh at kernel.org>
> Sent: 2023年6月20日 23:49
> To: Sandor Yu <sandor.yu at nxp.com>
> Cc: andrzej.hajda at intel.com; neil.armstrong at linaro.org;
> robert.foss at linaro.org; Laurent.pinchart at ideasonboard.com;
> jonas at kwiboo.se; jernej.skrabec at gmail.com; airlied at gmail.com;
> daniel at ffwll.ch; krzysztof.kozlowski+dt at linaro.org; shawnguo at kernel.org;
> s.hauer at pengutronix.de; festevam at gmail.com; vkoul at kernel.org;
> dri-devel at lists.freedesktop.org; devicetree at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> linux-phy at lists.infradead.org; kernel at pengutronix.de; dl-linux-imx
> <linux-imx at nxp.com>; Oliver Brown <oliver.brown at nxp.com>
> Subject: [EXT] Re: [PATCH v6 2/8] dt-bindings: display: bridge: Add Cadence
> MHDP8501 HDMI and DP
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On Thu, Jun 15, 2023 at 09:38:12AM +0800, Sandor Yu wrote:
> > Add bindings for Cadence MHDP8501 DisplayPort and HDMI driver.
> 
> Bindings are for h/w, not a driver.
OK, I will change it in the next version.
> 
> >
> > Signed-off-by: Sandor Yu <Sandor.yu at nxp.com>
> > ---
> >  .../display/bridge/cdns,mhdp8501.yaml         | 105
> ++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
> > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
> > new file mode 100644
> > index 000000000000..a54756815e6f
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.y
> > +++ aml
> > @@ -0,0 +1,105 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fcdns%2Cmhdp8501.yaml%
> 23&dat
> >
> +a=05%7C01%7CSandor.yu%40nxp.com%7C4d4e118d60d744b5dba708db71
> a5de79%7C
> >
> +686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63822872943965530
> 2%7CUnkno
> >
> +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> haWwi
> >
> +LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=UEsMdkZBmfD7tM1wzJ71
> DHQoi4zVOkpT
> > +A9TNE7Rxn%2B8%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CSandor.
> yu%40n
> >
> +xp.com%7C4d4e118d60d744b5dba708db71a5de79%7C686ea1d3bc2b4c6fa
> 92cd99c5
> >
> +c301635%7C0%7C0%7C638228729439655302%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoi
> >
> +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000%7C%
> >
> +7C%7C&sdata=Zu3v0yG2BXWXvTWV5oLiGvdu3O3PhK%2FrYNJIS2zHwpI%3
> D&reserved
> > +=0
> > +
> > +title: Cadence MHDP8501 Displayport bridge
> > +
> > +maintainers:
> > +  - Sandor Yu <Sandor.yu at nxp.com>
> > +
> > +description:
> > +  The Cadence MHDP8501 Displayport/HDMI TX interface.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - cdns,mhdp8501-dp
> > +      - cdns,mhdp8501-hdmi
> > +      - fsl,imx8mq-mhdp8501-dp
> > +      - fsl,imx8mq-mhdp8501-hdmi
> 
> Is DP vs. HDMI fixed for a particular SoC implementation or it's a board level
> decision. In the latter case, the type of connector should determine the mode,
> not compatible.
DP or HDMI is bord level decision. 
Because DP and HDMI have different initialize process and less functions could be reuse, so they have different drivers.
Please check it in patch
[PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP driver
[PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 HDMI driver

If use the type of connector to determine the mode, hdmi and DP driver have to combine into one driver.
So the compatible may the better choice.
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: MHDP8501 DP/HDMI APB clock.
> 
> Seems odd there's no clock tied to the pixel/serdes clock.
MHDP8501 for i.MX8MQ use the pixel clock from PHY PLL not from external CCM.
The pixel clock will be set in function phy_configure

B.R
Sandor
> 
> > +
> > +  phys:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Hotplug cable plugin.
> > +      - description: Hotplug cable plugout.
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: plug_in
> > +      - const: plug_out
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port at 0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Input port from display controller output.
> > +      port at 1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Output port to DP/HDMI connector.
> > +
> > +    required:
> > +      - port at 0
> > +      - port at 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - interrupts
> > +  - interrupt-names
> > +  - phys
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx8mq-clock.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    mhdp_dp: dp-bridge at 32c00000 {
> > +        compatible = "fsl,imx8mq-mhdp8501-dp";
> > +        reg = <0x32c00000 0x100000>;
> > +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > +                     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > +        interrupt-names = "plug_in", "plug_out";
> > +        clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>;
> > +        phys = <&dp_phy>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port at 0 {
> > +                reg = <0>;
> > +
> > +                mhdp_in: endpoint {
> > +                    remote-endpoint = <&dcss_out>;
> > +                };
> > +            };
> > +
> > +            port at 1 {
> > +                reg = <1>;
> > +
> > +                mhdp_out: endpoint {
> > +                    remote-endpoint = <&dp_con>;
> > +                };
> > +            };
> > +        };
> > +    };
> > --
> > 2.34.1
> >


More information about the linux-arm-kernel mailing list