The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI glue on the same Amlogic SoCs.
This adds support for the glue managing the transceiver, mimicing the init flow provided by Amlogic to setup the ENCL encoder, the glue, the transceiver, the digital D-PHY and the Analog PHY in the proper way.
The DW-MIPI-DSI transceiver + D-PHY are clocked by the GP0 PLL, and the ENCL encoder + VIU pixel reader by the VCLK2 clock using the HDMI PLL.
The DW-MIPI-DSI transceiver gets this pixel stream as input clocked with the VCLK2 clock.
An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the DW-MIPI-DSI transceiver.
This patchset is based on an earlier attempt at [1] for the AXG SoCs, but: - the AXG has a single clock source for both transceiver + pixel, which makes it an exception instead of a rule, it's simpler to add support for G12A then add AXG on it - previous glue code was a single monolitic code mixing encoders & bridges, this version is aligned on the previous cleanup done on HDMI & CVBS bridges architecture at [2] - since the only output of AXG is DSI, AXG VPU support is post-poned until we implement single-clock DSI support specific case on top of this.
[1] https://lore.kernel.org/r/20200907081825.1654-1-narmstrong@baylibre.com [2] https://lore.kernel.org/r/20211020123947.2585572-1-narmstrong@baylibre.com
Neil Armstrong (6): dt-bindings: display: add Amlogic MIPI DSI Host Controller bindings dt-bindings: display: meson-vpu: add third DPI output port drm/meson: venc: add ENCL encoder setup for MIPI-DSI output drm/meson: vclk: add DSI clock config drm/meson: add DSI encoder drm/meson: add support for MIPI-DSI transceiver
.../display/amlogic,meson-dw-mipi-dsi.yaml | 118 ++++++ .../bindings/display/amlogic,meson-vpu.yaml | 5 + drivers/gpu/drm/meson/Kconfig | 7 + drivers/gpu/drm/meson/Makefile | 3 +- drivers/gpu/drm/meson/meson_drv.c | 7 + drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 383 ++++++++++++++++++ drivers/gpu/drm/meson/meson_dw_mipi_dsi.h | 115 ++++++ drivers/gpu/drm/meson/meson_encoder_dsi.c | 159 ++++++++ drivers/gpu/drm/meson/meson_encoder_dsi.h | 12 + drivers/gpu/drm/meson/meson_vclk.c | 47 +++ drivers/gpu/drm/meson/meson_vclk.h | 1 + drivers/gpu/drm/meson/meson_venc.c | 230 ++++++++++- drivers/gpu/drm/meson/meson_venc.h | 6 + drivers/gpu/drm/meson/meson_vpp.h | 2 + 14 files changed, 1092 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.h create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.h
The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI Glue on the same Amlogic SoCs.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- .../display/amlogic,meson-dw-mipi-dsi.yaml | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml new file mode 100644 index 000000000000..f3070783d606 --- /dev/null +++ b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml @@ -0,0 +1,118 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2020 BayLibre, SAS +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/display/amlogic,meson-dw-mipi-dsi.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Amlogic specific extensions to the Synopsys Designware MIPI DSI Host Controller + +maintainers: + - Neil Armstrong narmstrong@baylibre.com + +description: | + The Amlogic Meson Synopsys Designware Integration is composed of + - A Synopsys DesignWare MIPI DSI Host Controller IP + - A TOP control block controlling the Clocks & Resets of the IP + +allOf: + - $ref: dsi-controller.yaml# + +properties: + compatible: + enum: + - amlogic,meson-g12a-dw-mipi-dsi + + reg: + maxItems: 1 + + clocks: + minItems: 2 + + clock-names: + minItems: 2 + items: + - const: pclk + - const: px_clk + - const: meas_clk + + resets: + minItems: 1 + + reset-names: + items: + - const: top + + phys: + minItems: 1 + + phy-names: + items: + - const: dphy + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: Input node to receive pixel data. + + port@1: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: DSI output node to panel. + + required: + - port@0 + - port@1 + +required: + - compatible + - reg + - clocks + - clock-names + - resets + - reset-names + - phys + - phy-names + - ports + +unevaluatedProperties: false + +examples: + - | + dsi@7000 { + compatible = "amlogic,meson-g12a-dw-mipi-dsi"; + reg = <0x6000 0x400>; + resets = <&reset_top>; + reset-names = "top"; + clocks = <&clk_pclk>, <&clk_px>; + clock-names = "pclk", "px_clk"; + phys = <&mipi_dphy>; + phy-names = "dphy"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + /* VPU VENC Input */ + mipi_dsi_venc_port: port@0 { + reg = <0>; + + mipi_dsi_in: endpoint { + remote-endpoint = <&dpi_out>; + }; + }; + + /* DSI Output */ + mipi_dsi_panel_port: port@1 { + reg = <1>; + + mipi_out_panel: endpoint { + remote-endpoint = <&mipi_in_panel>; + }; + }; + }; + };
On Fri, Jan 07, 2022 at 03:55:10PM +0100, Neil Armstrong wrote:
The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI Glue on the same Amlogic SoCs.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
.../display/amlogic,meson-dw-mipi-dsi.yaml | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml new file mode 100644 index 000000000000..f3070783d606 --- /dev/null +++ b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml @@ -0,0 +1,118 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2020 BayLibre, SAS +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/display/amlogic,meson-dw-mipi-dsi.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+title: Amlogic specific extensions to the Synopsys Designware MIPI DSI Host Controller
+maintainers:
- Neil Armstrong narmstrong@baylibre.com
+description: |
- The Amlogic Meson Synopsys Designware Integration is composed of
- A Synopsys DesignWare MIPI DSI Host Controller IP
- A TOP control block controlling the Clocks & Resets of the IP
+allOf:
- $ref: dsi-controller.yaml#
+properties:
- compatible:
- enum:
- amlogic,meson-g12a-dw-mipi-dsi- reg:
- maxItems: 1
- clocks:
- minItems: 2
- clock-names:
- minItems: 2
- items:
- const: pclk- const: px_clk- const: meas_clk- resets:
- minItems: 1
- reset-names:
- items:
- const: top- phys:
- minItems: 1
- phy-names:
- items:
- const: dphy- ports:
- $ref: /schemas/graph.yaml#/properties/ports
- properties:
port@0:$ref: /schemas/graph.yaml#/$defs/port-base
/schemas/graph.yaml#/properties/port
unevaluatedProperties: false
And this can be dropped.
description: Input node to receive pixel data.port@1:$ref: /schemas/graph.yaml#/$defs/port-baseunevaluatedProperties: false
Same here.
With that,
Reviewed-by: Rob Herring robh@kernel.org
description: DSI output node to panel.- required:
- port@0- port@1+required:
- compatible
- reg
- clocks
- clock-names
- resets
- reset-names
- phys
- phy-names
- ports
+unevaluatedProperties: false
+examples:
- |
- dsi@7000 {
compatible = "amlogic,meson-g12a-dw-mipi-dsi";reg = <0x6000 0x400>;resets = <&reset_top>;reset-names = "top";clocks = <&clk_pclk>, <&clk_px>;clock-names = "pclk", "px_clk";phys = <&mipi_dphy>;phy-names = "dphy";ports {#address-cells = <1>;#size-cells = <0>;/* VPU VENC Input */mipi_dsi_venc_port: port@0 {reg = <0>;mipi_dsi_in: endpoint {remote-endpoint = <&dpi_out>;};};/* DSI Output */mipi_dsi_panel_port: port@1 {reg = <1>;mipi_out_panel: endpoint {remote-endpoint = <&mipi_in_panel>;};};};- };
-- 2.25.1
Add third port corresponding to the ENCL DPI encoder used to connect to DSI or LVDS transceivers.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- .../devicetree/bindings/display/amlogic,meson-vpu.yaml | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml index 851cb0781217..525a01a38568 100644 --- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml +++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml @@ -92,6 +92,11 @@ properties: description: A port node pointing to the HDMI-TX port node.
+ port@2: + $ref: /schemas/graph.yaml#/properties/port + description: + A port node pointing to the DPI port node (e.g. DSI or LVDS transceiver). + "#address-cells": const: 1
On Fri, Jan 7, 2022 at 3:56 PM Neil Armstrong narmstrong@baylibre.com wrote:
Add third port corresponding to the ENCL DPI encoder used to connect to DSI or LVDS transceivers.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
Reviewed-by: Martin Blumenstingl martin.blumenstingl@googlemail.com
On Fri, 07 Jan 2022 15:55:11 +0100, Neil Armstrong wrote:
Add third port corresponding to the ENCL DPI encoder used to connect to DSI or LVDS transceivers.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
.../devicetree/bindings/display/amlogic,meson-vpu.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Rob Herring robh@kernel.org
This adds supports for the ENCL encoder connected to a MIPI-DSI transceiver on the Amlogic AXG SoCs.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/gpu/drm/meson/meson_venc.c | 230 ++++++++++++++++++++++++++++- drivers/gpu/drm/meson/meson_venc.h | 6 + drivers/gpu/drm/meson/meson_vpp.h | 2 + 3 files changed, 236 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c index 3c55ed003359..b430dc06aa34 100644 --- a/drivers/gpu/drm/meson/meson_venc.c +++ b/drivers/gpu/drm/meson/meson_venc.c @@ -6,6 +6,7 @@ */
#include <linux/export.h> +#include <linux/iopoll.h>
#include <drm/drm_modes.h>
@@ -1557,6 +1558,224 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic, } EXPORT_SYMBOL_GPL(meson_venc_hdmi_mode_set);
+static unsigned short meson_encl_gamma_table[256] = { + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56, 60, + 64, 68, 72, 76, 80, 84, 88, 92, 96, 100, 104, 108, 112, 116, 120, 124, + 128, 132, 136, 140, 144, 148, 152, 156, 160, 164, 168, 172, 176, 180, 184, 188, + 192, 196, 200, 204, 208, 212, 216, 220, 224, 228, 232, 236, 240, 244, 248, 252, + 256, 260, 264, 268, 272, 276, 280, 284, 288, 292, 296, 300, 304, 308, 312, 316, + 320, 324, 328, 332, 336, 340, 344, 348, 352, 356, 360, 364, 368, 372, 376, 380, + 384, 388, 392, 396, 400, 404, 408, 412, 416, 420, 424, 428, 432, 436, 440, 444, + 448, 452, 456, 460, 464, 468, 472, 476, 480, 484, 488, 492, 496, 500, 504, 508, + 512, 516, 520, 524, 528, 532, 536, 540, 544, 548, 552, 556, 560, 564, 568, 572, + 576, 580, 584, 588, 592, 596, 600, 604, 608, 612, 616, 620, 624, 628, 632, 636, + 640, 644, 648, 652, 656, 660, 664, 668, 672, 676, 680, 684, 688, 692, 696, 700, + 704, 708, 712, 716, 720, 724, 728, 732, 736, 740, 744, 748, 752, 756, 760, 764, + 768, 772, 776, 780, 784, 788, 792, 796, 800, 804, 808, 812, 816, 820, 824, 828, + 832, 836, 840, 844, 848, 852, 856, 860, 864, 868, 872, 876, 880, 884, 888, 892, + 896, 900, 904, 908, 912, 916, 920, 924, 928, 932, 936, 940, 944, 948, 952, 956, + 960, 964, 968, 972, 976, 980, 984, 988, 992, 996, 1000, 1004, 1008, 1012, 1016, 1020, +}; + +#define GAMMA_VCOM_POL 7 /* RW */ +#define GAMMA_RVS_OUT 6 /* RW */ +#define ADR_RDY 5 /* Read Only */ +#define WR_RDY 4 /* Read Only */ +#define RD_RDY 3 /* Read Only */ +#define GAMMA_TR 2 /* RW */ +#define GAMMA_SET 1 /* RW */ +#define GAMMA_EN 0 /* RW */ + +#define H_RD 12 +#define H_AUTO_INC 11 +#define H_SEL_R 10 +#define H_SEL_G 9 +#define H_SEL_B 8 +#define HADR_MSB 7 /* 7:0 */ +#define HADR 0 /* 7:0 */ + +#define GAMMA_RETRY 1000 + +static void meson_encl_set_gamma_table(struct meson_drm *priv, u16 *data, + u32 rgb_mask) +{ + int i, ret; + u32 reg; + + writel_bits_relaxed(BIT(GAMMA_EN), 0, + priv->io_base + _REG(L_GAMMA_CNTL_PORT)); + + ret = readl_relaxed_poll_timeout(priv->io_base + + _REG(L_GAMMA_CNTL_PORT), + reg, reg & BIT(ADR_RDY), 10, 10000); + if (ret) + pr_warn("%s: GAMMA ADR_RDY timeout\n", __func__); + + writel_relaxed(BIT(H_AUTO_INC) | + BIT(rgb_mask) | + (0 << HADR), + priv->io_base + _REG(L_GAMMA_ADDR_PORT)); + + for (i = 0; i < 256; i++) { + ret = readl_relaxed_poll_timeout(priv->io_base + + _REG(L_GAMMA_CNTL_PORT), + reg, reg & BIT(WR_RDY), + 10, 10000); + if (ret) + pr_warn_once("%s: GAMMA WR_RDY timeout\n", __func__); + + writel_relaxed(data[i], + priv->io_base + _REG(L_GAMMA_DATA_PORT)); + } + + ret = readl_relaxed_poll_timeout(priv->io_base + + _REG(L_GAMMA_CNTL_PORT), + reg, reg & BIT(ADR_RDY), 10, 10000); + if (ret) + pr_warn("%s: GAMMA ADR_RDY timeout\n", __func__); + + writel_relaxed(BIT(H_AUTO_INC) | + BIT(rgb_mask) | + (0x23 << HADR), + priv->io_base + _REG(L_GAMMA_ADDR_PORT)); +} + +void meson_encl_load_gamma(struct meson_drm *priv) +{ + meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_R); + meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_G); + meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_B); + + writel_bits_relaxed(BIT(GAMMA_EN), BIT(GAMMA_EN), + priv->io_base + _REG(L_GAMMA_CNTL_PORT)); +} + +void meson_venc_mipi_dsi_mode_set(struct meson_drm *priv, + const struct drm_display_mode *mode) +{ + unsigned int max_pxcnt; + unsigned int max_lncnt; + unsigned int havon_begin; + unsigned int havon_end; + unsigned int vavon_bline; + unsigned int vavon_eline; + unsigned int hso_begin; + unsigned int hso_end; + unsigned int vso_begin; + unsigned int vso_end; + unsigned int vso_bline; + unsigned int vso_eline; + + max_pxcnt = mode->htotal - 1; + max_lncnt = mode->vtotal - 1; + havon_begin = mode->htotal - mode->hsync_start; + havon_end = havon_begin + mode->hdisplay - 1; + vavon_bline = mode->vtotal - mode->vsync_start; + vavon_eline = vavon_bline + mode->vdisplay - 1; + hso_begin = 0; + hso_end = mode->hsync_end - mode->hsync_start; + vso_begin = 0; + vso_end = 0; + vso_bline = 0; + vso_eline = mode->vsync_end - mode->vsync_start; + + meson_vpp_setup_mux(priv, MESON_VIU_VPP_MUX_ENCL); + + writel_relaxed(0, priv->io_base + _REG(ENCL_VIDEO_EN)); + + writel_relaxed(0x8000, priv->io_base + _REG(ENCL_VIDEO_MODE)); + writel_relaxed(0x0418, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV)); + + writel_relaxed(0x1000, priv->io_base + _REG(ENCL_VIDEO_FILT_CTRL)); + writel_relaxed(max_pxcnt, priv->io_base + _REG(ENCL_VIDEO_MAX_PXCNT)); + writel_relaxed(max_lncnt, priv->io_base + _REG(ENCL_VIDEO_MAX_LNCNT)); + writel_relaxed(havon_begin, priv->io_base + _REG(ENCL_VIDEO_HAVON_BEGIN)); + writel_relaxed(havon_end, priv->io_base + _REG(ENCL_VIDEO_HAVON_END)); + writel_relaxed(vavon_bline, priv->io_base + _REG(ENCL_VIDEO_VAVON_BLINE)); + writel_relaxed(vavon_eline, priv->io_base + _REG(ENCL_VIDEO_VAVON_ELINE)); + + writel_relaxed(hso_begin, priv->io_base + _REG(ENCL_VIDEO_HSO_BEGIN)); + writel_relaxed(hso_end, priv->io_base + _REG(ENCL_VIDEO_HSO_END)); + writel_relaxed(vso_begin, priv->io_base + _REG(ENCL_VIDEO_VSO_BEGIN)); + writel_relaxed(vso_end, priv->io_base + _REG(ENCL_VIDEO_VSO_END)); + writel_relaxed(vso_bline, priv->io_base + _REG(ENCL_VIDEO_VSO_BLINE)); + writel_relaxed(vso_eline, priv->io_base + _REG(ENCL_VIDEO_VSO_ELINE)); + writel_relaxed(3, priv->io_base + _REG(ENCL_VIDEO_RGBIN_CTRL)); + + /* default black pattern */ + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_MDSEL)); + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_Y)); + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CB)); + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CR)); + writel_relaxed(1, priv->io_base + _REG(ENCL_TST_EN)); + writel_bits_relaxed(BIT(3), 0, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV)); + + writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN)); + + writel_relaxed(0, priv->io_base + _REG(L_RGB_BASE_ADDR)); + writel_relaxed(0x400, priv->io_base + _REG(L_RGB_COEFF_ADDR)); + writel_relaxed(0x400, priv->io_base + _REG(L_DITH_CNTL_ADDR)); + + /* DE signal for TTL */ + writel_relaxed(havon_begin, priv->io_base + _REG(L_OEH_HS_ADDR)); + writel_relaxed(havon_end + 1, priv->io_base + _REG(L_OEH_HE_ADDR)); + writel_relaxed(vavon_bline, priv->io_base + _REG(L_OEH_VS_ADDR)); + writel_relaxed(vavon_eline, priv->io_base + _REG(L_OEH_VE_ADDR)); + + /* DE signal for TTL */ + writel_relaxed(havon_begin, priv->io_base + _REG(L_OEV1_HS_ADDR)); + writel_relaxed(havon_end + 1, priv->io_base + _REG(L_OEV1_HE_ADDR)); + writel_relaxed(vavon_bline, priv->io_base + _REG(L_OEV1_VS_ADDR)); + writel_relaxed(vavon_eline, priv->io_base + _REG(L_OEV1_VE_ADDR)); + + /* Hsync signal for TTL */ + if (mode->flags & DRM_MODE_FLAG_PHSYNC) { + writel_relaxed(hso_end, priv->io_base + _REG(L_STH1_HS_ADDR)); + writel_relaxed(hso_begin, priv->io_base + _REG(L_STH1_HE_ADDR)); + } else { + writel_relaxed(hso_begin, priv->io_base + _REG(L_STH1_HS_ADDR)); + writel_relaxed(hso_end, priv->io_base + _REG(L_STH1_HE_ADDR)); + } + writel_relaxed(0, priv->io_base + _REG(L_STH1_VS_ADDR)); + writel_relaxed(max_lncnt, priv->io_base + _REG(L_STH1_VE_ADDR)); + + /* Vsync signal for TTL */ + writel_relaxed(vso_begin, priv->io_base + _REG(L_STV1_HS_ADDR)); + writel_relaxed(vso_end, priv->io_base + _REG(L_STV1_HE_ADDR)); + if (mode->flags & DRM_MODE_FLAG_PVSYNC) { + writel_relaxed(vso_eline, priv->io_base + _REG(L_STV1_VS_ADDR)); + writel_relaxed(vso_bline, priv->io_base + _REG(L_STV1_VE_ADDR)); + } else { + writel_relaxed(vso_bline, priv->io_base + _REG(L_STV1_VS_ADDR)); + writel_relaxed(vso_eline, priv->io_base + _REG(L_STV1_VE_ADDR)); + } + + /* DE signal */ + writel_relaxed(havon_begin, priv->io_base + _REG(L_DE_HS_ADDR)); + writel_relaxed(havon_end + 1, priv->io_base + _REG(L_DE_HE_ADDR)); + writel_relaxed(vavon_bline, priv->io_base + _REG(L_DE_VS_ADDR)); + writel_relaxed(vavon_eline, priv->io_base + _REG(L_DE_VE_ADDR)); + + /* Hsync signal */ + writel_relaxed(hso_begin, priv->io_base + _REG(L_HSYNC_HS_ADDR)); + writel_relaxed(hso_end, priv->io_base + _REG(L_HSYNC_HE_ADDR)); + writel_relaxed(0, priv->io_base + _REG(L_HSYNC_VS_ADDR)); + writel_relaxed(max_lncnt, priv->io_base + _REG(L_HSYNC_VE_ADDR)); + + /* Vsync signal */ + writel_relaxed(vso_begin, priv->io_base + _REG(L_VSYNC_HS_ADDR)); + writel_relaxed(vso_end, priv->io_base + _REG(L_VSYNC_HE_ADDR)); + writel_relaxed(vso_bline, priv->io_base + _REG(L_VSYNC_VS_ADDR)); + writel_relaxed(vso_eline, priv->io_base + _REG(L_VSYNC_VE_ADDR)); + + writel_relaxed(0, priv->io_base + _REG(L_INV_CNT_ADDR)); + writel_relaxed(BIT(4) | BIT(5), + priv->io_base + _REG(L_TCON_MISC_SEL_ADDR)); + + priv->venc.current_mode = MESON_VENC_MODE_MIPI_DSI; +} +EXPORT_SYMBOL_GPL(meson_venc_mipi_dsi_mode_set); + void meson_venci_cvbs_mode_set(struct meson_drm *priv, struct meson_cvbs_enci_mode *mode) { @@ -1747,8 +1966,15 @@ unsigned int meson_venci_get_field(struct meson_drm *priv)
void meson_venc_enable_vsync(struct meson_drm *priv) { - writel_relaxed(VENC_INTCTRL_ENCI_LNRST_INT_EN, - priv->io_base + _REG(VENC_INTCTRL)); + switch (priv->venc.current_mode) { + case MESON_VENC_MODE_MIPI_DSI: + writel_relaxed(0x200, + priv->io_base + _REG(VENC_INTCTRL)); + break; + default: + writel_relaxed(VENC_INTCTRL_ENCI_LNRST_INT_EN, + priv->io_base + _REG(VENC_INTCTRL)); + } regmap_update_bits(priv->hhi, HHI_GCLK_MPEG2, BIT(25), BIT(25)); }
diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h index 9138255ffc9e..0f59adb1c6db 100644 --- a/drivers/gpu/drm/meson/meson_venc.h +++ b/drivers/gpu/drm/meson/meson_venc.h @@ -21,6 +21,7 @@ enum { MESON_VENC_MODE_CVBS_PAL, MESON_VENC_MODE_CVBS_NTSC, MESON_VENC_MODE_HDMI, + MESON_VENC_MODE_MIPI_DSI, };
struct meson_cvbs_enci_mode { @@ -47,6 +48,9 @@ struct meson_cvbs_enci_mode { unsigned int analog_sync_adj; };
+/* LCD Encoder gamma setup */ +void meson_encl_load_gamma(struct meson_drm *priv); + /* HDMI Clock parameters */ enum drm_mode_status meson_venc_hdmi_supported_mode(const struct drm_display_mode *mode); @@ -63,6 +67,8 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic, unsigned int ycrcb_map, bool yuv420_mode, const struct drm_display_mode *mode); +void meson_venc_mipi_dsi_mode_set(struct meson_drm *priv, + const struct drm_display_mode *mode); unsigned int meson_venci_get_field(struct meson_drm *priv);
void meson_venc_enable_vsync(struct meson_drm *priv); diff --git a/drivers/gpu/drm/meson/meson_vpp.h b/drivers/gpu/drm/meson/meson_vpp.h index afc9553ed8d3..b790042a1650 100644 --- a/drivers/gpu/drm/meson/meson_vpp.h +++ b/drivers/gpu/drm/meson/meson_vpp.h @@ -12,6 +12,8 @@ struct drm_rect; struct meson_drm;
+/* Mux VIU/VPP to ENCL */ +#define MESON_VIU_VPP_MUX_ENCL 0x0 /* Mux VIU/VPP to ENCI */ #define MESON_VIU_VPP_MUX_ENCI 0x5 /* Mux VIU/VPP to ENCP */
Hi Neil,
On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong narmstrong@baylibre.com wrote:
This adds supports for the ENCL encoder connected to a MIPI-DSI transceiver on the Amlogic AXG SoCs.
Should this be "AXG and newer SoCs" or is this really AXG specific?
[...]
+#define GAMMA_VCOM_POL 7 /* RW */ +#define GAMMA_RVS_OUT 6 /* RW */ +#define ADR_RDY 5 /* Read Only */ +#define WR_RDY 4 /* Read Only */ +#define RD_RDY 3 /* Read Only */ +#define GAMMA_TR 2 /* RW */ +#define GAMMA_SET 1 /* RW */ +#define GAMMA_EN 0 /* RW */
+#define H_RD 12 +#define H_AUTO_INC 11 +#define H_SEL_R 10 +#define H_SEL_G 9 +#define H_SEL_B 8
I think all values above can be wrapped in the BIT() macro, then you don't need that below.
+#define HADR_MSB 7 /* 7:0 */ +#define HADR 0 /* 7:0 */
Here GENMASK(7, 0) can be used for HADR
Also I think prefixing all macros above with their register name (L_GAMMA_CNTL_PORT_ or L_GAMMA_ADDR_PORT_) will make the code easier to read.
[...]
writel_relaxed(0x8000, priv->io_base + _REG(ENCL_VIDEO_MODE));
The public S905 datasheet calls 0x8000 ENCL_PX_LN_CNT_SHADOW_EN
writel_relaxed(0x0418, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));
According to the public S905 datasheet this is: - BIT(3): ENCL_VIDEO_MODE_ADV_VFIFO_EN - BIT(4): ENCL_VIDEO_MODE_ADV_GAIN_HDTV - BIT(10): ENCL_SEL_GAMMA_RGB_IN
writel_relaxed(0x1000, priv->io_base + _REG(ENCL_VIDEO_FILT_CTRL));
I don't know the exact name but the 32-bit vendor kernel sources have a comment [0] saying that 0x1000 is "bypass filter" But maybe we can simply call it ENCL_VIDEO_FILT_CTRL_BYPASS_FILTER
[...]
writel_relaxed(3, priv->io_base + _REG(ENCL_VIDEO_RGBIN_CTRL));
The public S905 datasheet says: - BIT(0): USE RGB data from VIU, furthermore a comment in the 3.10 kernel sources make this more clear: bit[0] 1:RGB, 0:YUV - BIT(1): CFG_VIDEO_RGBIN_ZBLK
/* default black pattern */writel_relaxed(0, priv->io_base + _REG(ENCL_TST_MDSEL));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_Y));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CB));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CR));writel_relaxed(1, priv->io_base + _REG(ENCL_TST_EN));writel_bits_relaxed(BIT(3), 0, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));
same as above: ENCL_VIDEO_MODE_ADV_VFIFO_EN
writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN));writel_relaxed(0, priv->io_base + _REG(L_RGB_BASE_ADDR));writel_relaxed(0x400, priv->io_base + _REG(L_RGB_COEFF_ADDR));
note to self: L_RGB_COEFF_ADDR seems to contain some "magic" value, there's no further info in the 3.10 kernel sources or datasheet
writel_relaxed(0x400, priv->io_base + _REG(L_DITH_CNTL_ADDR));
According to the public S905 datasheet BIT(10) is DITH10_EN (10-bits Dithering to 8 Bits Enable). I am not sure if this would belong to the selected video mode/bit depth. I'll let other reviewers decide if this is relevant or not because I don't know.
[...]
writel_relaxed(0, priv->io_base + _REG(L_INV_CNT_ADDR));writel_relaxed(BIT(4) | BIT(5),priv->io_base + _REG(L_TCON_MISC_SEL_ADDR));
the public S905 datasheet states: - BIT(4): STV1_SEL (STV1 is frame Signal) - BIT(5): STV2_SEL (STV2 is frame Signal) This doesn't seem helpful to me though, but maybe you can still create preprocessor macros for this (for consistency)?
[...]
switch (priv->venc.current_mode) {case MESON_VENC_MODE_MIPI_DSI:writel_relaxed(0x200,priv->io_base + _REG(VENC_INTCTRL));
the public S905 datasheet documents this as: - BIT(9): ENCP_LNRST_INT_EN (Progressive encoder filed change interrupt enable) Please add a preprocessor macro to make it consistent with VENC_INTCTRL_ENCI_LNRST_INT_EN which already exists and is used below.
Best regards, Martin
On 07/01/2022 23:33, Martin Blumenstingl wrote:
Hi Neil,
On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong narmstrong@baylibre.com wrote:
This adds supports for the ENCL encoder connected to a MIPI-DSI transceiver on the Amlogic AXG SoCs> Should this be "AXG and newer SoCs" or is this really AXG specific?
Yup should be, thanks for noting
[...]
+#define GAMMA_VCOM_POL 7 /* RW */ +#define GAMMA_RVS_OUT 6 /* RW */ +#define ADR_RDY 5 /* Read Only */ +#define WR_RDY 4 /* Read Only */ +#define RD_RDY 3 /* Read Only */ +#define GAMMA_TR 2 /* RW */ +#define GAMMA_SET 1 /* RW */ +#define GAMMA_EN 0 /* RW */
+#define H_RD 12 +#define H_AUTO_INC 11 +#define H_SEL_R 10 +#define H_SEL_G 9 +#define H_SEL_B 8
I think all values above can be wrapped in the BIT() macro, then you don't need that below.
yep
+#define HADR_MSB 7 /* 7:0 */ +#define HADR 0 /* 7:0 */
Here GENMASK(7, 0) can be used for HADR
Also I think prefixing all macros above with their register name (L_GAMMA_CNTL_PORT_ or L_GAMMA_ADDR_PORT_) will make the code easier to read.
[...]
writel_relaxed(0x8000, priv->io_base + _REG(ENCL_VIDEO_MODE));The public S905 datasheet calls 0x8000 ENCL_PX_LN_CNT_SHADOW_EN
Thanks for searching !
writel_relaxed(0x0418, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));According to the public S905 datasheet this is:
- BIT(3): ENCL_VIDEO_MODE_ADV_VFIFO_EN
- BIT(4): ENCL_VIDEO_MODE_ADV_GAIN_HDTV
- BIT(10): ENCL_SEL_GAMMA_RGB_IN
writel_relaxed(0x1000, priv->io_base + _REG(ENCL_VIDEO_FILT_CTRL));I don't know the exact name but the 32-bit vendor kernel sources have a comment [0] saying that 0x1000 is "bypass filter" But maybe we can simply call it ENCL_VIDEO_FILT_CTRL_BYPASS_FILTER
Yep
[...]
writel_relaxed(3, priv->io_base + _REG(ENCL_VIDEO_RGBIN_CTRL));The public S905 datasheet says:
- BIT(0): USE RGB data from VIU, furthermore a comment in the 3.10
kernel sources make this more clear: bit[0] 1:RGB, 0:YUV
- BIT(1): CFG_VIDEO_RGBIN_ZBLK
/* default black pattern */writel_relaxed(0, priv->io_base + _REG(ENCL_TST_MDSEL));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_Y));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CB));writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CR));writel_relaxed(1, priv->io_base + _REG(ENCL_TST_EN));writel_bits_relaxed(BIT(3), 0, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));same as above: ENCL_VIDEO_MODE_ADV_VFIFO_EN
writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN));writel_relaxed(0, priv->io_base + _REG(L_RGB_BASE_ADDR));writel_relaxed(0x400, priv->io_base + _REG(L_RGB_COEFF_ADDR));note to self: L_RGB_COEFF_ADDR seems to contain some "magic" value, there's no further info in the 3.10 kernel sources or datasheet
writel_relaxed(0x400, priv->io_base + _REG(L_DITH_CNTL_ADDR));According to the public S905 datasheet BIT(10) is DITH10_EN (10-bits Dithering to 8 Bits Enable). I am not sure if this would belong to the selected video mode/bit depth. I'll let other reviewers decide if this is relevant or not because I don't know.
it would probably for pre-GXL when the pipeline was 8bit, would probably need to add a comment if someone wants to us DPI/LVDS on pre-GXL.
[...]
writel_relaxed(0, priv->io_base + _REG(L_INV_CNT_ADDR));writel_relaxed(BIT(4) | BIT(5),priv->io_base + _REG(L_TCON_MISC_SEL_ADDR));the public S905 datasheet states:
- BIT(4): STV1_SEL (STV1 is frame Signal)
- BIT(5): STV2_SEL (STV2 is frame Signal)
This doesn't seem helpful to me though, but maybe you can still create preprocessor macros for this (for consistency)?
yep
[...]
switch (priv->venc.current_mode) {case MESON_VENC_MODE_MIPI_DSI:writel_relaxed(0x200,priv->io_base + _REG(VENC_INTCTRL));the public S905 datasheet documents this as:
- BIT(9): ENCP_LNRST_INT_EN (Progressive encoder filed change interrupt enable)
Please add a preprocessor macro to make it consistent with VENC_INTCTRL_ENCI_LNRST_INT_EN which already exists and is used below.
Yep
Thanks for the review :-)
Neil
Best regards, Martin
The DSI path used the ENCL pixel encoder, thus this adds a clock config using the HDMI PLL in order to feed the ENCL encoder via the VCLK2 path and the CTS_ENCL clock output.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/gpu/drm/meson/meson_vclk.c | 47 ++++++++++++++++++++++++++++++ drivers/gpu/drm/meson/meson_vclk.h | 1 + 2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c index 2a82119eb58e..5e4d982be1c8 100644 --- a/drivers/gpu/drm/meson/meson_vclk.c +++ b/drivers/gpu/drm/meson/meson_vclk.c @@ -55,6 +55,8 @@ #define VCLK2_DIV_MASK 0xff #define VCLK2_DIV_EN BIT(16) #define VCLK2_DIV_RESET BIT(17) +#define CTS_ENCL_SEL_MASK (0xf << 12) +#define CTS_ENCL_SEL_SHIFT 12 #define CTS_VDAC_SEL_MASK (0xf << 28) #define CTS_VDAC_SEL_SHIFT 28 #define HHI_VIID_CLK_CNTL 0x12c /* 0x4b offset in data sheet */ @@ -83,6 +85,7 @@ #define VCLK_DIV12_EN BIT(4) #define HHI_VID_CLK_CNTL2 0x194 /* 0x65 offset in data sheet */ #define CTS_ENCI_EN BIT(0) +#define CTS_ENCL_EN BIT(3) #define CTS_ENCP_EN BIT(2) #define CTS_VDAC_EN BIT(4) #define HDMI_TX_PIXEL_EN BIT(5) @@ -1024,6 +1027,47 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq, regmap_update_bits(priv->hhi, HHI_VID_CLK_CNTL, VCLK_EN, VCLK_EN); }
+static void meson_dsi_clock_config(struct meson_drm *priv, unsigned int freq) +{ + meson_hdmi_pll_generic_set(priv, freq * 10); + + /* Setup vid_pll divider value /5 */ + meson_vid_pll_set(priv, VID_PLL_DIV_5); + + /* Disable VCLK2 */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, 0); + + /* Setup the VCLK2 divider value /2 */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV, VCLK2_DIV_MASK, 2 - 1); + + /* select vid_pll for vclk2 */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, + VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT)); + + /* enable vclk2 gate */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, VCLK2_EN); + + /* select vclk2_div1 for encl */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV, + CTS_ENCL_SEL_MASK, (8 << CTS_ENCL_SEL_SHIFT)); + + /* release vclk2_div_reset and enable vclk2_div */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV, VCLK2_DIV_EN | VCLK2_DIV_RESET, + VCLK2_DIV_EN); + + /* enable vclk2_div1 gate */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_DIV1_EN, VCLK2_DIV1_EN); + + /* reset vclk2 */ + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_SOFT_RESET, VCLK2_SOFT_RESET); + regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_SOFT_RESET, 0); + + /* enable encl_clk */ + regmap_update_bits(priv->hhi, HHI_VID_CLK_CNTL2, CTS_ENCL_EN, CTS_ENCL_EN); + + usleep_range(10000, 11000); +} + void meson_vclk_setup(struct meson_drm *priv, unsigned int target, unsigned int phy_freq, unsigned int vclk_freq, unsigned int venc_freq, unsigned int dac_freq, @@ -1050,6 +1094,9 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target, meson_vclk_set(priv, phy_freq, 0, 0, 0, VID_PLL_DIV_5, 2, 1, 1, false, false); return; + } else if (target == MESON_VCLK_TARGET_DSI) { + meson_dsi_clock_config(priv, phy_freq); + return; }
hdmi_tx_div = vclk_freq / dac_freq; diff --git a/drivers/gpu/drm/meson/meson_vclk.h b/drivers/gpu/drm/meson/meson_vclk.h index 60617aaf18dd..1152b3af8d2e 100644 --- a/drivers/gpu/drm/meson/meson_vclk.h +++ b/drivers/gpu/drm/meson/meson_vclk.h @@ -17,6 +17,7 @@ enum { MESON_VCLK_TARGET_CVBS = 0, MESON_VCLK_TARGET_HDMI = 1, MESON_VCLK_TARGET_DMT = 2, + MESON_VCLK_TARGET_DSI = 3, };
/* 27MHz is the CVBS Pixel Clock */
This adds an encoder bridge designed to drive a MIPI-DSI display by using the ENCL encoder through the internal MIPI DSI transceiver connected to the output of the ENCL pixel encoder.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/gpu/drm/meson/Makefile | 2 +- drivers/gpu/drm/meson/meson_drv.c | 7 + drivers/gpu/drm/meson/meson_encoder_dsi.c | 159 ++++++++++++++++++++++ drivers/gpu/drm/meson/meson_encoder_dsi.h | 12 ++ 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.h
diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile index 3afa31bdc950..833e18c20603 100644 --- a/drivers/gpu/drm/meson/Makefile +++ b/drivers/gpu/drm/meson/Makefile @@ -2,7 +2,7 @@ meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_encoder_cvbs.o meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_overlay.o meson-drm-y += meson_rdma.o meson_osd_afbcd.o -meson-drm-y += meson_encoder_hdmi.o +meson-drm-y += meson_encoder_hdmi.o meson_encoder_dsi.o
obj-$(CONFIG_DRM_MESON) += meson-drm.o obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 80f1d439841a..ff278a2b9e6e 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -33,6 +33,7 @@ #include "meson_registers.h" #include "meson_encoder_cvbs.h" #include "meson_encoder_hdmi.h" +#include "meson_encoder_dsi.h" #include "meson_viu.h" #include "meson_vpp.h" #include "meson_rdma.h" @@ -323,6 +324,12 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (ret) goto free_drm;
+ if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) { + ret = meson_encoder_dsi_init(priv); + if (ret) + goto free_drm; + } + ret = meson_plane_create(priv); if (ret) goto free_drm; diff --git a/drivers/gpu/drm/meson/meson_encoder_dsi.c b/drivers/gpu/drm/meson/meson_encoder_dsi.c new file mode 100644 index 000000000000..90347821cf96 --- /dev/null +++ b/drivers/gpu/drm/meson/meson_encoder_dsi.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2016 BayLibre, SAS + * Author: Neil Armstrong narmstrong@baylibre.com + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_graph.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> +#include <drm/drm_device.h> +#include <drm/drm_probe_helper.h> + +#include "meson_drv.h" +#include "meson_encoder_dsi.h" +#include "meson_registers.h" +#include "meson_venc.h" +#include "meson_vclk.h" + +struct meson_encoder_dsi { + struct drm_encoder encoder; + struct drm_bridge bridge; + struct drm_bridge *next_bridge; + struct meson_drm *priv; +}; + +#define bridge_to_meson_encoder_dsi(x) \ + container_of(x, struct meson_encoder_dsi, bridge) + +static int meson_encoder_dsi_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct meson_encoder_dsi *encoder_dsi = bridge_to_meson_encoder_dsi(bridge); + + return drm_bridge_attach(bridge->encoder, encoder_dsi->next_bridge, + &encoder_dsi->bridge, flags); +} + +static void meson_encoder_dsi_mode_set(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + const struct drm_display_mode *adjusted_mode) +{ + struct meson_encoder_dsi *encoder_dsi = bridge_to_meson_encoder_dsi(bridge); + struct meson_drm *priv = encoder_dsi->priv; + + meson_vclk_setup(priv, MESON_VCLK_TARGET_DSI, mode->clock, 0, 0, 0, false); + + meson_venc_mipi_dsi_mode_set(priv, mode); + meson_encl_load_gamma(priv); + + writel_relaxed(0, priv->io_base + _REG(ENCL_VIDEO_EN)); + + writel_bits_relaxed(BIT(3), BIT(3), priv->io_base + _REG(ENCL_VIDEO_MODE_ADV)); + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_EN)); +} + +static void meson_encoder_dsi_atomic_enable(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state) +{ + struct meson_encoder_dsi *encoder_dsi = bridge_to_meson_encoder_dsi(bridge); + struct meson_drm *priv = encoder_dsi->priv; + + writel_bits_relaxed(BIT(0), 0, priv->io_base + _REG(VPP_WRAP_OSD1_MATRIX_EN_CTRL)); + + writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN)); +} + +static void meson_encoder_dsi_atomic_disable(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state) +{ + struct meson_encoder_dsi *meson_encoder_dsi = + bridge_to_meson_encoder_dsi(bridge); + struct meson_drm *priv = meson_encoder_dsi->priv; + + writel_relaxed(0, priv->io_base + _REG(ENCL_VIDEO_EN)); + + writel_bits_relaxed(BIT(0), BIT(0), priv->io_base + _REG(VPP_WRAP_OSD1_MATRIX_EN_CTRL)); +} + +static const struct drm_bridge_funcs meson_encoder_dsi_bridge_funcs = { + .attach = meson_encoder_dsi_attach, + /* + * TOFIX: remove when dw-mipi-dsi moves out of mode_set + * We should get rid of mode_set, but until dw-mipi-dsi uses it + * we need to setup the pixel clock before the following + * bridge tries to setup the HW. + */ + .mode_set = meson_encoder_dsi_mode_set, + .atomic_enable = meson_encoder_dsi_atomic_enable, + .atomic_disable = meson_encoder_dsi_atomic_disable, + .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, +}; + +int meson_encoder_dsi_init(struct meson_drm *priv) +{ + struct meson_encoder_dsi *meson_encoder_dsi; + struct device_node *remote; + int ret; + + meson_encoder_dsi = devm_kzalloc(priv->dev, sizeof(*meson_encoder_dsi), GFP_KERNEL); + if (!meson_encoder_dsi) + return -ENOMEM; + + /* DSI Transceiver Bridge */ + remote = of_graph_get_remote_node(priv->dev->of_node, 2, 0); + if (!remote) { + dev_err(priv->dev, "DSI transceiver device is disabled"); + return 0; + } + + meson_encoder_dsi->next_bridge = of_drm_find_bridge(remote); + if (!meson_encoder_dsi->next_bridge) { + dev_dbg(priv->dev, "Failed to find DSI transceiver bridge: %d\n", ret); + return -EPROBE_DEFER; + } + + /* DSI Encoder Bridge */ + meson_encoder_dsi->bridge.funcs = &meson_encoder_dsi_bridge_funcs; + meson_encoder_dsi->bridge.of_node = priv->dev->of_node; + meson_encoder_dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; + + drm_bridge_add(&meson_encoder_dsi->bridge); + + meson_encoder_dsi->priv = priv; + + /* Encoder */ + ret = drm_simple_encoder_init(priv->drm, &meson_encoder_dsi->encoder, + DRM_MODE_ENCODER_DSI); + if (ret) { + dev_err(priv->dev, "Failed to init DSI encoder: %d\n", ret); + return ret; + } + + meson_encoder_dsi->encoder.possible_crtcs = BIT(0); + + /* Attach DSI Encoder Bridge to Encoder */ + ret = drm_bridge_attach(&meson_encoder_dsi->encoder, &meson_encoder_dsi->bridge, NULL, 0); + if (ret) { + dev_err(priv->dev, "Failed to attach bridge: %d\n", ret); + return ret; + } + + /* + * We should have now in place: + * encoder->[dsi encoder bridge]->[dw-mipi-dsi bridge]->[panel bridge]->[panel] + */ + + dev_dbg(priv->dev, "DSI encoder initialized\n"); + + return 0; +} diff --git a/drivers/gpu/drm/meson/meson_encoder_dsi.h b/drivers/gpu/drm/meson/meson_encoder_dsi.h new file mode 100644 index 000000000000..0f4b641eb633 --- /dev/null +++ b/drivers/gpu/drm/meson/meson_encoder_dsi.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021 BayLibre, SAS + * Author: Neil Armstrong narmstrong@baylibre.com + */ + +#ifndef __MESON_ENCODER_DSI_H +#define __MESON_ENCODER_DSI_H + +int meson_encoder_dsi_init(struct meson_drm *priv); + +#endif /* __MESON_ENCODER_DSI_H */
Hi Neil,
On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong narmstrong@baylibre.com wrote:
[...]
writel_bits_relaxed(BIT(3), BIT(3), priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));
see my comment on patch #3 from this series for BIT(3)
Best regards, Martin
On 07/01/2022 23:35, Martin Blumenstingl wrote:
Hi Neil,
On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong narmstrong@baylibre.com wrote:
[...]
writel_bits_relaxed(BIT(3), BIT(3), priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));see my comment on patch #3 from this series for BIT(3)
Yep, thanks,
Neil
Best regards, Martin
The Amlogic G12A/G12B/SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI Glue on other Amlogic SoCs.
This adds support for the Glue managing the transceiver, mimicing the init flow provided by Amlogic to setup the ENCL encoder, the glue, the transceiver, the digital D-PHY and the Analog PHY in the proper way.
An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the DW-MIPI-DSI transceiver.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/gpu/drm/meson/Kconfig | 7 + drivers/gpu/drm/meson/Makefile | 1 + drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 383 ++++++++++++++++++++++ drivers/gpu/drm/meson/meson_dw_mipi_dsi.h | 115 +++++++ 4 files changed, 506 insertions(+) create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.h
diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig index 6c70fc3214af..71a1364b51e1 100644 --- a/drivers/gpu/drm/meson/Kconfig +++ b/drivers/gpu/drm/meson/Kconfig @@ -17,3 +17,10 @@ config DRM_MESON_DW_HDMI default y if DRM_MESON select DRM_DW_HDMI imply DRM_DW_HDMI_I2S_AUDIO + +config DRM_MESON_DW_MIPI_DSI + tristate "MIPI DSI Synopsys Controller support for Amlogic Meson Display" + depends on DRM_MESON + default y if DRM_MESON + select DRM_DW_MIPI_DSI + select GENERIC_PHY_MIPI_DPHY diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile index 833e18c20603..43071bdbd4b9 100644 --- a/drivers/gpu/drm/meson/Makefile +++ b/drivers/gpu/drm/meson/Makefile @@ -6,3 +6,4 @@ meson-drm-y += meson_encoder_hdmi.o meson_encoder_dsi.o
obj-$(CONFIG_DRM_MESON) += meson-drm.o obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o +obj-$(CONFIG_DRM_MESON_DW_MIPI_DSI) += meson_dw_mipi_dsi.o diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c new file mode 100644 index 000000000000..75af3eec1b74 --- /dev/null +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c @@ -0,0 +1,383 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 BayLibre, SAS + * Author: Neil Armstrong narmstrong@baylibre.com + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. + */ + +#include <linux/clk.h> +#include <linux/component.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_graph.h> +#include <linux/reset.h> +#include <linux/phy/phy.h> + +#include <video/mipi_display.h> + +#include <drm/bridge/dw_mipi_dsi.h> +#include <drm/drm_mipi_dsi.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_device.h> +#include <drm/drm_probe_helper.h> +#include <drm/drm_print.h> + +#include "meson_drv.h" +#include "meson_dw_mipi_dsi.h" +#include "meson_registers.h" +#include "meson_venc.h" + +#define DRIVER_NAME "meson-dw-mipi-dsi" +#define DRIVER_DESC "Amlogic Meson MIPI-DSI DRM driver" + +/* MIPI DSI/VENC Color Format Definitions */ +#define MIPI_DSI_VENC_COLOR_30B 0x0 +#define MIPI_DSI_VENC_COLOR_24B 0x1 +#define MIPI_DSI_VENC_COLOR_18B 0x2 +#define MIPI_DSI_VENC_COLOR_16B 0x3 + +#define COLOR_16BIT_CFG_1 0x0 +#define COLOR_16BIT_CFG_2 0x1 +#define COLOR_16BIT_CFG_3 0x2 +#define COLOR_18BIT_CFG_1 0x3 +#define COLOR_18BIT_CFG_2 0x4 +#define COLOR_24BIT 0x5 +#define COLOR_20BIT_LOOSE 0x6 +#define COLOR_24_BIT_YCBCR 0x7 +#define COLOR_16BIT_YCBCR 0x8 +#define COLOR_30BIT 0x9 +#define COLOR_36BIT 0xa +#define COLOR_12BIT 0xb +#define COLOR_RGB_111 0xc +#define COLOR_RGB_332 0xd +#define COLOR_RGB_444 0xe + +/* MIPI DSI Relative REGISTERs Definitions */ +/* For MIPI_DSI_TOP_CNTL */ +#define BIT_DPI_COLOR_MODE 20 +#define BIT_IN_COLOR_MODE 16 +#define BIT_CHROMA_SUBSAMPLE 14 +#define BIT_COMP2_SEL 12 +#define BIT_COMP1_SEL 10 +#define BIT_COMP0_SEL 8 +#define BIT_DE_POL 6 +#define BIT_HSYNC_POL 5 +#define BIT_VSYNC_POL 4 +#define BIT_DPICOLORM 3 +#define BIT_DPISHUTDN 2 +#define BIT_EDPITE_INTR_PULSE 1 +#define BIT_ERR_INTR_PULSE 0 + +struct meson_dw_mipi_dsi { + struct meson_drm *priv; + struct device *dev; + void __iomem *base; + struct phy *phy; + union phy_configure_opts phy_opts; + struct dw_mipi_dsi *dmd; + struct dw_mipi_dsi_plat_data pdata; + struct mipi_dsi_device *dsi_device; + const struct drm_display_mode *mode; + struct clk *px_clk; +}; + +#define encoder_to_meson_dw_mipi_dsi(x) \ + container_of(x, struct meson_dw_mipi_dsi, encoder) + +static void meson_dw_mipi_dsi_hw_init(struct meson_dw_mipi_dsi *mipi_dsi) +{ + writel_relaxed((1 << 4) | (1 << 5) | (0 << 6), + mipi_dsi->base + MIPI_DSI_TOP_CNTL); + + writel_bits_relaxed(0xf, 0xf, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET); + writel_bits_relaxed(0xf, 0, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET); + + writel_bits_relaxed(0x3, 0x3, mipi_dsi->base + MIPI_DSI_TOP_CLK_CNTL); + + writel_relaxed(0, mipi_dsi->base + MIPI_DSI_TOP_MEM_PD); +} + +static int dw_mipi_dsi_phy_init(void *priv_data) +{ + struct meson_dw_mipi_dsi *mipi_dsi = priv_data; + unsigned int dpi_data_format, venc_data_width; + int ret; + + ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->phy_opts.mipi_dphy.hs_clk_rate); + if (ret) { + pr_err("Failed to set DSI PLL rate %lu\n", + mipi_dsi->phy_opts.mipi_dphy.hs_clk_rate); + + return ret; + } + + switch (mipi_dsi->dsi_device->format) { + case MIPI_DSI_FMT_RGB888: + dpi_data_format = COLOR_24BIT; + venc_data_width = MIPI_DSI_VENC_COLOR_24B; + break; + case MIPI_DSI_FMT_RGB666: + dpi_data_format = COLOR_18BIT_CFG_2; + venc_data_width = MIPI_DSI_VENC_COLOR_18B; + break; + case MIPI_DSI_FMT_RGB666_PACKED: + case MIPI_DSI_FMT_RGB565: + return -EINVAL; + }; + + /* Configure color format for DPI register */ + writel_relaxed((dpi_data_format << BIT_DPI_COLOR_MODE) | + (venc_data_width << BIT_IN_COLOR_MODE) | + 0 << BIT_COMP0_SEL | + 1 << BIT_COMP1_SEL | + 2 << BIT_COMP2_SEL | + (mipi_dsi->mode->flags & DRM_MODE_FLAG_NHSYNC ? 0 : BIT(BIT_HSYNC_POL)) | + (mipi_dsi->mode->flags & DRM_MODE_FLAG_NVSYNC ? 0 : BIT(BIT_VSYNC_POL)), + mipi_dsi->base + MIPI_DSI_TOP_CNTL); + + phy_power_on(mipi_dsi->phy); + + return 0; +} + +static void dw_mipi_dsi_phy_power_off(void *priv_data) +{ + struct meson_dw_mipi_dsi *mipi_dsi = priv_data; + + phy_power_off(mipi_dsi->phy); +} + +static int +dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode, + unsigned long mode_flags, u32 lanes, u32 format, + unsigned int *lane_mbps) +{ + struct meson_dw_mipi_dsi *mipi_dsi = priv_data; + int bpp; + + mipi_dsi->mode = mode; + + bpp = mipi_dsi_pixel_format_to_bpp(mipi_dsi->dsi_device->format); + + phy_mipi_dphy_get_default_config(mode->clock * 1000, + bpp, mipi_dsi->dsi_device->lanes, + &mipi_dsi->phy_opts.mipi_dphy); + + phy_configure(mipi_dsi->phy, &mipi_dsi->phy_opts); + + *lane_mbps = mipi_dsi->phy_opts.mipi_dphy.hs_clk_rate / 1000000; + + return 0; +} + +static int +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, + struct dw_mipi_dsi_dphy_timing *timing) +{ + /* TOFIX handle other cases */ + + timing->clk_lp2hs = 37; + timing->clk_hs2lp = 135; + timing->data_lp2hs = 50; + timing->data_hs2lp = 3; + + return 0; +} + +static int +dw_mipi_dsi_get_esc_clk_rate(void *priv_data, unsigned int *esc_clk_rate) +{ + *esc_clk_rate = 4; /* Mhz */ + + return 0; +} + +static const struct dw_mipi_dsi_phy_ops meson_dw_mipi_dsi_phy_ops = { + .init = dw_mipi_dsi_phy_init, + .power_off = dw_mipi_dsi_phy_power_off, + .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, + .get_timing = dw_mipi_dsi_phy_get_timing, + .get_esc_clk_rate = dw_mipi_dsi_get_esc_clk_rate, +}; + +static int meson_dw_mipi_dsi_bind(struct device *dev, struct device *master, void *data) +{ + struct meson_dw_mipi_dsi *mipi_dsi = dev_get_drvdata(dev); + struct drm_device *drm = data; + struct meson_drm *priv = drm->dev_private; + + /* Check before if we are supposed to have a sub-device... */ + if (!mipi_dsi->dsi_device) { + dw_mipi_dsi_remove(mipi_dsi->dmd); + return -EPROBE_DEFER; + } + + mipi_dsi->priv = priv; + + meson_dw_mipi_dsi_hw_init(mipi_dsi); + + return 0; +} + +static const struct component_ops meson_dw_mipi_dsi_ops = { + .bind = meson_dw_mipi_dsi_bind, +}; + +static int meson_dw_mipi_dsi_host_attach(void *priv_data, + struct mipi_dsi_device *device) +{ + struct meson_dw_mipi_dsi *mipi_dsi = priv_data; + + mipi_dsi->dsi_device = device; + + switch (device->format) { + case MIPI_DSI_FMT_RGB888: + break; + case MIPI_DSI_FMT_RGB666: + break; + case MIPI_DSI_FMT_RGB666_PACKED: + case MIPI_DSI_FMT_RGB565: + DRM_DEV_ERROR(mipi_dsi->dev, "invalid pixel format %d\n", device->format); + return -EINVAL; + }; + + phy_init(mipi_dsi->phy); + + return 0; +} + +static int meson_dw_mipi_dsi_host_detach(void *priv_data, + struct mipi_dsi_device *device) +{ + struct meson_dw_mipi_dsi *mipi_dsi = priv_data; + + if (device == mipi_dsi->dsi_device) + mipi_dsi->dsi_device = NULL; + else + return -EINVAL; + + phy_exit(mipi_dsi->phy); + + return 0; +} + +static const struct dw_mipi_dsi_host_ops meson_dw_mipi_dsi_host_ops = { + .attach = meson_dw_mipi_dsi_host_attach, + .detach = meson_dw_mipi_dsi_host_detach, +}; + +static int meson_dw_mipi_dsi_probe(struct platform_device *pdev) +{ + struct meson_dw_mipi_dsi *mipi_dsi; + struct reset_control *top_rst; + struct resource *res; + int ret; + + mipi_dsi = devm_kzalloc(&pdev->dev, sizeof(*mipi_dsi), GFP_KERNEL); + if (!mipi_dsi) + return -ENOMEM; + + mipi_dsi->dev = &pdev->dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + mipi_dsi->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(mipi_dsi->base)) + return PTR_ERR(mipi_dsi->base); + + mipi_dsi->phy = devm_phy_get(&pdev->dev, "dphy"); + if (IS_ERR(mipi_dsi->phy)) { + ret = PTR_ERR(mipi_dsi->phy); + dev_err(&pdev->dev, "failed to get mipi dphy: %d\n", ret); + return ret; + } + + mipi_dsi->px_clk = devm_clk_get(&pdev->dev, "px_clk"); + if (IS_ERR(mipi_dsi->px_clk)) { + dev_err(&pdev->dev, "Unable to get PLL clk\n"); + return PTR_ERR(mipi_dsi->px_clk); + } + + /* + * We use a TOP reset signal because the APB reset signal + * is handled by the TOP control registers. + */ + top_rst = devm_reset_control_get_exclusive(&pdev->dev, "top"); + if (IS_ERR(top_rst)) { + ret = PTR_ERR(top_rst); + + if (ret != -EPROBE_DEFER) + dev_err(&pdev->dev, "Unable to get reset control: %d\n", ret); + + return ret; + } + + ret = clk_prepare_enable(mipi_dsi->px_clk); + if (ret) { + dev_err(&pdev->dev, "Unable to prepare/enable PX clock\n"); + return ret; + } + + reset_control_assert(top_rst); + usleep_range(10, 20); + reset_control_deassert(top_rst); + + /* MIPI DSI Controller */ + + mipi_dsi->pdata.base = mipi_dsi->base; + mipi_dsi->pdata.max_data_lanes = 4; + mipi_dsi->pdata.phy_ops = &meson_dw_mipi_dsi_phy_ops; + mipi_dsi->pdata.host_ops = &meson_dw_mipi_dsi_host_ops; + mipi_dsi->pdata.priv_data = mipi_dsi; + platform_set_drvdata(pdev, mipi_dsi); + + mipi_dsi->dmd = dw_mipi_dsi_probe(pdev, &mipi_dsi->pdata); + if (IS_ERR(mipi_dsi->dmd)) { + ret = PTR_ERR(mipi_dsi->dmd); + if (ret != -EPROBE_DEFER) + dev_err(&pdev->dev, + "Failed to probe dw_mipi_dsi: %d\n", ret); + goto err_clkdisable; + } + + return component_add(mipi_dsi->dev, &meson_dw_mipi_dsi_ops); + +err_clkdisable: + clk_disable_unprepare(mipi_dsi->px_clk); + + return ret; +} + +static int meson_dw_mipi_dsi_remove(struct platform_device *pdev) +{ + struct meson_dw_mipi_dsi *mipi_dsi = dev_get_drvdata(&pdev->dev); + + dw_mipi_dsi_remove(mipi_dsi->dmd); + + component_del(mipi_dsi->dev, &meson_dw_mipi_dsi_ops); + + clk_disable_unprepare(mipi_dsi->px_clk); + + return 0; +} + +static const struct of_device_id meson_dw_mipi_dsi_of_table[] = { + { .compatible = "amlogic,meson-g12a-dw-mipi-dsi", }, + { } +}; +MODULE_DEVICE_TABLE(of, meson_dw_mipi_dsi_of_table); + +static struct platform_driver meson_dw_mipi_dsi_platform_driver = { + .probe = meson_dw_mipi_dsi_probe, + .remove = meson_dw_mipi_dsi_remove, + .driver = { + .name = DRIVER_NAME, + .of_match_table = meson_dw_mipi_dsi_of_table, + }, +}; +module_platform_driver(meson_dw_mipi_dsi_platform_driver); + +MODULE_AUTHOR("Neil Armstrong narmstrong@baylibre.com"); +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.h b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.h new file mode 100644 index 000000000000..4075a132e005 --- /dev/null +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.h @@ -0,0 +1,115 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020 BayLibre, SAS + * Author: Neil Armstrong narmstrong@baylibre.com + * Copyright (C) 2018 Amlogic, Inc. All rights reserved. + */ + +#ifndef __MESON_DW_MIPI_DSI_H +#define __MESON_DW_MIPI_DSI_H + +/* Top-level registers */ +/* [31: 3] Reserved. Default 0. + * [2] RW dpi_rst_n: Default 1. + * 1=Assert SW reset on mipi_dsi_host_dpi block. 0=Release reset. + * [1] RW intr_rst_n: Default 1. + * 1=Assert SW reset on mipi_dsi_host_intr block. 0=Release reset. + * [0] RW dwc_rst_n: Default 1. + * 1=Assert SW reset on IP core. 0=Release reset. + */ +#define MIPI_DSI_TOP_SW_RESET 0x3c0 +/* [31: 5] Reserved. Default 0. + * [4] RW manual_edpihalt: Default 0. + * 1=Manual suspend VencL; 0=do not suspend VencL. + * [3] RW auto_edpihalt_en: Default 0. + * 1=Enable IP's edpihalt signal to suspend VencL; + * 0=IP's edpihalt signal does not affect VencL. + * [2] RW clock_freerun: Apply to auto-clock gate only. Default 0. + * 0=Default, use auto-clock gating to save power; + * 1=use free-run clock, disable auto-clock gating, for debug mode. + * [1] RW enable_pixclk: A manual clock gate option, due to DWC IP does not + * have auto-clock gating. 1=Enable pixclk. Default 0. + * [0] RW enable_sysclk: A manual clock gate option, due to DWC IP does not + * have auto-clock gating. 1=Enable sysclk. Default 0. + */ +#define MIPI_DSI_TOP_CLK_CNTL 0x3c4 +/* [31:24] Reserved. Default 0. + * [23:20] RW dpi_color_mode: Define DPI pixel format. Default 0. + * 0=16-bit RGB565 config 1; + * 1=16-bit RGB565 config 2; + * 2=16-bit RGB565 config 3; + * 3=18-bit RGB666 config 1; + * 4=18-bit RGB666 config 2; + * 5=24-bit RGB888; + * 6=20-bit YCbCr 4:2:2; + * 7=24-bit YCbCr 4:2:2; + * 8=16-bit YCbCr 4:2:2; + * 9=30-bit RGB; + * 10=36-bit RGB; + * 11=12-bit YCbCr 4:2:0. + * [19] Reserved. Default 0. + * [18:16] RW in_color_mode: Define VENC data width. Default 0. + * 0=30-bit pixel; + * 1=24-bit pixel; + * 2=18-bit pixel, RGB666; + * 3=16-bit pixel, RGB565. + * [15:14] RW chroma_subsample: Define method of chroma subsampling. Default 0. + * Applicable to YUV422 or YUV420 only. + * 0=Use even pixel's chroma; + * 1=Use odd pixel's chroma; + * 2=Use averaged value between even and odd pair. + * [13:12] RW comp2_sel: Select which component to be Cr or B: Default 2. + * 0=comp0; 1=comp1; 2=comp2. + * [11:10] RW comp1_sel: Select which component to be Cb or G: Default 1. + * 0=comp0; 1=comp1; 2=comp2. + * [9: 8] RW comp0_sel: Select which component to be Y or R: Default 0. + * 0=comp0; 1=comp1; 2=comp2. + * [7] Reserved. Default 0. + * [6] RW de_pol: Default 0. + * If DE input is active low, set to 1 to invert to active high. + * [5] RW hsync_pol: Default 0. + * If HS input is active low, set to 1 to invert to active high. + * [4] RW vsync_pol: Default 0. + * If VS input is active low, set to 1 to invert to active high. + * [3] RW dpicolorm: Signal to IP. Default 0. + * [2] RW dpishutdn: Signal to IP. Default 0. + * [1] Reserved. Default 0. + * [0] Reserved. Default 0. + */ +#define MIPI_DSI_TOP_CNTL 0x3c8 +#define MIPI_DSI_TOP_SUSPEND_CNTL 0x3cc +#define MIPI_DSI_TOP_SUSPEND_LINE 0x3d0 +#define MIPI_DSI_TOP_SUSPEND_PIX 0x3d4 +#define MIPI_DSI_TOP_MEAS_CNTL 0x3d8 +/* [0] R stat_edpihalt: edpihalt signal from IP. Default 0. */ +#define MIPI_DSI_TOP_STAT 0x3dc +#define MIPI_DSI_TOP_MEAS_STAT_TE0 0x3e0 +#define MIPI_DSI_TOP_MEAS_STAT_TE1 0x3e4 +#define MIPI_DSI_TOP_MEAS_STAT_VS0 0x3e8 +#define MIPI_DSI_TOP_MEAS_STAT_VS1 0x3ec +/* [31:16] RW intr_stat/clr. Default 0. + * For each bit, read as this interrupt level status, + * write 1 to clear. + * [31:22] Reserved + * [ 21] stat/clr of eof interrupt + * [ 21] vde_fall interrupt + * [ 19] stat/clr of de_rise interrupt + * [ 18] stat/clr of vs_fall interrupt + * [ 17] stat/clr of vs_rise interrupt + * [ 16] stat/clr of dwc_edpite interrupt + * [15: 0] RW intr_enable. Default 0. + * For each bit, 1=enable this interrupt, 0=disable. + * [15: 6] Reserved + * [ 5] eof interrupt + * [ 4] de_fall interrupt + * [ 3] de_rise interrupt + * [ 2] vs_fall interrupt + * [ 1] vs_rise interrupt + * [ 0] dwc_edpite interrupt + */ +#define MIPI_DSI_TOP_INTR_CNTL_STAT 0x3f0 +// 31: 2 Reserved. Default 0. +// 1: 0 RW mem_pd. Default 3. +#define MIPI_DSI_TOP_MEM_PD 0x3f4 + +#endif /* __MESON_DW_MIPI_DSI_H */
Hi Neil,
some high-level comments from me below.
On Fri, Jan 7, 2022 at 3:58 PM Neil Armstrong narmstrong@baylibre.com wrote: [...]
+/* MIPI DSI Relative REGISTERs Definitions */ +/* For MIPI_DSI_TOP_CNTL */ +#define BIT_DPI_COLOR_MODE 20 +#define BIT_IN_COLOR_MODE 16 +#define BIT_CHROMA_SUBSAMPLE 14 +#define BIT_COMP2_SEL 12 +#define BIT_COMP1_SEL 10 +#define BIT_COMP0_SEL 8 +#define BIT_DE_POL 6 +#define BIT_HSYNC_POL 5 +#define BIT_VSYNC_POL 4 +#define BIT_DPICOLORM 3 +#define BIT_DPISHUTDN 2 +#define BIT_EDPITE_INTR_PULSE 1 +#define BIT_ERR_INTR_PULSE 0
Why not use BIT() and GENMASK() for these and prefixing them with MIPI_DSI_TOP_CNTL_? That would make them consistent with other parts of the meson sub-driver.
[...]
+static void meson_dw_mipi_dsi_hw_init(struct meson_dw_mipi_dsi *mipi_dsi) +{
writel_relaxed((1 << 4) | (1 << 5) | (0 << 6),mipi_dsi->base + MIPI_DSI_TOP_CNTL);
please use the macros from above
writel_bits_relaxed(0xf, 0xf, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);writel_bits_relaxed(0xf, 0, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);
[...]
phy_power_on(mipi_dsi->phy);
Please propagate the error code here. Also shouldn't this go to a new dw_mipi_dsi_phy_power_on() as the PHY driver uses the updated settings from phy_configure only in it's .power_on callback?
[...]
phy_configure(mipi_dsi->phy, &mipi_dsi->phy_opts);
please propagate the error code here as the PHY driver has some explicit code to return an error in it's .phy_configure callback
[...]
phy_init(mipi_dsi->phy);
please propagate the error code here
[...]
phy_exit(mipi_dsi->phy);
please propagate the error code here
[...]
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);mipi_dsi->base = devm_ioremap_resource(&pdev->dev, res);
other parts of the meson DRM driver have been converted to use devm_platform_ioremap_resource() I suggest updating this as well to simplify the code here
[...]
mipi_dsi->phy = devm_phy_get(&pdev->dev, "dphy");if (IS_ERR(mipi_dsi->phy)) {ret = PTR_ERR(mipi_dsi->phy);dev_err(&pdev->dev, "failed to get mipi dphy: %d\n", ret);return ret;
you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->phy, "failed to get mipi dphy\n");
[...]
mipi_dsi->px_clk = devm_clk_get(&pdev->dev, "px_clk");if (IS_ERR(mipi_dsi->px_clk)) {dev_err(&pdev->dev, "Unable to get PLL clk\n");return PTR_ERR(mipi_dsi->px_clk);
you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->px_clk, "Unable to get PLL clk\n"); Also should it say s/PLL clk/px clock/?
[...]
top_rst = devm_reset_control_get_exclusive(&pdev->dev, "top");if (IS_ERR(top_rst)) {ret = PTR_ERR(top_rst);if (ret != -EPROBE_DEFER)dev_err(&pdev->dev, "Unable to get reset control: %d\n", ret);return ret;
you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(top_rst, "Unable to get reset control\n");
[...]
mipi_dsi->dmd = dw_mipi_dsi_probe(pdev, &mipi_dsi->pdata);if (IS_ERR(mipi_dsi->dmd)) {ret = PTR_ERR(mipi_dsi->dmd);if (ret != -EPROBE_DEFER)dev_err(&pdev->dev,"Failed to probe dw_mipi_dsi: %d\n", ret);
you can simplify this with: dev_err_probe(&pdev->dev, ret, "Failed to probe dw_mipi_dsi\n");
Best regards, Martin
On 07/01/2022 23:49, Martin Blumenstingl wrote:
Hi Neil,
some high-level comments from me below.
On Fri, Jan 7, 2022 at 3:58 PM Neil Armstrong narmstrong@baylibre.com wrote: [...]
+/* MIPI DSI Relative REGISTERs Definitions */ +/* For MIPI_DSI_TOP_CNTL */ +#define BIT_DPI_COLOR_MODE 20 +#define BIT_IN_COLOR_MODE 16 +#define BIT_CHROMA_SUBSAMPLE 14 +#define BIT_COMP2_SEL 12 +#define BIT_COMP1_SEL 10 +#define BIT_COMP0_SEL 8 +#define BIT_DE_POL 6 +#define BIT_HSYNC_POL 5 +#define BIT_VSYNC_POL 4 +#define BIT_DPICOLORM 3 +#define BIT_DPISHUTDN 2 +#define BIT_EDPITE_INTR_PULSE 1 +#define BIT_ERR_INTR_PULSE 0
Why not use BIT() and GENMASK() for these and prefixing them with MIPI_DSI_TOP_CNTL_? That would make them consistent with other parts of the meson sub-driver.
Yeah it was a lousy copy-paste from vendor driver, and I was lazy, but I'll fix this.
[...]
+static void meson_dw_mipi_dsi_hw_init(struct meson_dw_mipi_dsi *mipi_dsi) +{
writel_relaxed((1 << 4) | (1 << 5) | (0 << 6),mipi_dsi->base + MIPI_DSI_TOP_CNTL);please use the macros from above
writel_bits_relaxed(0xf, 0xf, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);writel_bits_relaxed(0xf, 0, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);[...]
phy_power_on(mipi_dsi->phy);Please propagate the error code here. Also shouldn't this go to a new dw_mipi_dsi_phy_power_on() as the PHY driver uses the updated settings from phy_configure only in it's .power_on callback?
Good point, let me check that.
[...]
phy_configure(mipi_dsi->phy, &mipi_dsi->phy_opts);please propagate the error code here as the PHY driver has some explicit code to return an error in it's .phy_configure callback
[...]
phy_init(mipi_dsi->phy);please propagate the error code here
[...]
phy_exit(mipi_dsi->phy);please propagate the error code here
ok for the 3
[...]
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);mipi_dsi->base = devm_ioremap_resource(&pdev->dev, res);other parts of the meson DRM driver have been converted to use devm_platform_ioremap_resource() I suggest updating this as well to simplify the code here
Yep, again lazyness
[...]
mipi_dsi->phy = devm_phy_get(&pdev->dev, "dphy");if (IS_ERR(mipi_dsi->phy)) {ret = PTR_ERR(mipi_dsi->phy);dev_err(&pdev->dev, "failed to get mipi dphy: %d\n", ret);return ret;you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->phy, "failed to get mipi dphy\n");
[...]
mipi_dsi->px_clk = devm_clk_get(&pdev->dev, "px_clk");if (IS_ERR(mipi_dsi->px_clk)) {dev_err(&pdev->dev, "Unable to get PLL clk\n");return PTR_ERR(mipi_dsi->px_clk);you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->px_clk, "Unable to get PLL clk\n"); Also should it say s/PLL clk/px clock/?
[...]
top_rst = devm_reset_control_get_exclusive(&pdev->dev, "top");if (IS_ERR(top_rst)) {ret = PTR_ERR(top_rst);if (ret != -EPROBE_DEFER)dev_err(&pdev->dev, "Unable to get reset control: %d\n", ret);return ret;you can simplify this with: return dev_err_probe(&pdev->dev, PTR_ERR(top_rst, "Unable to get reset control\n");
[...]
mipi_dsi->dmd = dw_mipi_dsi_probe(pdev, &mipi_dsi->pdata);if (IS_ERR(mipi_dsi->dmd)) {ret = PTR_ERR(mipi_dsi->dmd);if (ret != -EPROBE_DEFER)dev_err(&pdev->dev,"Failed to probe dw_mipi_dsi: %d\n", ret);you can simplify this with: dev_err_probe(&pdev->dev, ret, "Failed to probe dw_mipi_dsi\n");
Again 4 lazyness effects, will fix !
Thanks, Neil
Best regards, Martin
dri-devel@lists.freedesktop.org
-
Martin Blumenstingl -
Neil Armstrong -
Rob Herring