| Message ID | 20200612015030.16072-1-tanmay@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add support for DisplayPort driver on | expand |
On Thu, Jun 11, 2020 at 7:51 PM Tanmay Shah <tanmay@codeaurora.org> wrote: > > From: Chandan Uddaraju <chandanu@codeaurora.org> > > Add bindings for Snapdragon DisplayPort controller driver. > > Changes in V2: > Provide details about sel-gpio > > Changes in V4: > Provide details about max dp lanes > Change the commit text > > Changes in V5: > moved dp.txt to yaml file > > Changes in v6: > - Squash all AUX LUT properties into one pattern Property > - Make aux-cfg[0-9]-settings properties optional > - Remove PLL/PHY bindings from DP controller dts > - Add DP clocks description > - Remove _clk suffix from clock names > - Rename pixel clock to stream_pixel > - Remove redundant bindings (GPIO, PHY, HDCP clock, etc..) > - Fix indentation > - Add Display Port as interface of DPU in DPU bindings > and add port mapping accordingly. > > Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org> > Signed-off-by: Vara Reddy <varar@codeaurora.org> > Signed-off-by: Tanmay Shah <tanmay@codeaurora.org> > --- > .../bindings/display/msm/dp-sc7180.yaml | 142 ++++++++++++++++++ > .../devicetree/bindings/display/msm/dpu.txt | 8 + > 2 files changed, 150 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml Is it too much to ask for Qualcomm to coordinate your work? I'm not going to do that for you. This conflicts with "[v4] dt-bindings: msm: disp: add yaml schemas for DPU and DSI bindings". > diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > new file mode 100644 > index 000000000000..5fdb9153df00 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > @@ -0,0 +1,142 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) Extra space. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Display Port Controller. > + > +maintainers: > + - Chandan Uddaraju <chandanu@codeaurora.org> > + - Vara Reddy <varar@codeaurora.org> > + - Tanmay Shah <tanmay@codeaurora.org> > + > +description: | > + Device tree bindings for MSM Display Port which supports DP host controllers > + that are compatible with VESA Display Port interface specification. > + > +properties: > + compatible: > + items: > + - const: qcom,dp-display That's what the h/w reference manual calls this? It should be SoC specific. > + > + cell-index: > + description: Specifies the controller instance. Pretty sure I already said no to this. > + > + reg: > + items: > + - description: DP controller registers Just: 'maxItems: 1' > + > + interrupts: > + description: The interrupt signal from the DP block. How many? The description is useless. That's every 'interrupts'. > + > + clocks: > + description: List of clock specifiers for clocks needed by the device. That's every 'clocks' property. Drop. > + items: > + - description: Display Port AUX clock > + - description: Display Port Link clock > + - description: Link interface clock between DP and PHY > + - description: Display Port Pixel clock > + - description: Root clock generator for pixel clock > + > + clock-names: > + description: | > + Device clock names in the same order as mentioned in clocks property. > + The required clocks are mentioned below. Drop. > + items: > + - const: core_aux > + - const: ctrl_link > + - const: ctrl_link_iface > + - const: stream_pixel > + - const: pixel_rcg blank line > + "#clock-cells": > + const: 1 > + > + vdda-1p2-supply: > + description: phandle to vdda 1.2V regulator node. > + > + vdda-0p9-supply: > + description: phandle to vdda 0.9V regulator node. > + > + data-lanes = <0 1>: Err, what?! > + type: object This is a DT node? > + description: Maximum number of lanes that can be used for Display port. > + > + ports: > + description: | > + Contains display port controller endpoint subnode. > + remote-endpoint: | > + For port@0, set to phandle of the connected panel/bridge's > + input endpoint. For port@1, set to the DPU interface output. Look at other schemas and see how they are done. > + Documentation/devicetree/bindings/graph.txt and > + Documentation/devicetree/bindings/media/video-interfaces.txt. > + > +patternProperties: > + "^aux-cfg([0-9])-settings$": > + type: object This is a DT node? > + description: | > + Specifies the DP AUX configuration [0-9] settings. > + The first entry in this array corresponds to the register offset > + within DP AUX, while the remaining entries indicate the > + programmable values. > + > +required: > + - compatible > + - cell-index > + - reg > + - interrupts > + - clocks > + - clock-names > + - vdda-1p2-supply > + - vdda-0p9-supply > + - data-lanes > + - ports Add: additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/clock/qcom,dispcc-sdm845.h> > + #include <dt-bindings/clock/qcom,gcc-sdm845.h> > + msm_dp: displayport-controller@ae90000{ > + compatible = "qcom,dp-display"; > + cell-index = <0>; > + reg = <0 0xae90000 0 0x1400>; > + reg-names = "dp_controller"; > + > + interrupt-parent = <&display_subsystem>; > + interrupts = <12 0>; > + > + clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>, > + <&dispcc DISP_CC_MDSS_DP_LINK_CLK>, > + <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>, > + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>, > + <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; > + clock-names = "core_aux", > + "ctrl_link", > + "ctrl_link_iface", "stream_pixel", > + "pixel_rcg"; > + #clock-cells = <1>; > + > + vdda-1p2-supply = <&vreg_l3c_1p2>; > + vdda-0p9-supply = <&vreg_l4a_0p8>; > + > + data-lanes = <0 1>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dp_in: endpoint { > + remote-endpoint = <&dpu_intf0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + dp_out: endpoint { > + }; > + }; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt > index 551ae26f60da..30c8ab479b02 100644 > --- a/Documentation/devicetree/bindings/display/msm/dpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt > @@ -65,6 +65,7 @@ Required properties: > > Port 0 -> DPU_INTF1 (DSI1) > Port 1 -> DPU_INTF2 (DSI2) > + Port 2 -> DPU_INTF0 (DP) > > Optional properties: > - assigned-clocks: list of clock specifiers for clocks needing rate assignment > @@ -136,6 +137,13 @@ Example: > remote-endpoint = <&dsi1_in>; > }; > }; > + > + port@2 { > + reg = <2>; > + dpu_intf0_out: endpoint { > + remote-endpoint = <&dp_in>; > + }; > + }; > }; > }; > }; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On Mon, Jun 15, 2020 at 4:51 PM <tanmay@codeaurora.org> wrote: > > On 2020-06-12 16:26, Stephen Boyd wrote: > > Thanks for reviews Stephen. > > > Quoting Tanmay Shah (2020-06-11 18:50:25) > >> These patches add support for Display-Port driver on SnapDragon > >> hardware. It adds > >> DP driver and DP PLL driver files along with the needed device-tree > >> bindings. > >> > >> The block diagram of DP driver is shown below: > >> > >> > >> +-------------+ > >> |DRM FRAMEWORK| > >> +------+------+ > >> | > >> +----v----+ > >> | DP DRM | > >> +----+----+ > >> | > >> +----v----+ > >> +------------+| DP +----------++------+ > >> + +---+| DISPLAY |+---+ | | > >> | + +-+-----+-+ | | | > >> | | | | | | | > >> | | | | | | | > >> | | | | | | | > >> v v v v v v v > >> +------+ +------+ +---+ +----+ +----+ +---+ +-----+ > >> | DP | | DP | |DP | | DP | | DP | |DP | | DP | > >> |PARSER| | HPD | |AUX| |LINK| |CTRL| |PHY| |POWER| > >> +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+ > >> | | | > >> +--v---+ +v-----v+ > >> |DEVICE| | DP | > >> | TREE | |CATALOG| > >> +------+ +---+---+ > >> | > >> +---v----+ > >> |CTRL/PHY| > >> | HW | > >> +--------+ > >> > > > > I've never seen a block diagram for a driver before... > > > It is here for v5. https://patchwork.freedesktop.org/series/74312/ I think Stephen is nitpicking your wording, and you seem to not be understanding his comment. I'm sorry if I am mistaken. The "DP driver" would seem to refer to the linux software driver you are proposing patches for, however this diagram looks like a hardware diagram of the various hardware blocks that the Linux driver code (the "DP driver") is expected to interact with. I believe you should re-word "The block diagram of DP driver is shown below:" to be more specific of what you are describing with your figure. IE your words say this is a block diagram of the software, when it looks like it is a block diagram of the hardware.
On 2020-06-15 16:04, Jeffrey Hugo wrote: > On Mon, Jun 15, 2020 at 4:51 PM <tanmay@codeaurora.org> wrote: >> >> On 2020-06-12 16:26, Stephen Boyd wrote: >> >> Thanks for reviews Stephen. >> >> > Quoting Tanmay Shah (2020-06-11 18:50:25) >> >> These patches add support for Display-Port driver on SnapDragon >> >> hardware. It adds >> >> DP driver and DP PLL driver files along with the needed device-tree >> >> bindings. >> >> >> >> The block diagram of DP driver is shown below: >> >> >> >> >> >> +-------------+ >> >> |DRM FRAMEWORK| >> >> +------+------+ >> >> | >> >> +----v----+ >> >> | DP DRM | >> >> +----+----+ >> >> | >> >> +----v----+ >> >> +------------+| DP +----------++------+ >> >> + +---+| DISPLAY |+---+ | | >> >> | + +-+-----+-+ | | | >> >> | | | | | | | >> >> | | | | | | | >> >> | | | | | | | >> >> v v v v v v v >> >> +------+ +------+ +---+ +----+ +----+ +---+ +-----+ >> >> | DP | | DP | |DP | | DP | | DP | |DP | | DP | >> >> |PARSER| | HPD | |AUX| |LINK| |CTRL| |PHY| |POWER| >> >> +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+ >> >> | | | >> >> +--v---+ +v-----v+ >> >> |DEVICE| | DP | >> >> | TREE | |CATALOG| >> >> +------+ +---+---+ >> >> | >> >> +---v----+ >> >> |CTRL/PHY| >> >> | HW | >> >> +--------+ >> >> >> > >> > I've never seen a block diagram for a driver before... >> > >> It is here for v5. https://patchwork.freedesktop.org/series/74312/ > > I think Stephen is nitpicking your wording, and you seem to not be > understanding his comment. I'm sorry if I am mistaken. > > The "DP driver" would seem to refer to the linux software driver you > are proposing patches for, however this diagram looks like a hardware > diagram of the various hardware blocks that the Linux driver code (the > "DP driver") is expected to interact with. I believe you should > re-word "The block diagram of DP driver is shown below:" to be more > specific of what you are describing with your figure. IE your words > say this is a block diagram of the software, when it looks like it is > a block diagram of the hardware. Thanks for reviews. I am not sure what Stephen meant, but this diagram was available before. Just for clarification this is not hardware diagram at all. This is modeling of DP driver for msm. Each box name above except "DRM framework", is file name in driver i.e. software module. Each line and arrow shows how modules interact with each other. For example, "DP PARSER" Box is pointing towards "DEVICE TREE" Box, that means dp_parser.c file contains functions which are parsing device tree properties and so on...
On Tue, Jun 16, 2020 at 5:15 AM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Tanmay Shah (2020-06-11 18:50:26) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > > new file mode 100644 > > index 000000000000..5fdb9153df00 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml > > @@ -0,0 +1,142 @@ > > + data-lanes = <0 1>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + dp_in: endpoint { > > + remote-endpoint = <&dpu_intf0_out>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + dp_out: endpoint { > > Just curious what is eventually connected here? This is possibly a > question for Rob Herring, but I can't figure out how we're supposed to > connect this to the USB type-c connector that is receiving the DP > signal. Does the type-c connector binding support connecting to this end > of the graph? Or should this connect to the DP phy and then the phy > connects to the USB type-c connector node? Right now it is empty which > seems wrong. It should connect to the Type-C connector perhaps thru some sort of switching/muxing node, but that's not really flushed out though. See 'dt-bindings: chrome: Add cros-ec-typec mux props' discussion with your CrOS colleagues. Rob
These patches add support for Display-Port driver on SnapDragon hardware. It adds DP driver and DP PLL driver files along with the needed device-tree bindings. The block diagram of DP driver is shown below: +-------------+ |DRM FRAMEWORK| +------+------+ | +----v----+ | DP DRM | +----+----+ | +----v----+ +------------+| DP +----------++------+ + +---+| DISPLAY |+---+ | | | + +-+-----+-+ | | | | | | | | | | | | | | | | | | | | | | | | v v v v v v v +------+ +------+ +---+ +----+ +----+ +---+ +-----+ | DP | | DP | |DP | | DP | | DP | |DP | | DP | |PARSER| | HPD | |AUX| |LINK| |CTRL| |PHY| |POWER| +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+ | | | +--v---+ +v-----v+ |DEVICE| | DP | | TREE | |CATALOG| +------+ +---+---+ | +---v----+ |CTRL/PHY| | HW | +--------+ These patches have dependency on clock driver changes mentioned below: https://patchwork.kernel.org/patch/11245895/ https://patchwork.kernel.org/cover/11069083/ Chandan Uddaraju (4): dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon drm: add constant N value in helper file drm/msm/dp: add displayPort driver support drm/msm/dp: add support for DP PLL driver Jeykumar Sankaran (1): drm/msm/dpu: add display port support in DPU .../bindings/display/msm/dp-sc7180.yaml | 142 ++ .../devicetree/bindings/display/msm/dpu.txt | 8 + drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/msm/Kconfig | 21 + drivers/gpu/drm/msm/Makefile | 15 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 65 +- drivers/gpu/drm/msm/dp/dp_aux.c | 530 +++++ drivers/gpu/drm/msm/dp/dp_aux.h | 35 + drivers/gpu/drm/msm/dp/dp_catalog.c | 1025 ++++++++++ drivers/gpu/drm/msm/dp/dp_catalog.h | 86 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 1709 +++++++++++++++++ drivers/gpu/drm/msm/dp/dp_ctrl.h | 35 + drivers/gpu/drm/msm/dp/dp_display.c | 912 +++++++++ drivers/gpu/drm/msm/dp/dp_display.h | 31 + drivers/gpu/drm/msm/dp/dp_drm.c | 170 ++ drivers/gpu/drm/msm/dp/dp_drm.h | 18 + drivers/gpu/drm/msm/dp/dp_hpd.c | 69 + drivers/gpu/drm/msm/dp/dp_hpd.h | 79 + drivers/gpu/drm/msm/dp/dp_link.c | 1216 ++++++++++++ drivers/gpu/drm/msm/dp/dp_link.h | 132 ++ drivers/gpu/drm/msm/dp/dp_panel.c | 490 +++++ drivers/gpu/drm/msm/dp/dp_panel.h | 95 + drivers/gpu/drm/msm/dp/dp_parser.c | 390 ++++ drivers/gpu/drm/msm/dp/dp_parser.h | 204 ++ drivers/gpu/drm/msm/dp/dp_pll.c | 93 + drivers/gpu/drm/msm/dp/dp_pll.h | 59 + drivers/gpu/drm/msm/dp/dp_pll_10nm.c | 903 +++++++++ drivers/gpu/drm/msm/dp/dp_pll_private.h | 103 + drivers/gpu/drm/msm/dp/dp_power.c | 422 ++++ drivers/gpu/drm/msm/dp/dp_power.h | 115 ++ drivers/gpu/drm/msm/dp/dp_reg.h | 505 +++++ drivers/gpu/drm/msm/msm_drv.c | 2 + drivers/gpu/drm/msm/msm_drv.h | 53 +- include/drm/drm_dp_helper.h | 1 + 36 files changed, 9753 insertions(+), 19 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.h create mode 100644 drivers/gpu/drm/msm/dp/dp_hpd.c create mode 100644 drivers/gpu/drm/msm/dp/dp_hpd.h create mode 100644 drivers/gpu/drm/msm/dp/dp_link.c create mode 100644 drivers/gpu/drm/msm/dp/dp_link.h create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.c create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.h create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.c create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.h create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.c create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.h create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_10nm.c create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_private.h create mode 100644 drivers/gpu/drm/msm/dp/dp_power.c create mode 100644 drivers/gpu/drm/msm/dp/dp_power.h create mode 100644 drivers/gpu/drm/msm/dp/dp_reg.h base-commit: 48f99181fc118d82dc8bf6c7221ad1c654cb8bc2