Re: [PATCH v9 1/5] dt-bindings: display: Add support for Intel KeemBay Display

On Fri, Oct 9, 2020 at 11:09 AM Neil Armstrong narmstrong@baylibre.com wrote:

Hi,

On 09/10/2020 03:03, Anitha Chrisanthus wrote:

...

This patch adds bindings for Intel KeemBay Display

v2: review changes from Rob Herring

Signed-off-by: Anitha Chrisanthus anitha.chrisanthus@intel.com

.../bindings/display/intel,keembay-display.yaml | 99 ++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/intel,keembay-display.yaml

diff --git a/Documentation/devicetree/bindings/display/intel,keembay-display.yaml b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml new file mode 100644 index 0000000..a38493d --- /dev/null +++ b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/intel,keembay-display.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#

+title: Devicetree bindings for Intel Keem Bay display controller

+maintainers:

• • Anitha Chrisanthus anitha.chrisanthus@intel.com
• • Edmond J Dea edmund.j.dea@intel.com

+properties:

• compatible:
• const: intel,kmb_display
• reg:
• items:

•  - description: Lcd registers range
  

•  - description: Mipi registers range
  

Looking at the registers, the MIPI transceiver seems to be a separate IP, same for D-PHY which should have a proper PHY driver instead of beeing handled here.

...

•  - description: Msscam registers range
  

MSScam here seems to be a clock and reset controller for the LCD and MIPI IPs, thus should be handler out of DRM.

...

• reg-names:
• items:

•  - const: lcd
  

•  - const: mipi
  

•  - const: msscam
  

• clocks:
• items:

•  - description: LCD controller clock
  

•  - description: Mipi DSI clock
  

•  - description: Mipi DSI econfig clock
  

•  - description: Mipi DSI config clock
  

•  - description: System clock or pll0 clock
  

• clock-names:
• items:

•  - const: clk_lcd
  

•  - const: clk_mipi
  

•  - const: clk_mipi_ecfg
  

•  - const: clk_mipi_cfg
  

•  - const: clk_pll0
  

• interrupts:
• maxItems: 1
• encoder-slave:
• description: bridge node entry for mipi to hdmi converter
• port:
• type: object
• description: >

•      Port node with one endpoint connected to mipi to hdmi converter node.
  

+required:

• • compatible
• • reg
• • reg-names
• • clocks
• • clock-names
• • interrupts
• • encoder-slave
• • port

+additionalProperties: false

+examples:

• • |
• #include <dt-bindings/interrupt-controller/irq.h>
• #include <dt-bindings/interrupt-controller/arm-gic.h>
• #define MOVISOC_KMB_MSS_AUX_LCD
• #define MOVISOC_KMB_MSS_AUX_MIPI_TX0
• #define MOVISOC_KMB_MSS_AUX_MIPI_ECFG
• #define MOVISOC_KMB_MSS_AUX_MIPI_CFG
• #define MOVISOC_KMB_A53_PLL_0_OUT_0
• display@20900000 {

•  compatible = "intel,keembay-display";
  

•  reg = <0x20930000 0x3000>,
  

•        <0x20900000 0x4000>,
  

•        <0x20910000 0x30>;
  

•  reg-names = "lcd", "mipi", "msscam";
  

•  interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
  

•  clocks = <&scmi_clk MOVISOC_KMB_MSS_AUX_LCD>,
  

•           <&scmi_clk MOVISOC_KMB_MSS_AUX_MIPI_TX0>,
  

•           <&scmi_clk MOVISOC_KMB_MSS_AUX_MIPI_ECFG>,
  

•           <&scmi_clk MOVISOC_KMB_MSS_AUX_MIPI_CFG>,
  

•           <&scmi_clk MOVISOC_KMB_A53_PLL_0_OUT_0>;
  

•  clock-names = "clk_lcd", "clk_mipi", "clk_mipi_ecfg",
  

•                "clk_mipi_cfg", "clk_pll0";
  

•  encoder-slave = <&adv7535>;
  

•  port {
  

•        dsi_output: endpoint {
  

•            remote-endpoint = <&adv7535_input>;
  

•        };
  

•  };
  

• };

Anitha, Daniel, this keembay driver should be architectured like other ARM-like display controllers, with separate drivers for MIPI DSI bridge and msscam clock & reset controller.

tbh I have no clue about dt, and very little clue about how to do armsoc drivers properly. My only take in this entire area is that drm/bridges looks confusing with too many different approaches, and people should stuck more together and figure out what to do (the entire component.c vs not so much component.c discussion, and another one is about creating drm-connector or not doing that).

But how this all should be done I really have not much useful clue, so please keep me out of that. -Daniel