* [PATCH v2 0/2] Add support of Lontium lt8912 MIPI to HDMI bridge @ 2021-02-14 23:29 Adrien Grassein 2021-02-14 23:29 ` [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 Adrien Grassein 2021-02-14 23:29 ` [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge Adrien Grassein 0 siblings, 2 replies; 8+ messages in thread From: Adrien Grassein @ 2021-02-14 23:29 UTC (permalink / raw) Cc: airlied, daniel, a.hajda, robh+dt, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, dri-devel, devicetree, linux-kernel, Adrien Grassein Hello, this patch set adds the support of the Lontium lt8912 MIPI to HDMI bridge in the kernel. It's only support the video part, not the audio part yet since I don't have the datasheet of this component. I get the current i2c configuration from Digi and Boundary drivers. Developed using the DB_DSIHD board from BoundaryDevices. Update in v2 - Use standard data-lanes instead of a custom prop; - Use hdmi-connector node. Thanks, Adrien Grassein Adrien Grassein (2): dt-bindings: display: bridge: Add documentation for LT8912 drm/bridge: Introduce LT8912 DSI to HDMI bridge .../display/bridge/lontium,lt8912.yaml | 102 +++ MAINTAINERS | 6 + drivers/gpu/drm/bridge/Kconfig | 14 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt8912.c | 719 ++++++++++++++++++ 5 files changed, 842 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912.c -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 2021-02-14 23:29 [PATCH v2 0/2] Add support of Lontium lt8912 MIPI to HDMI bridge Adrien Grassein @ 2021-02-14 23:29 ` Adrien Grassein 2021-02-17 20:24 ` Rob Herring 2021-02-17 22:16 ` Rob Herring 2021-02-14 23:29 ` [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge Adrien Grassein 1 sibling, 2 replies; 8+ messages in thread From: Adrien Grassein @ 2021-02-14 23:29 UTC (permalink / raw) Cc: airlied, daniel, a.hajda, robh+dt, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, dri-devel, devicetree, linux-kernel, Adrien Grassein Lontium LT8912 is a DSI to HDMI bridge. Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> --- .../display/bridge/lontium,lt8912.yaml | 102 ++++++++++++++++++ MAINTAINERS | 5 + 2 files changed, 107 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml new file mode 100644 index 000000000000..1e5a2ad5eb47 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml @@ -0,0 +1,102 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/lontium,lt8912.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Lontium LT8912 MIPI to HDMI Bridge + +maintainers: + - Adrien Grassein <adrien.grassein@gmail.com> + +description: | + The LT8912 is a bridge device which convert DSI to HDMI + +properties: + compatible: + enum: + - lontium,lt8912 + + reg: + maxItems: 1 + + reset-gpios: + maxItems: 1 + description: GPIO connected to active high RESET pin. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + Primary MIPI port for MIPI input + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: true + + required: + - data-lanes + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: | + HDMI port, should be connected to a node compatible with the + hdmi-connector binding. + + required: + - port@0 + - port@1 + +required: + - compatible + - reg + - reset-gpios + - ports + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c4 { + #address-cells = <1>; + #size-cells = <0>; + + hdmi-bridge@48 { + compatible = "lontium,lt8912"; + reg = <0x48>; + reset-gpios = <&max7323 0 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + hdmi_out_in: endpoint { + data-lanes = <0 1 2 3>; + remote-endpoint = <&mipi_dsi_out>; + }; + }; + + port@1 { + reg = <1>; + + endpoint { + remote-endpoint = <&hdmi_in>; + }; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index c38651ca59a5..f010b485aa7c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10440,6 +10440,11 @@ S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git F: drivers/hid/hid-lg-g15.c +LONTIUM LT8912 MIPI TO HDMI BRIDGE +M: Adrien Grassein <adrien.grassein@gmail.com> +S: Maintained +F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml + LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> M: Sreekanth Reddy <sreekanth.reddy@broadcom.com> -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 2021-02-14 23:29 ` [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 Adrien Grassein @ 2021-02-17 20:24 ` Rob Herring 2021-02-17 22:16 ` Rob Herring 1 sibling, 0 replies; 8+ messages in thread From: Rob Herring @ 2021-02-17 20:24 UTC (permalink / raw) To: Adrien Grassein Cc: linux-kernel, jonas, dri-devel, robh+dt, devicetree, narmstrong, jernej.skrabec, a.hajda, Laurent.pinchart, airlied On Mon, 15 Feb 2021 00:29:03 +0100, Adrien Grassein wrote: > Lontium LT8912 is a DSI to HDMI bridge. > > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > --- > .../display/bridge/lontium,lt8912.yaml | 102 ++++++++++++++++++ > MAINTAINERS | 5 + > 2 files changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/lontium,lt8912.example.dt.yaml' Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/media/video-interfaces.yaml' make[1]: *** [scripts/Makefile.lib:344: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.example.dt.yaml] Error 255 make: *** [Makefile:1370: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1440379 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 2021-02-14 23:29 ` [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 Adrien Grassein 2021-02-17 20:24 ` Rob Herring @ 2021-02-17 22:16 ` Rob Herring 1 sibling, 0 replies; 8+ messages in thread From: Rob Herring @ 2021-02-17 22:16 UTC (permalink / raw) To: Adrien Grassein Cc: airlied, daniel, a.hajda, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, dri-devel, devicetree, linux-kernel On Mon, Feb 15, 2021 at 12:29:03AM +0100, Adrien Grassein wrote: > Lontium LT8912 is a DSI to HDMI bridge. > > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > --- > .../display/bridge/lontium,lt8912.yaml | 102 ++++++++++++++++++ > MAINTAINERS | 5 + > 2 files changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml Other than needing to wait for 5.12-rc1 to be merged into drm-misc, Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge 2021-02-14 23:29 [PATCH v2 0/2] Add support of Lontium lt8912 MIPI to HDMI bridge Adrien Grassein 2021-02-14 23:29 ` [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 Adrien Grassein @ 2021-02-14 23:29 ` Adrien Grassein 2021-02-17 17:32 ` Robert Foss 1 sibling, 1 reply; 8+ messages in thread From: Adrien Grassein @ 2021-02-14 23:29 UTC (permalink / raw) Cc: airlied, daniel, a.hajda, robh+dt, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, dri-devel, devicetree, linux-kernel, Adrien Grassein Lontium Lt8912 is a DSI to HDMI bridge. Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> --- MAINTAINERS | 1 + drivers/gpu/drm/bridge/Kconfig | 14 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lontium-lt8912.c | 719 ++++++++++++++++++++++++ 4 files changed, 735 insertions(+) create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912.c diff --git a/MAINTAINERS b/MAINTAINERS index f010b485aa7c..8b3d64b689c7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10444,6 +10444,7 @@ LONTIUM LT8912 MIPI TO HDMI BRIDGE M: Adrien Grassein <adrien.grassein@gmail.com> S: Maintained F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml +F: drivers/gpu/drm/bridge/lontium-lt8912.c LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index e4110d6ca7b3..5b36d4b86e3c 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -48,6 +48,20 @@ config DRM_DISPLAY_CONNECTOR on ARM-based platforms. Saying Y here when this driver is not needed will not cause any issue. +config DRM_LONTIUM_LT8912 + tristate "Lontium LT8912 DSI/HDMI bridge" + depends on OF + select DRM_PANEL_BRIDGE + select DRM_KMS_HELPER + select REGMAP_I2C + help + Driver for Lontium LT8912 DSI to HDMI bridge + chip driver. + Please say Y if you have such hardware. + + Say M here if you want to support this hardware as a module. + The module will be named "lontium-lt8912". + config DRM_LONTIUM_LT9611 tristate "Lontium LT9611 DSI/HDMI bridge" select SND_SOC_HDMI_CODEC if SND_SOC diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 86e7acc76f8d..5a1b201cea1f 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o +obj-$(CONFIG_DRM_LONTIUM_LT8912) += lontium-lt8912.o obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o diff --git a/drivers/gpu/drm/bridge/lontium-lt8912.c b/drivers/gpu/drm/bridge/lontium-lt8912.c new file mode 100644 index 000000000000..d713d3365c3e --- /dev/null +++ b/drivers/gpu/drm/bridge/lontium-lt8912.c @@ -0,0 +1,719 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/device.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/gpio.h> +#include <linux/of_gpio.h> +#include <linux/regmap.h> + +#include <drm/drm_probe_helper.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h> + +#include <video/videomode.h> + +#define I2C_MAIN 0 +#define I2C_ADDR_MAIN 0x48 + +#define I2C_CEC_DSI 1 +#define I2C_ADDR_CEC_DSI 0x49 + +#define I2C_MAX_IDX 2 + +#define HPD_WORK_DELAY_MS 1000 + +struct lt8912 { + struct device *dev; + struct drm_bridge bridge; + struct drm_connector connector; + + struct i2c_client *i2c_client[I2C_MAX_IDX]; + struct regmap *regmap[I2C_MAX_IDX]; + + struct device_node *host_node; + struct drm_bridge *hdmi_port; + + struct mipi_dsi_device *dsi; + + struct gpio_desc *gp_reset; + + struct videomode mode; + + u8 data_lanes; + bool is_power_on; + bool is_attached; +}; + +static int lt8912_write_init_config(struct lt8912 *lt) +{ + const struct reg_sequence seq[] = { + /* Digital clock en*/ + {0x08, 0xff}, + {0x09, 0xff}, + {0x0a, 0xff}, + {0x0b, 0x7c}, + {0x0c, 0xff}, + {0x42, 0x04}, + + /*Tx Analog*/ + {0x31, 0xb1}, + {0x32, 0xb1}, + {0x33, 0x0e}, + {0x37, 0x00}, + {0x38, 0x22}, + {0x60, 0x82}, + + /*Cbus Analog*/ + {0x39, 0x45}, + {0x3a, 0x00}, + {0x3b, 0x00}, + + /*HDMI Pll Analog*/ + {0x44, 0x31}, + {0x55, 0x44}, + {0x57, 0x01}, + {0x5a, 0x02}, + + /*MIPI Analog*/ + {0x3e, 0xd6}, + {0x3f, 0xd4}, + {0x41, 0x3c}, + {0xB2, 0x00}, + }; + + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq)); +} + +static int lt8912_write_mipi_basic_config(struct lt8912 *lt) +{ + const struct reg_sequence seq[] = { + {0x12, 0x04}, + {0x14, 0x00}, + {0x15, 0x00}, + {0x1a, 0x03}, + {0x1b, 0x03}, + }; + + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); +}; + +static int lt8912_write_dds_config(struct lt8912 *lt) +{ + const struct reg_sequence seq[] = { + {0x4e, 0xff}, + {0x4f, 0x56}, + {0x50, 0x69}, + {0x51, 0x80}, + {0x1f, 0x5e}, + {0x20, 0x01}, + {0x21, 0x2c}, + {0x22, 0x01}, + {0x23, 0xfa}, + {0x24, 0x00}, + {0x25, 0xc8}, + {0x26, 0x00}, + {0x27, 0x5e}, + {0x28, 0x01}, + {0x29, 0x2c}, + {0x2a, 0x01}, + {0x2b, 0xfa}, + {0x2c, 0x00}, + {0x2d, 0xc8}, + {0x2e, 0x00}, + {0x42, 0x64}, + {0x43, 0x00}, + {0x44, 0x04}, + {0x45, 0x00}, + {0x46, 0x59}, + {0x47, 0x00}, + {0x48, 0xf2}, + {0x49, 0x06}, + {0x4a, 0x00}, + {0x4b, 0x72}, + {0x4c, 0x45}, + {0x4d, 0x00}, + {0x52, 0x08}, + {0x53, 0x00}, + {0x54, 0xb2}, + {0x55, 0x00}, + {0x56, 0xe4}, + {0x57, 0x0d}, + {0x58, 0x00}, + {0x59, 0xe4}, + {0x5a, 0x8a}, + {0x5b, 0x00}, + {0x5c, 0x34}, + {0x1e, 0x4f}, + {0x51, 0x00}, + }; + + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); +} + +static int lt8912_write_rxlogicres_config(struct lt8912 *lt) +{ + int ret; + + ret = regmap_write(lt->regmap[I2C_MAIN], 0x03, 0x7f); + usleep_range(10000, 20000); + ret |= regmap_write(lt->regmap[I2C_MAIN], 0x03, 0xff); + + return ret; +}; + +static int lt8912_write_lvds_config(struct lt8912 *lt) +{ + const struct reg_sequence seq[] = { + {0x44, 0x30}, + {0x51, 0x05}, + {0x50, 0x24}, + {0x51, 0x2d}, + {0x52, 0x04}, + {0x69, 0x0e}, + {0x69, 0x8e}, + {0x6a, 0x00}, + {0x6c, 0xb8}, + {0x6b, 0x51}, + {0x04, 0xfb}, + {0x04, 0xff}, + {0x7f, 0x00}, + {0xa8, 0x13}, + {0x02, 0xf7}, + {0x02, 0xff}, + {0x03, 0xcf}, + {0x03, 0xff}, + }; + + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); +}; + +static inline struct lt8912 *bridge_to_lt8912(struct drm_bridge *b) +{ + return container_of(b, struct lt8912, bridge); +} + +static inline struct lt8912 *connector_to_lt8912(struct drm_connector *c) +{ + return container_of(c, struct lt8912, connector); +} + +static const struct regmap_config lt8912_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0xff, +}; + +static int lt8912_init_i2c(struct lt8912 *lt) +{ + unsigned int i; + int ret; + struct i2c_board_info info[] = { + { I2C_BOARD_INFO("lt8912p0", I2C_ADDR_MAIN), }, + { I2C_BOARD_INFO("lt8912p1", I2C_ADDR_CEC_DSI), }, + }; + + if (!lt) + return -ENODEV; + + for (i = 0; i < ARRAY_SIZE(info); i++) { + if (i > 0) { + lt->i2c_client[i] = i2c_new_dummy_device(lt->i2c_client[0]->adapter, + info[i].addr); + if (!lt->i2c_client[i]) + return -ENODEV; + } + lt->regmap[i] = devm_regmap_init_i2c(lt->i2c_client[i], + <8912_regmap_config); + if (IS_ERR(lt->regmap[i])) { + ret = PTR_ERR(lt->regmap[i]); + return ret; + } + } + return 0; +} + +static int lt8912_free_i2c(struct lt8912 *lt) +{ + unsigned int i; + + for (i = 1; i < I2C_MAX_IDX; i++) + i2c_unregister_device(lt->i2c_client[i]); + + return 0; +} + +static int lt8912_hard_power_on(struct lt8912 *lt) +{ + gpiod_set_value_cansleep(lt->gp_reset, 0); + msleep(20); + + return 0; +} + +static void lt8912_hard_power_off(struct lt8912 *lt) +{ + gpiod_set_value_cansleep(lt->gp_reset, 1); + msleep(20); + lt->is_power_on = false; +} + +static int lt8912_video_setup(struct lt8912 *lt) +{ + u32 hactive, h_total, hpw, hfp, hbp; + u32 vactive, v_total, vpw, vfp, vbp; + u8 settle = 0x08; + int ret; + + if (!lt) + return -EINVAL; + + hactive = lt->mode.hactive; + hfp = lt->mode.hfront_porch; + hpw = lt->mode.hsync_len; + hbp = lt->mode.hback_porch; + h_total = hactive + hfp + hpw + hbp; + + vactive = lt->mode.vactive; + vfp = lt->mode.vfront_porch; + vpw = lt->mode.vsync_len; + vbp = lt->mode.vback_porch; + v_total = vactive + vfp + vpw + vbp; + + if (vactive <= 600) + settle = 0x04; + else if (vactive == 1080) + settle = 0x0a; + + ret = regmap_write(lt->regmap[I2C_CEC_DSI], 0x10, 0x01); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x11, settle); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x18, hpw); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x19, vpw); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1c, hactive & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1d, hactive >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x2f, 0x0c); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x34, h_total & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x35, h_total >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x36, v_total & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x37, v_total >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x38, vbp & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x39, vbp >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3a, vfp & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3b, vfp >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3c, hbp & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3d, hbp >> 8); + + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3e, hfp & 0xff); + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3f, hfp >> 8); + + return ret; +} + +static int lt8912_soft_power_on(struct lt8912 *lt) +{ + if (!lt->is_power_on) { + u32 lanes = lt->data_lanes; + + lt8912_write_init_config(lt); + regmap_write(lt->regmap[I2C_CEC_DSI], 0x13, lanes & 3); + + lt8912_write_mipi_basic_config(lt); + + lt->is_power_on = true; + } + + return 0; +} + +static int lt8912_video_on(struct lt8912 *lt) +{ + int ret; + + ret = lt8912_video_setup(lt); + if (ret < 0) + goto end; + + ret = lt8912_write_dds_config(lt); + if (ret < 0) + goto end; + + ret = lt8912_write_rxlogicres_config(lt); + if (ret < 0) + goto end; + + ret = lt8912_write_lvds_config(lt); + if (ret < 0) + goto end; + +end: + return ret; +} + +static enum drm_connector_status +lt8912_connector_detect(struct drm_connector *connector, bool force) +{ + struct lt8912 *lt = connector_to_lt8912(connector); + + return drm_bridge_detect(lt->hdmi_port); +} + +static const struct drm_connector_funcs lt8912_connector_funcs = { + .detect = lt8912_connector_detect, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static enum drm_mode_status +lt8912_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) +{ + if (mode->clock > 150000) + return MODE_CLOCK_HIGH; + + if (mode->hdisplay > 1920) + return MODE_BAD_HVALUE; + + if (mode->vdisplay > 1080) + return MODE_BAD_VVALUE; + + return MODE_OK; +} + +static int lt8912_connector_get_modes(struct drm_connector *connector) +{ + struct edid *edid; + int ret, num = 0; + struct lt8912 *lt = connector_to_lt8912(connector); + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + + edid = drm_bridge_get_edid(lt->hdmi_port, connector); + if (edid) { + drm_connector_update_edid_property(connector, edid); + num = drm_add_edid_modes(connector, edid); + } else + return ret; + + ret = drm_display_info_set_bus_formats(&connector->display_info, + &bus_format, 1); + if (ret) + return ret; + + return num; +} + +static const struct drm_connector_helper_funcs lt8912_connector_helper_funcs = { + .get_modes = lt8912_connector_get_modes, + .mode_valid = lt8912_connector_mode_valid, +}; + +static void lt8912_bridge_mode_set(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + const struct drm_display_mode *adj) +{ + struct lt8912 *lt = bridge_to_lt8912(bridge); + + drm_display_mode_to_videomode(adj, <->mode); +} + +static void lt8912_bridge_enable(struct drm_bridge *bridge) +{ + struct lt8912 *lt = bridge_to_lt8912(bridge); + + lt8912_video_on(lt); +} + +static int lt8912_attach_dsi(struct lt8912 *lt) +{ + struct device *dev = lt->dev; + struct mipi_dsi_host *host; + struct mipi_dsi_device *dsi; + int ret = -1; + const struct mipi_dsi_device_info info = { .type = "lt8912", + .channel = 0, + .node = NULL, + }; + + host = of_find_mipi_dsi_host_by_node(lt->host_node); + if (!host) { + dev_err(dev, "failed to find dsi host\n"); + return -EPROBE_DEFER; + } + + dsi = mipi_dsi_device_register_full(host, &info); + if (IS_ERR(dsi)) { + ret = PTR_ERR(dsi); + dev_err(dev, "failed to create dsi device (%d)\n", ret); + goto err_dsi_device; + } + + lt->dsi = dsi; + + dsi->lanes = lt->data_lanes; + dsi->format = MIPI_DSI_FMT_RGB888; + + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | + MIPI_DSI_MODE_VIDEO_BURST | + MIPI_DSI_MODE_LPM | + MIPI_DSI_MODE_EOT_PACKET; + + ret = mipi_dsi_attach(dsi); + if (ret < 0) { + dev_err(dev, "failed to attach dsi to host\n"); + goto err_dsi_attach; + } + + return 0; + +err_dsi_attach: + mipi_dsi_device_unregister(dsi); +err_dsi_device: + return ret; +} + +static void lt8912_detach_dsi(struct lt8912 *lt) +{ + mipi_dsi_detach(lt->dsi); + mipi_dsi_device_unregister(lt->dsi); +} + +static int lt8912_bridge_init(struct drm_bridge *bridge) +{ + int ret; + struct lt8912 *lt = bridge_to_lt8912(bridge); + struct drm_connector *connector = <->connector; + + + if (lt->hdmi_port->ops & DRM_BRIDGE_OP_DETECT) { + connector->polled = DRM_CONNECTOR_POLL_HPD; + } else { + connector->polled = DRM_CONNECTOR_POLL_CONNECT | + DRM_CONNECTOR_POLL_DISCONNECT; + } + + + ret = drm_connector_init(bridge->dev, connector, + <8912_connector_funcs, + lt->hdmi_port->type); + if (ret) + goto exit; + + drm_connector_helper_add(connector, <8912_connector_helper_funcs); + + connector->dpms = DRM_MODE_DPMS_OFF; + drm_connector_attach_encoder(connector, bridge->encoder); + + + ret = lt8912_hard_power_on(lt); + if (ret) + goto exit; + + ret = lt8912_soft_power_on(lt); + +exit: + return ret; +} + +static int lt8912_bridge_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct lt8912 *lt = bridge_to_lt8912(bridge); + int ret; + + /* No need to check the return of this function */ + drm_bridge_attach(lt->bridge.encoder, + lt->hdmi_port, + <->bridge, + flags); + + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { + ret = lt8912_bridge_init(bridge); + if (ret) { + dev_err(lt->dev, "Failed to init bridge ! (%d)\n", ret); + return ret; + } + } + + ret = lt8912_attach_dsi(lt); + if (ret) + return ret; + + lt->is_attached = true; + + return ret; +} + +static void lt8912_bridge_detach(struct drm_bridge *bridge) +{ + struct lt8912 *lt = bridge_to_lt8912(bridge); + + if (lt->is_attached) { + lt8912_detach_dsi(lt); + lt8912_hard_power_off(lt); + } +} + +static const struct drm_bridge_funcs lt8912_bridge_funcs = { + .attach = lt8912_bridge_attach, + .detach = lt8912_bridge_detach, + .mode_set = lt8912_bridge_mode_set, + .enable = lt8912_bridge_enable, +}; + +static int lt8912_parse_dt(struct lt8912 *lt) +{ + struct gpio_desc *gp_reset; + struct device *dev = lt->dev; + int ret = 0; + struct device_node *port_node; + struct device_node *endpoint; + + gp_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(gp_reset)) { + ret = PTR_ERR(gp_reset); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get reset gpio: %d\n", ret); + return ret; + } + lt->gp_reset = gp_reset; + + + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); + if (IS_ERR(endpoint)) { + ret = PTR_ERR(endpoint); + goto end; + } + + lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); + of_node_put(endpoint); + + lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); + if (!lt->host_node) { + dev_err(lt->dev, "%s: Failed to get remote port\n", __func__); + ret = -ENODEV; + goto end; + } + + port_node = of_graph_get_remote_node(dev->of_node, 1, -1); + if (!port_node) { + dev_err(lt->dev, "%s: Failed to get connector port\n", __func__); + ret = -ENODEV; + goto err_free_host_node; + } + + lt->hdmi_port = of_drm_find_bridge(port_node); + if (IS_ERR(lt->hdmi_port)) { + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); + ret = PTR_ERR(lt->hdmi_port); + of_node_put(lt->host_node); + goto end; + } + + if (!of_device_is_compatible(port_node, "hdmi-connector")) { + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); + ret = -EINVAL; + } + + of_node_put(port_node); + +end: + return ret; + +err_free_host_node: + of_node_put(lt->host_node); + return ret; +} + +static int lt8912_put_dt(struct lt8912 *lt) +{ + of_node_put(lt->host_node); + return 0; +} + +static int lt8912_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + static struct lt8912 *lt; + int ret = 0; + struct device *dev = &client->dev; + + lt = devm_kzalloc(dev, sizeof(struct lt8912), GFP_KERNEL); + if (!lt) + return -ENOMEM; + + lt->dev = dev; + lt->i2c_client[0] = client; + + ret = lt8912_parse_dt(lt); + if (ret) + goto err_dt_parse; + + ret = lt8912_init_i2c(lt); + if (ret) + goto err_i2c; + + i2c_set_clientdata(client, lt); + + lt->bridge.funcs = <8912_bridge_funcs; + lt->bridge.of_node = dev->of_node; + drm_bridge_add(<->bridge); + + return 0; + +err_i2c: + lt8912_put_dt(lt); +err_dt_parse: + return ret; +} + +static int lt8912_remove(struct i2c_client *client) +{ + struct lt8912 *lt = i2c_get_clientdata(client); + + lt8912_bridge_detach(<->bridge); + drm_bridge_remove(<->bridge); + lt8912_free_i2c(lt); + lt8912_put_dt(lt); + + return 0; +} + +static const struct of_device_id lt8912_dt_match[] = { + {.compatible = "lontium,lt8912"}, + {} +}; +MODULE_DEVICE_TABLE(of, lt8912_dt_match); + +static const struct i2c_device_id lt8912_id[] = { + {"lt8912", 0}, + {}, +}; +MODULE_DEVICE_TABLE(i2c, lt8912_id); + +static struct i2c_driver lt8912_i2c_driver = { + .driver = { + .name = "lt8912", + .of_match_table = lt8912_dt_match, + .owner = THIS_MODULE, + }, + .probe = lt8912_probe, + .remove = lt8912_remove, + .id_table = lt8912_id, +}; +module_i2c_driver(lt8912_i2c_driver); + +MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>"); +MODULE_DESCRIPTION("lt8912 drm driver"); +MODULE_LICENSE("GPL v2"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge 2021-02-14 23:29 ` [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge Adrien Grassein @ 2021-02-17 17:32 ` Robert Foss 2021-02-18 20:35 ` Adrien Grassein 0 siblings, 1 reply; 8+ messages in thread From: Robert Foss @ 2021-02-17 17:32 UTC (permalink / raw) To: Adrien Grassein Cc: David Airlie, Daniel Vetter, Andrzej Hajda, Rob Herring, Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, dri-devel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel Hey Adrien, Thanks for submitting this! On Mon, 15 Feb 2021 at 00:31, Adrien Grassein <adrien.grassein@gmail.com> wrote: > > Lontium Lt8912 is a DSI to HDMI bridge. > > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > --- > MAINTAINERS | 1 + > drivers/gpu/drm/bridge/Kconfig | 14 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/lontium-lt8912.c | 719 ++++++++++++++++++++++++ > 4 files changed, 735 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index f010b485aa7c..8b3d64b689c7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10444,6 +10444,7 @@ LONTIUM LT8912 MIPI TO HDMI BRIDGE > M: Adrien Grassein <adrien.grassein@gmail.com> > S: Maintained > F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml > +F: drivers/gpu/drm/bridge/lontium-lt8912.c > > LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) > M: Sathya Prakash <sathya.prakash@broadcom.com> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index e4110d6ca7b3..5b36d4b86e3c 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -48,6 +48,20 @@ config DRM_DISPLAY_CONNECTOR > on ARM-based platforms. Saying Y here when this driver is not needed > will not cause any issue. > > +config DRM_LONTIUM_LT8912 > + tristate "Lontium LT8912 DSI/HDMI bridge" > + depends on OF > + select DRM_PANEL_BRIDGE > + select DRM_KMS_HELPER > + select REGMAP_I2C > + help > + Driver for Lontium LT8912 DSI to HDMI bridge > + chip driver. > + Please say Y if you have such hardware. > + > + Say M here if you want to support this hardware as a module. > + The module will be named "lontium-lt8912". > + > config DRM_LONTIUM_LT9611 > tristate "Lontium LT9611 DSI/HDMI bridge" > select SND_SOC_HDMI_CODEC if SND_SOC > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 86e7acc76f8d..5a1b201cea1f 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -2,6 +2,7 @@ > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > +obj-$(CONFIG_DRM_LONTIUM_LT8912) += lontium-lt8912.o > obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o > obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o > obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > diff --git a/drivers/gpu/drm/bridge/lontium-lt8912.c b/drivers/gpu/drm/bridge/lontium-lt8912.c > new file mode 100644 > index 000000000000..d713d3365c3e > --- /dev/null > +++ b/drivers/gpu/drm/bridge/lontium-lt8912.c > @@ -0,0 +1,719 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/device.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/gpio.h> > +#include <linux/of_gpio.h> > +#include <linux/regmap.h> > + > +#include <drm/drm_probe_helper.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_mipi_dsi.h> > +#include <drm/drm_of.h> > + > +#include <video/videomode.h> > + > +#define I2C_MAIN 0 > +#define I2C_ADDR_MAIN 0x48 > + > +#define I2C_CEC_DSI 1 > +#define I2C_ADDR_CEC_DSI 0x49 > + > +#define I2C_MAX_IDX 2 > + > +#define HPD_WORK_DELAY_MS 1000 > + > +struct lt8912 { > + struct device *dev; > + struct drm_bridge bridge; > + struct drm_connector connector; > + > + struct i2c_client *i2c_client[I2C_MAX_IDX]; > + struct regmap *regmap[I2C_MAX_IDX]; > + > + struct device_node *host_node; > + struct drm_bridge *hdmi_port; > + > + struct mipi_dsi_device *dsi; > + > + struct gpio_desc *gp_reset; > + > + struct videomode mode; > + > + u8 data_lanes; > + bool is_power_on; > + bool is_attached; > +}; > + > +static int lt8912_write_init_config(struct lt8912 *lt) > +{ For the following register accesses it would be nice to see the registers named and the value written to them documented. If you don't have access to all of that information, not adding it is acceptable, but it will make the driver harder to maintain & extend going forward. So if possible, document registers in the same way it was done for lt9611 [1]. [1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/lontium-lt9611.c#L444 > + const struct reg_sequence seq[] = { > + /* Digital clock en*/ > + {0x08, 0xff}, > + {0x09, 0xff}, > + {0x0a, 0xff}, > + {0x0b, 0x7c}, > + {0x0c, 0xff}, > + {0x42, 0x04}, > + > + /*Tx Analog*/ > + {0x31, 0xb1}, > + {0x32, 0xb1}, > + {0x33, 0x0e}, > + {0x37, 0x00}, > + {0x38, 0x22}, > + {0x60, 0x82}, > + > + /*Cbus Analog*/ > + {0x39, 0x45}, > + {0x3a, 0x00}, > + {0x3b, 0x00}, > + > + /*HDMI Pll Analog*/ > + {0x44, 0x31}, > + {0x55, 0x44}, > + {0x57, 0x01}, > + {0x5a, 0x02}, > + > + /*MIPI Analog*/ > + {0x3e, 0xd6}, > + {0x3f, 0xd4}, > + {0x41, 0x3c}, > + {0xB2, 0x00}, > + }; > + > + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq)); > +} > + > +static int lt8912_write_mipi_basic_config(struct lt8912 *lt) > +{ > + const struct reg_sequence seq[] = { > + {0x12, 0x04}, > + {0x14, 0x00}, > + {0x15, 0x00}, > + {0x1a, 0x03}, > + {0x1b, 0x03}, > + }; > + > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > +}; > + > +static int lt8912_write_dds_config(struct lt8912 *lt) > +{ > + const struct reg_sequence seq[] = { > + {0x4e, 0xff}, > + {0x4f, 0x56}, > + {0x50, 0x69}, > + {0x51, 0x80}, > + {0x1f, 0x5e}, > + {0x20, 0x01}, > + {0x21, 0x2c}, > + {0x22, 0x01}, > + {0x23, 0xfa}, > + {0x24, 0x00}, > + {0x25, 0xc8}, > + {0x26, 0x00}, > + {0x27, 0x5e}, > + {0x28, 0x01}, > + {0x29, 0x2c}, > + {0x2a, 0x01}, > + {0x2b, 0xfa}, > + {0x2c, 0x00}, > + {0x2d, 0xc8}, > + {0x2e, 0x00}, > + {0x42, 0x64}, > + {0x43, 0x00}, > + {0x44, 0x04}, > + {0x45, 0x00}, > + {0x46, 0x59}, > + {0x47, 0x00}, > + {0x48, 0xf2}, > + {0x49, 0x06}, > + {0x4a, 0x00}, > + {0x4b, 0x72}, > + {0x4c, 0x45}, > + {0x4d, 0x00}, > + {0x52, 0x08}, > + {0x53, 0x00}, > + {0x54, 0xb2}, > + {0x55, 0x00}, > + {0x56, 0xe4}, > + {0x57, 0x0d}, > + {0x58, 0x00}, > + {0x59, 0xe4}, > + {0x5a, 0x8a}, > + {0x5b, 0x00}, > + {0x5c, 0x34}, > + {0x1e, 0x4f}, > + {0x51, 0x00}, > + }; > + > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > +} > + > +static int lt8912_write_rxlogicres_config(struct lt8912 *lt) > +{ > + int ret; > + > + ret = regmap_write(lt->regmap[I2C_MAIN], 0x03, 0x7f); > + usleep_range(10000, 20000); > + ret |= regmap_write(lt->regmap[I2C_MAIN], 0x03, 0xff); > + > + return ret; > +}; > + > +static int lt8912_write_lvds_config(struct lt8912 *lt) > +{ > + const struct reg_sequence seq[] = { > + {0x44, 0x30}, > + {0x51, 0x05}, > + {0x50, 0x24}, > + {0x51, 0x2d}, > + {0x52, 0x04}, > + {0x69, 0x0e}, > + {0x69, 0x8e}, > + {0x6a, 0x00}, > + {0x6c, 0xb8}, > + {0x6b, 0x51}, > + {0x04, 0xfb}, > + {0x04, 0xff}, > + {0x7f, 0x00}, > + {0xa8, 0x13}, > + {0x02, 0xf7}, > + {0x02, 0xff}, > + {0x03, 0xcf}, > + {0x03, 0xff}, > + }; > + > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > +}; > + > +static inline struct lt8912 *bridge_to_lt8912(struct drm_bridge *b) > +{ > + return container_of(b, struct lt8912, bridge); > +} > + > +static inline struct lt8912 *connector_to_lt8912(struct drm_connector *c) > +{ > + return container_of(c, struct lt8912, connector); > +} > + > +static const struct regmap_config lt8912_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0xff, > +}; > + > +static int lt8912_init_i2c(struct lt8912 *lt) > +{ > + unsigned int i; > + int ret; > + struct i2c_board_info info[] = { > + { I2C_BOARD_INFO("lt8912p0", I2C_ADDR_MAIN), }, > + { I2C_BOARD_INFO("lt8912p1", I2C_ADDR_CEC_DSI), }, > + }; > + > + if (!lt) > + return -ENODEV; > + > + for (i = 0; i < ARRAY_SIZE(info); i++) { > + if (i > 0) { > + lt->i2c_client[i] = i2c_new_dummy_device(lt->i2c_client[0]->adapter, > + info[i].addr); Bad indentation. Some of these bad indentations look ok for tab-width=4, however tab-width=8 is what the style guide dictates. > + if (!lt->i2c_client[i]) > + return -ENODEV; > + } > + lt->regmap[i] = devm_regmap_init_i2c(lt->i2c_client[i], > + <8912_regmap_config); Bad indentation. > + if (IS_ERR(lt->regmap[i])) { > + ret = PTR_ERR(lt->regmap[i]); > + return ret; > + } > + } > + return 0; > +} > + > +static int lt8912_free_i2c(struct lt8912 *lt) > +{ > + unsigned int i; > + > + for (i = 1; i < I2C_MAX_IDX; i++) > + i2c_unregister_device(lt->i2c_client[i]); > + > + return 0; > +} > + > +static int lt8912_hard_power_on(struct lt8912 *lt) > +{ > + gpiod_set_value_cansleep(lt->gp_reset, 0); > + msleep(20); > + > + return 0; > +} > + > +static void lt8912_hard_power_off(struct lt8912 *lt) > +{ > + gpiod_set_value_cansleep(lt->gp_reset, 1); > + msleep(20); > + lt->is_power_on = false; > +} > + > +static int lt8912_video_setup(struct lt8912 *lt) > +{ > + u32 hactive, h_total, hpw, hfp, hbp; > + u32 vactive, v_total, vpw, vfp, vbp; > + u8 settle = 0x08; > + int ret; > + > + if (!lt) > + return -EINVAL; > + > + hactive = lt->mode.hactive; > + hfp = lt->mode.hfront_porch; > + hpw = lt->mode.hsync_len; > + hbp = lt->mode.hback_porch; > + h_total = hactive + hfp + hpw + hbp; > + > + vactive = lt->mode.vactive; > + vfp = lt->mode.vfront_porch; > + vpw = lt->mode.vsync_len; > + vbp = lt->mode.vback_porch; > + v_total = vactive + vfp + vpw + vbp; > + > + if (vactive <= 600) > + settle = 0x04; > + else if (vactive == 1080) > + settle = 0x0a; > + > + ret = regmap_write(lt->regmap[I2C_CEC_DSI], 0x10, 0x01); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x11, settle); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x18, hpw); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x19, vpw); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1c, hactive & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1d, hactive >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x2f, 0x0c); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x34, h_total & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x35, h_total >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x36, v_total & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x37, v_total >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x38, vbp & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x39, vbp >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3a, vfp & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3b, vfp >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3c, hbp & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3d, hbp >> 8); > + > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3e, hfp & 0xff); > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3f, hfp >> 8); > + > + return ret; > +} > + > +static int lt8912_soft_power_on(struct lt8912 *lt) > +{ > + if (!lt->is_power_on) { > + u32 lanes = lt->data_lanes; > + > + lt8912_write_init_config(lt); > + regmap_write(lt->regmap[I2C_CEC_DSI], 0x13, lanes & 3); > + > + lt8912_write_mipi_basic_config(lt); > + > + lt->is_power_on = true; > + } > + > + return 0; > +} > + > +static int lt8912_video_on(struct lt8912 *lt) > +{ > + int ret; > + > + ret = lt8912_video_setup(lt); > + if (ret < 0) > + goto end; > + > + ret = lt8912_write_dds_config(lt); > + if (ret < 0) > + goto end; > + > + ret = lt8912_write_rxlogicres_config(lt); > + if (ret < 0) > + goto end; > + > + ret = lt8912_write_lvds_config(lt); > + if (ret < 0) > + goto end; > + > +end: > + return ret; > +} > + > +static enum drm_connector_status > +lt8912_connector_detect(struct drm_connector *connector, bool force) > +{ > + struct lt8912 *lt = connector_to_lt8912(connector); > + > + return drm_bridge_detect(lt->hdmi_port); > +} > + > +static const struct drm_connector_funcs lt8912_connector_funcs = { > + .detect = lt8912_connector_detect, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = drm_connector_cleanup, > + .reset = drm_atomic_helper_connector_reset, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > +static enum drm_mode_status > +lt8912_connector_mode_valid(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + if (mode->clock > 150000) > + return MODE_CLOCK_HIGH; > + > + if (mode->hdisplay > 1920) > + return MODE_BAD_HVALUE; > + > + if (mode->vdisplay > 1080) > + return MODE_BAD_VVALUE; > + > + return MODE_OK; > +} > + > +static int lt8912_connector_get_modes(struct drm_connector *connector) > +{ > + struct edid *edid; > + int ret, num = 0; > + struct lt8912 *lt = connector_to_lt8912(connector); > + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + > + edid = drm_bridge_get_edid(lt->hdmi_port, connector); Memory leak, edid is never free'd. > + if (edid) { > + drm_connector_update_edid_property(connector, edid); > + num = drm_add_edid_modes(connector, edid); > + } else > + return ret; > + > + ret = drm_display_info_set_bus_formats(&connector->display_info, > + &bus_format, 1); Bad indentation. > + if (ret) > + return ret; > + > + return num; > +} > + > +static const struct drm_connector_helper_funcs lt8912_connector_helper_funcs = { > + .get_modes = lt8912_connector_get_modes, > + .mode_valid = lt8912_connector_mode_valid, > +}; > + > +static void lt8912_bridge_mode_set(struct drm_bridge *bridge, > + const struct drm_display_mode *mode, > + const struct drm_display_mode *adj) > +{ > + struct lt8912 *lt = bridge_to_lt8912(bridge); > + > + drm_display_mode_to_videomode(adj, <->mode); > +} > + > +static void lt8912_bridge_enable(struct drm_bridge *bridge) > +{ > + struct lt8912 *lt = bridge_to_lt8912(bridge); > + > + lt8912_video_on(lt); > +} > + > +static int lt8912_attach_dsi(struct lt8912 *lt) > +{ > + struct device *dev = lt->dev; > + struct mipi_dsi_host *host; > + struct mipi_dsi_device *dsi; > + int ret = -1; > + const struct mipi_dsi_device_info info = { .type = "lt8912", > + .channel = 0, > + .node = NULL, > + }; > + > + host = of_find_mipi_dsi_host_by_node(lt->host_node); > + if (!host) { > + dev_err(dev, "failed to find dsi host\n"); > + return -EPROBE_DEFER; > + } > + > + dsi = mipi_dsi_device_register_full(host, &info); > + if (IS_ERR(dsi)) { > + ret = PTR_ERR(dsi); > + dev_err(dev, "failed to create dsi device (%d)\n", ret); > + goto err_dsi_device; > + } > + > + lt->dsi = dsi; > + > + dsi->lanes = lt->data_lanes; > + dsi->format = MIPI_DSI_FMT_RGB888; > + > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > + MIPI_DSI_MODE_VIDEO_BURST | > + MIPI_DSI_MODE_LPM | > + MIPI_DSI_MODE_EOT_PACKET; Maybe align the 3 last lines with where the 1st one starts. > + > + ret = mipi_dsi_attach(dsi); > + if (ret < 0) { > + dev_err(dev, "failed to attach dsi to host\n"); > + goto err_dsi_attach; > + } > + > + return 0; > + > +err_dsi_attach: > + mipi_dsi_device_unregister(dsi); > +err_dsi_device: > + return ret; > +} > + > +static void lt8912_detach_dsi(struct lt8912 *lt) > +{ > + mipi_dsi_detach(lt->dsi); > + mipi_dsi_device_unregister(lt->dsi); > +} > + > +static int lt8912_bridge_init(struct drm_bridge *bridge) > +{ > + int ret; > + struct lt8912 *lt = bridge_to_lt8912(bridge); > + struct drm_connector *connector = <->connector; > + > + > + if (lt->hdmi_port->ops & DRM_BRIDGE_OP_DETECT) { > + connector->polled = DRM_CONNECTOR_POLL_HPD; > + } else { > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | > + DRM_CONNECTOR_POLL_DISCONNECT; Bad indentation. > + } > + > + > + ret = drm_connector_init(bridge->dev, connector, > + <8912_connector_funcs, > + lt->hdmi_port->type); > + if (ret) > + goto exit; > + > + drm_connector_helper_add(connector, <8912_connector_helper_funcs); > + > + connector->dpms = DRM_MODE_DPMS_OFF; > + drm_connector_attach_encoder(connector, bridge->encoder); > + > + > + ret = lt8912_hard_power_on(lt); > + if (ret) > + goto exit; > + > + ret = lt8912_soft_power_on(lt); > + > +exit: > + return ret; > +} > + > +static int lt8912_bridge_attach(struct drm_bridge *bridge, > + enum drm_bridge_attach_flags flags) Bad indentation. > +{ > + struct lt8912 *lt = bridge_to_lt8912(bridge); > + int ret; > + > + /* No need to check the return of this function */ > + drm_bridge_attach(lt->bridge.encoder, > + lt->hdmi_port, > + <->bridge, > + flags); Bad indentation. > + > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { > + ret = lt8912_bridge_init(bridge); > + if (ret) { > + dev_err(lt->dev, "Failed to init bridge ! (%d)\n", ret); > + return ret; > + } > + } > + > + ret = lt8912_attach_dsi(lt); > + if (ret) > + return ret; > + > + lt->is_attached = true; > + > + return ret; > +} > + > +static void lt8912_bridge_detach(struct drm_bridge *bridge) > +{ > + struct lt8912 *lt = bridge_to_lt8912(bridge); > + > + if (lt->is_attached) { > + lt8912_detach_dsi(lt); > + lt8912_hard_power_off(lt); > + } > +} > + > +static const struct drm_bridge_funcs lt8912_bridge_funcs = { > + .attach = lt8912_bridge_attach, > + .detach = lt8912_bridge_detach, > + .mode_set = lt8912_bridge_mode_set, > + .enable = lt8912_bridge_enable, Have a look at the documentation around in drm_bridge_funcs[1], some additional functions have to be implemented. I think you can look at lt9611[2] & lt9611uxc[3] for inspiration. [1] https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridge-operations [2] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611.c#L947 [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611uxc.c#L542 > +}; > + > +static int lt8912_parse_dt(struct lt8912 *lt) > +{ > + struct gpio_desc *gp_reset; > + struct device *dev = lt->dev; > + int ret = 0; > + struct device_node *port_node; > + struct device_node *endpoint; > + > + gp_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(gp_reset)) { > + ret = PTR_ERR(gp_reset); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to get reset gpio: %d\n", ret); > + return ret; > + } > + lt->gp_reset = gp_reset; > + > + > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); > + if (IS_ERR(endpoint)) { > + ret = PTR_ERR(endpoint); > + goto end; > + } > + > + lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > + of_node_put(endpoint); > + > + lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); > + if (!lt->host_node) { > + dev_err(lt->dev, "%s: Failed to get remote port\n", __func__); > + ret = -ENODEV; > + goto end; > + } > + > + port_node = of_graph_get_remote_node(dev->of_node, 1, -1); > + if (!port_node) { > + dev_err(lt->dev, "%s: Failed to get connector port\n", __func__); > + ret = -ENODEV; > + goto err_free_host_node; > + } > + > + lt->hdmi_port = of_drm_find_bridge(port_node); > + if (IS_ERR(lt->hdmi_port)) { > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > + ret = PTR_ERR(lt->hdmi_port); > + of_node_put(lt->host_node); > + goto end; > + } > + > + if (!of_device_is_compatible(port_node, "hdmi-connector")) { > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > + ret = -EINVAL; > + } > + > + of_node_put(port_node); > + > +end: > + return ret; > + > +err_free_host_node: > + of_node_put(lt->host_node); > + return ret; > +} > + > +static int lt8912_put_dt(struct lt8912 *lt) > +{ > + of_node_put(lt->host_node); > + return 0; > +} > + > +static int lt8912_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + static struct lt8912 *lt; > + int ret = 0; > + struct device *dev = &client->dev; > + > + lt = devm_kzalloc(dev, sizeof(struct lt8912), GFP_KERNEL); > + if (!lt) > + return -ENOMEM; > + > + lt->dev = dev; > + lt->i2c_client[0] = client; The i2c_client that is created from the DT address and passed into lt8912_probe will be overwritten by the i2c clients created in lt8912_init_i2c before it is ever used. A related issue is that this device has two I2C addresses, which doesn't fit super neatly into the I2C driver model[1]. Since both addresses can't be provided by the DT, method one doesn't fit this driver. Method 2 doesn't fit well either, since the I2C bus isn't internal to the device. And 3&4 are not applicable. I'd say the two options are #1 splitting this driver, #2 keeping it like it is. I'm leaning towards #2 since both addresses correspond to a physical chip. [1] https://www.kernel.org/doc/html/latest/i2c/instantiating-devices.html > + > + ret = lt8912_parse_dt(lt); > + if (ret) > + goto err_dt_parse; > + > + ret = lt8912_init_i2c(lt); > + if (ret) > + goto err_i2c; > + > + i2c_set_clientdata(client, lt); > + > + lt->bridge.funcs = <8912_bridge_funcs; > + lt->bridge.of_node = dev->of_node; > + drm_bridge_add(<->bridge); > + > + return 0; > + > +err_i2c: > + lt8912_put_dt(lt); > +err_dt_parse: > + return ret; > +} > + > +static int lt8912_remove(struct i2c_client *client) > +{ > + struct lt8912 *lt = i2c_get_clientdata(client); > + > + lt8912_bridge_detach(<->bridge); > + drm_bridge_remove(<->bridge); > + lt8912_free_i2c(lt); > + lt8912_put_dt(lt); > + > + return 0; > +} > + > +static const struct of_device_id lt8912_dt_match[] = { > + {.compatible = "lontium,lt8912"}, > + {} > +}; > +MODULE_DEVICE_TABLE(of, lt8912_dt_match); > + > +static const struct i2c_device_id lt8912_id[] = { > + {"lt8912", 0}, > + {}, > +}; > +MODULE_DEVICE_TABLE(i2c, lt8912_id); > + > +static struct i2c_driver lt8912_i2c_driver = { > + .driver = { > + .name = "lt8912", > + .of_match_table = lt8912_dt_match, > + .owner = THIS_MODULE, The .owner assignment is redundant as it is later re-assigned in[1]. module_i2c_driver -> i2c_add_driver -> i2c_register_driver [1] https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L1796 > + }, > + .probe = lt8912_probe, > + .remove = lt8912_remove, > + .id_table = lt8912_id, > +}; > +module_i2c_driver(lt8912_i2c_driver); > + > +MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>"); > +MODULE_DESCRIPTION("lt8912 drm driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge 2021-02-17 17:32 ` Robert Foss @ 2021-02-18 20:35 ` Adrien Grassein 2021-02-18 21:26 ` Robert Foss 0 siblings, 1 reply; 8+ messages in thread From: Adrien Grassein @ 2021-02-18 20:35 UTC (permalink / raw) To: Robert Foss Cc: David Airlie, Daniel Vetter, Andrzej Hajda, Rob Herring, Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, dri-devel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel Hey Robert, Thank you for your time and for your review. Le mer. 17 févr. 2021 à 18:32, Robert Foss <robert.foss@linaro.org> a écrit : > > Hey Adrien, > > Thanks for submitting this! > > On Mon, 15 Feb 2021 at 00:31, Adrien Grassein <adrien.grassein@gmail.com> wrote: > > > > Lontium Lt8912 is a DSI to HDMI bridge. > > > > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > > --- > > MAINTAINERS | 1 + > > drivers/gpu/drm/bridge/Kconfig | 14 + > > drivers/gpu/drm/bridge/Makefile | 1 + > > drivers/gpu/drm/bridge/lontium-lt8912.c | 719 ++++++++++++++++++++++++ > > 4 files changed, 735 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f010b485aa7c..8b3d64b689c7 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -10444,6 +10444,7 @@ LONTIUM LT8912 MIPI TO HDMI BRIDGE > > M: Adrien Grassein <adrien.grassein@gmail.com> > > S: Maintained > > F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml > > +F: drivers/gpu/drm/bridge/lontium-lt8912.c > > > > LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) > > M: Sathya Prakash <sathya.prakash@broadcom.com> > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > index e4110d6ca7b3..5b36d4b86e3c 100644 > > --- a/drivers/gpu/drm/bridge/Kconfig > > +++ b/drivers/gpu/drm/bridge/Kconfig > > @@ -48,6 +48,20 @@ config DRM_DISPLAY_CONNECTOR > > on ARM-based platforms. Saying Y here when this driver is not needed > > will not cause any issue. > > > > +config DRM_LONTIUM_LT8912 > > + tristate "Lontium LT8912 DSI/HDMI bridge" > > + depends on OF > > + select DRM_PANEL_BRIDGE > > + select DRM_KMS_HELPER > > + select REGMAP_I2C > > + help > > + Driver for Lontium LT8912 DSI to HDMI bridge > > + chip driver. > > + Please say Y if you have such hardware. > > + > > + Say M here if you want to support this hardware as a module. > > + The module will be named "lontium-lt8912". > > + > > config DRM_LONTIUM_LT9611 > > tristate "Lontium LT9611 DSI/HDMI bridge" > > select SND_SOC_HDMI_CODEC if SND_SOC > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > > index 86e7acc76f8d..5a1b201cea1f 100644 > > --- a/drivers/gpu/drm/bridge/Makefile > > +++ b/drivers/gpu/drm/bridge/Makefile > > @@ -2,6 +2,7 @@ > > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > > obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o > > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > > +obj-$(CONFIG_DRM_LONTIUM_LT8912) += lontium-lt8912.o > > obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o > > obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o > > obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > > diff --git a/drivers/gpu/drm/bridge/lontium-lt8912.c b/drivers/gpu/drm/bridge/lontium-lt8912.c > > new file mode 100644 > > index 000000000000..d713d3365c3e > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/lontium-lt8912.c > > @@ -0,0 +1,719 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > + */ > > + > > +#include <linux/device.h> > > +#include <linux/delay.h> > > +#include <linux/i2c.h> > > +#include <linux/gpio.h> > > +#include <linux/of_gpio.h> > > +#include <linux/regmap.h> > > + > > +#include <drm/drm_probe_helper.h> > > +#include <drm/drm_atomic_helper.h> > > +#include <drm/drm_mipi_dsi.h> > > +#include <drm/drm_of.h> > > + > > +#include <video/videomode.h> > > + > > +#define I2C_MAIN 0 > > +#define I2C_ADDR_MAIN 0x48 > > + > > +#define I2C_CEC_DSI 1 > > +#define I2C_ADDR_CEC_DSI 0x49 > > + > > +#define I2C_MAX_IDX 2 > > + > > +#define HPD_WORK_DELAY_MS 1000 > > + > > +struct lt8912 { > > + struct device *dev; > > + struct drm_bridge bridge; > > + struct drm_connector connector; > > + > > + struct i2c_client *i2c_client[I2C_MAX_IDX]; > > + struct regmap *regmap[I2C_MAX_IDX]; > > + > > + struct device_node *host_node; > > + struct drm_bridge *hdmi_port; > > + > > + struct mipi_dsi_device *dsi; > > + > > + struct gpio_desc *gp_reset; > > + > > + struct videomode mode; > > + > > + u8 data_lanes; > > + bool is_power_on; > > + bool is_attached; > > +}; > > + > > +static int lt8912_write_init_config(struct lt8912 *lt) > > +{ > > For the following register accesses it would be nice to see the > registers named and the value written to them documented. If you don't > have access to all of that information, not adding it is acceptable, > but it will make the driver harder to maintain & extend going forward. > > So if possible, document registers in the same way it was done for lt9611 [1]. > > [1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/lontium-lt9611.c#L444 > Sorry, I don't have any documentation about this chip. I can't document these registers. > > + const struct reg_sequence seq[] = { > > + /* Digital clock en*/ > > + {0x08, 0xff}, > > + {0x09, 0xff}, > > + {0x0a, 0xff}, > > + {0x0b, 0x7c}, > > + {0x0c, 0xff}, > > + {0x42, 0x04}, > > + > > + /*Tx Analog*/ > > + {0x31, 0xb1}, > > + {0x32, 0xb1}, > > + {0x33, 0x0e}, > > + {0x37, 0x00}, > > + {0x38, 0x22}, > > + {0x60, 0x82}, > > + > > + /*Cbus Analog*/ > > + {0x39, 0x45}, > > + {0x3a, 0x00}, > > + {0x3b, 0x00}, > > + > > + /*HDMI Pll Analog*/ > > + {0x44, 0x31}, > > + {0x55, 0x44}, > > + {0x57, 0x01}, > > + {0x5a, 0x02}, > > + > > + /*MIPI Analog*/ > > + {0x3e, 0xd6}, > > + {0x3f, 0xd4}, > > + {0x41, 0x3c}, > > + {0xB2, 0x00}, > > + }; > > + > > + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq)); > > +} > > + > > +static int lt8912_write_mipi_basic_config(struct lt8912 *lt) > > +{ > > + const struct reg_sequence seq[] = { > > + {0x12, 0x04}, > > + {0x14, 0x00}, > > + {0x15, 0x00}, > > + {0x1a, 0x03}, > > + {0x1b, 0x03}, > > + }; > > + > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > +}; > > + > > +static int lt8912_write_dds_config(struct lt8912 *lt) > > +{ > > + const struct reg_sequence seq[] = { > > + {0x4e, 0xff}, > > + {0x4f, 0x56}, > > + {0x50, 0x69}, > > + {0x51, 0x80}, > > + {0x1f, 0x5e}, > > + {0x20, 0x01}, > > + {0x21, 0x2c}, > > + {0x22, 0x01}, > > + {0x23, 0xfa}, > > + {0x24, 0x00}, > > + {0x25, 0xc8}, > > + {0x26, 0x00}, > > + {0x27, 0x5e}, > > + {0x28, 0x01}, > > + {0x29, 0x2c}, > > + {0x2a, 0x01}, > > + {0x2b, 0xfa}, > > + {0x2c, 0x00}, > > + {0x2d, 0xc8}, > > + {0x2e, 0x00}, > > + {0x42, 0x64}, > > + {0x43, 0x00}, > > + {0x44, 0x04}, > > + {0x45, 0x00}, > > + {0x46, 0x59}, > > + {0x47, 0x00}, > > + {0x48, 0xf2}, > > + {0x49, 0x06}, > > + {0x4a, 0x00}, > > + {0x4b, 0x72}, > > + {0x4c, 0x45}, > > + {0x4d, 0x00}, > > + {0x52, 0x08}, > > + {0x53, 0x00}, > > + {0x54, 0xb2}, > > + {0x55, 0x00}, > > + {0x56, 0xe4}, > > + {0x57, 0x0d}, > > + {0x58, 0x00}, > > + {0x59, 0xe4}, > > + {0x5a, 0x8a}, > > + {0x5b, 0x00}, > > + {0x5c, 0x34}, > > + {0x1e, 0x4f}, > > + {0x51, 0x00}, > > + }; > > + > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > +} > > + > > +static int lt8912_write_rxlogicres_config(struct lt8912 *lt) > > +{ > > + int ret; > > + > > + ret = regmap_write(lt->regmap[I2C_MAIN], 0x03, 0x7f); > > + usleep_range(10000, 20000); > > + ret |= regmap_write(lt->regmap[I2C_MAIN], 0x03, 0xff); > > + > > + return ret; > > +}; > > + > > +static int lt8912_write_lvds_config(struct lt8912 *lt) > > +{ > > + const struct reg_sequence seq[] = { > > + {0x44, 0x30}, > > + {0x51, 0x05}, > > + {0x50, 0x24}, > > + {0x51, 0x2d}, > > + {0x52, 0x04}, > > + {0x69, 0x0e}, > > + {0x69, 0x8e}, > > + {0x6a, 0x00}, > > + {0x6c, 0xb8}, > > + {0x6b, 0x51}, > > + {0x04, 0xfb}, > > + {0x04, 0xff}, > > + {0x7f, 0x00}, > > + {0xa8, 0x13}, > > + {0x02, 0xf7}, > > + {0x02, 0xff}, > > + {0x03, 0xcf}, > > + {0x03, 0xff}, > > + }; > > + > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > +}; > > + > > +static inline struct lt8912 *bridge_to_lt8912(struct drm_bridge *b) > > +{ > > + return container_of(b, struct lt8912, bridge); > > +} > > + > > +static inline struct lt8912 *connector_to_lt8912(struct drm_connector *c) > > +{ > > + return container_of(c, struct lt8912, connector); > > +} > > + > > +static const struct regmap_config lt8912_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = 0xff, > > +}; > > + > > +static int lt8912_init_i2c(struct lt8912 *lt) > > +{ > > + unsigned int i; > > + int ret; > > + struct i2c_board_info info[] = { > > + { I2C_BOARD_INFO("lt8912p0", I2C_ADDR_MAIN), }, > > + { I2C_BOARD_INFO("lt8912p1", I2C_ADDR_CEC_DSI), }, > > + }; > > + > > + if (!lt) > > + return -ENODEV; > > + > > + for (i = 0; i < ARRAY_SIZE(info); i++) { > > + if (i > 0) { > > + lt->i2c_client[i] = i2c_new_dummy_device(lt->i2c_client[0]->adapter, > > + info[i].addr); > > Bad indentation. Some of these bad indentations look ok for > tab-width=4, however tab-width=8 is what the style guide dictates. > > > + if (!lt->i2c_client[i]) > > + return -ENODEV; > > + } > > + lt->regmap[i] = devm_regmap_init_i2c(lt->i2c_client[i], > > + <8912_regmap_config); > > Bad indentation. > > > + if (IS_ERR(lt->regmap[i])) { > > + ret = PTR_ERR(lt->regmap[i]); > > + return ret; > > + } > > + } > > + return 0; > > +} > > + > > +static int lt8912_free_i2c(struct lt8912 *lt) > > +{ > > + unsigned int i; > > + > > + for (i = 1; i < I2C_MAX_IDX; i++) > > + i2c_unregister_device(lt->i2c_client[i]); > > + > > + return 0; > > +} > > + > > +static int lt8912_hard_power_on(struct lt8912 *lt) > > +{ > > + gpiod_set_value_cansleep(lt->gp_reset, 0); > > + msleep(20); > > + > > + return 0; > > +} > > + > > +static void lt8912_hard_power_off(struct lt8912 *lt) > > +{ > > + gpiod_set_value_cansleep(lt->gp_reset, 1); > > + msleep(20); > > + lt->is_power_on = false; > > +} > > + > > +static int lt8912_video_setup(struct lt8912 *lt) > > +{ > > + u32 hactive, h_total, hpw, hfp, hbp; > > + u32 vactive, v_total, vpw, vfp, vbp; > > + u8 settle = 0x08; > > + int ret; > > + > > + if (!lt) > > + return -EINVAL; > > + > > + hactive = lt->mode.hactive; > > + hfp = lt->mode.hfront_porch; > > + hpw = lt->mode.hsync_len; > > + hbp = lt->mode.hback_porch; > > + h_total = hactive + hfp + hpw + hbp; > > + > > + vactive = lt->mode.vactive; > > + vfp = lt->mode.vfront_porch; > > + vpw = lt->mode.vsync_len; > > + vbp = lt->mode.vback_porch; > > + v_total = vactive + vfp + vpw + vbp; > > + > > + if (vactive <= 600) > > + settle = 0x04; > > + else if (vactive == 1080) > > + settle = 0x0a; > > + > > + ret = regmap_write(lt->regmap[I2C_CEC_DSI], 0x10, 0x01); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x11, settle); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x18, hpw); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x19, vpw); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1c, hactive & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1d, hactive >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x2f, 0x0c); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x34, h_total & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x35, h_total >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x36, v_total & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x37, v_total >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x38, vbp & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x39, vbp >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3a, vfp & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3b, vfp >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3c, hbp & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3d, hbp >> 8); > > + > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3e, hfp & 0xff); > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3f, hfp >> 8); > > + > > + return ret; > > +} > > + > > +static int lt8912_soft_power_on(struct lt8912 *lt) > > +{ > > + if (!lt->is_power_on) { > > + u32 lanes = lt->data_lanes; > > + > > + lt8912_write_init_config(lt); > > + regmap_write(lt->regmap[I2C_CEC_DSI], 0x13, lanes & 3); > > + > > + lt8912_write_mipi_basic_config(lt); > > + > > + lt->is_power_on = true; > > + } > > + > > + return 0; > > +} > > + > > +static int lt8912_video_on(struct lt8912 *lt) > > +{ > > + int ret; > > + > > + ret = lt8912_video_setup(lt); > > + if (ret < 0) > > + goto end; > > + > > + ret = lt8912_write_dds_config(lt); > > + if (ret < 0) > > + goto end; > > + > > + ret = lt8912_write_rxlogicres_config(lt); > > + if (ret < 0) > > + goto end; > > + > > + ret = lt8912_write_lvds_config(lt); > > + if (ret < 0) > > + goto end; > > + > > +end: > > + return ret; > > +} > > + > > +static enum drm_connector_status > > +lt8912_connector_detect(struct drm_connector *connector, bool force) > > +{ > > + struct lt8912 *lt = connector_to_lt8912(connector); > > + > > + return drm_bridge_detect(lt->hdmi_port); > > +} > > + > > +static const struct drm_connector_funcs lt8912_connector_funcs = { > > + .detect = lt8912_connector_detect, > > + .fill_modes = drm_helper_probe_single_connector_modes, > > + .destroy = drm_connector_cleanup, > > + .reset = drm_atomic_helper_connector_reset, > > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > +}; > > + > > +static enum drm_mode_status > > +lt8912_connector_mode_valid(struct drm_connector *connector, > > + struct drm_display_mode *mode) > > +{ > > + if (mode->clock > 150000) > > + return MODE_CLOCK_HIGH; > > + > > + if (mode->hdisplay > 1920) > > + return MODE_BAD_HVALUE; > > + > > + if (mode->vdisplay > 1080) > > + return MODE_BAD_VVALUE; > > + > > + return MODE_OK; > > +} > > + > > +static int lt8912_connector_get_modes(struct drm_connector *connector) > > +{ > > + struct edid *edid; > > + int ret, num = 0; > > + struct lt8912 *lt = connector_to_lt8912(connector); > > + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > > + > > + edid = drm_bridge_get_edid(lt->hdmi_port, connector); > > Memory leak, edid is never free'd. > > > + if (edid) { > > + drm_connector_update_edid_property(connector, edid); > > + num = drm_add_edid_modes(connector, edid); > > + } else > > + return ret; > > + > > + ret = drm_display_info_set_bus_formats(&connector->display_info, > > + &bus_format, 1); > > Bad indentation. > > > + if (ret) > > + return ret; > > + > > + return num; > > +} > > + > > +static const struct drm_connector_helper_funcs lt8912_connector_helper_funcs = { > > + .get_modes = lt8912_connector_get_modes, > > + .mode_valid = lt8912_connector_mode_valid, > > +}; > > + > > +static void lt8912_bridge_mode_set(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode, > > + const struct drm_display_mode *adj) > > +{ > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > + > > + drm_display_mode_to_videomode(adj, <->mode); > > +} > > + > > +static void lt8912_bridge_enable(struct drm_bridge *bridge) > > +{ > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > + > > + lt8912_video_on(lt); > > +} > > + > > +static int lt8912_attach_dsi(struct lt8912 *lt) > > +{ > > + struct device *dev = lt->dev; > > + struct mipi_dsi_host *host; > > + struct mipi_dsi_device *dsi; > > + int ret = -1; > > + const struct mipi_dsi_device_info info = { .type = "lt8912", > > + .channel = 0, > > + .node = NULL, > > + }; > > + > > + host = of_find_mipi_dsi_host_by_node(lt->host_node); > > + if (!host) { > > + dev_err(dev, "failed to find dsi host\n"); > > + return -EPROBE_DEFER; > > + } > > + > > + dsi = mipi_dsi_device_register_full(host, &info); > > + if (IS_ERR(dsi)) { > > + ret = PTR_ERR(dsi); > > + dev_err(dev, "failed to create dsi device (%d)\n", ret); > > + goto err_dsi_device; > > + } > > + > > + lt->dsi = dsi; > > + > > + dsi->lanes = lt->data_lanes; > > + dsi->format = MIPI_DSI_FMT_RGB888; > > + > > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > + MIPI_DSI_MODE_VIDEO_BURST | > > + MIPI_DSI_MODE_LPM | > > + MIPI_DSI_MODE_EOT_PACKET; > > Maybe align the 3 last lines with where the 1st one starts. > > > + > > + ret = mipi_dsi_attach(dsi); > > + if (ret < 0) { > > + dev_err(dev, "failed to attach dsi to host\n"); > > + goto err_dsi_attach; > > + } > > + > > + return 0; > > + > > +err_dsi_attach: > > + mipi_dsi_device_unregister(dsi); > > +err_dsi_device: > > + return ret; > > +} > > + > > +static void lt8912_detach_dsi(struct lt8912 *lt) > > +{ > > + mipi_dsi_detach(lt->dsi); > > + mipi_dsi_device_unregister(lt->dsi); > > +} > > + > > +static int lt8912_bridge_init(struct drm_bridge *bridge) > > +{ > > + int ret; > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > + struct drm_connector *connector = <->connector; > > + > > + > > + if (lt->hdmi_port->ops & DRM_BRIDGE_OP_DETECT) { > > + connector->polled = DRM_CONNECTOR_POLL_HPD; > > + } else { > > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | > > + DRM_CONNECTOR_POLL_DISCONNECT; > > Bad indentation. > > > + } > > + > > + > > + ret = drm_connector_init(bridge->dev, connector, > > + <8912_connector_funcs, > > + lt->hdmi_port->type); > > + if (ret) > > + goto exit; > > + > > + drm_connector_helper_add(connector, <8912_connector_helper_funcs); > > + > > + connector->dpms = DRM_MODE_DPMS_OFF; > > + drm_connector_attach_encoder(connector, bridge->encoder); > > + > > + > > + ret = lt8912_hard_power_on(lt); > > + if (ret) > > + goto exit; > > + > > + ret = lt8912_soft_power_on(lt); > > + > > +exit: > > + return ret; > > +} > > + > > +static int lt8912_bridge_attach(struct drm_bridge *bridge, > > + enum drm_bridge_attach_flags flags) > > Bad indentation. > > > +{ > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > + int ret; > > + > > + /* No need to check the return of this function */ > > + drm_bridge_attach(lt->bridge.encoder, > > + lt->hdmi_port, > > + <->bridge, > > + flags); > > Bad indentation. > > > + > > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { > > + ret = lt8912_bridge_init(bridge); > > + if (ret) { > > + dev_err(lt->dev, "Failed to init bridge ! (%d)\n", ret); > > + return ret; > > + } > > + } > > + > > + ret = lt8912_attach_dsi(lt); > > + if (ret) > > + return ret; > > + > > + lt->is_attached = true; > > + > > + return ret; > > +} > > + > > +static void lt8912_bridge_detach(struct drm_bridge *bridge) > > +{ > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > + > > + if (lt->is_attached) { > > + lt8912_detach_dsi(lt); > > + lt8912_hard_power_off(lt); > > + } > > +} > > + > > +static const struct drm_bridge_funcs lt8912_bridge_funcs = { > > + .attach = lt8912_bridge_attach, > > + .detach = lt8912_bridge_detach, > > + .mode_set = lt8912_bridge_mode_set, > > + .enable = lt8912_bridge_enable, > > Have a look at the documentation around in drm_bridge_funcs[1], some > additional functions have to be implemented. I think you can look at > lt9611[2] & lt9611uxc[3] for inspiration. > > [1] https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridge-operations > [2] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611.c#L947 > [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611uxc.c#L542 > > > > +}; > > + > > +static int lt8912_parse_dt(struct lt8912 *lt) > > +{ > > + struct gpio_desc *gp_reset; > > + struct device *dev = lt->dev; > > + int ret = 0; > > + struct device_node *port_node; > > + struct device_node *endpoint; > > + > > + gp_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > + if (IS_ERR(gp_reset)) { > > + ret = PTR_ERR(gp_reset); > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, "Failed to get reset gpio: %d\n", ret); > > + return ret; > > + } > > + lt->gp_reset = gp_reset; > > + > > + > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); > > + if (IS_ERR(endpoint)) { > > + ret = PTR_ERR(endpoint); > > + goto end; > > + } > > + > > + lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > > + of_node_put(endpoint); > > + > > + lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); > > + if (!lt->host_node) { > > + dev_err(lt->dev, "%s: Failed to get remote port\n", __func__); > > + ret = -ENODEV; > > + goto end; > > + } > > + > > + port_node = of_graph_get_remote_node(dev->of_node, 1, -1); > > + if (!port_node) { > > + dev_err(lt->dev, "%s: Failed to get connector port\n", __func__); > > + ret = -ENODEV; > > + goto err_free_host_node; > > + } > > + > > + lt->hdmi_port = of_drm_find_bridge(port_node); > > + if (IS_ERR(lt->hdmi_port)) { > > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > > + ret = PTR_ERR(lt->hdmi_port); > > + of_node_put(lt->host_node); > > + goto end; > > + } > > + > > + if (!of_device_is_compatible(port_node, "hdmi-connector")) { > > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > > + ret = -EINVAL; > > + } > > + > > + of_node_put(port_node); > > + > > +end: > > + return ret; > > + > > +err_free_host_node: > > + of_node_put(lt->host_node); > > + return ret; > > +} > > + > > +static int lt8912_put_dt(struct lt8912 *lt) > > +{ > > + of_node_put(lt->host_node); > > + return 0; > > +} > > + > > +static int lt8912_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + static struct lt8912 *lt; > > + int ret = 0; > > + struct device *dev = &client->dev; > > + > > + lt = devm_kzalloc(dev, sizeof(struct lt8912), GFP_KERNEL); > > + if (!lt) > > + return -ENOMEM; > > + > > + lt->dev = dev; > > + lt->i2c_client[0] = client; > > The i2c_client that is created from the DT address and passed into > lt8912_probe will be overwritten by the i2c clients created in > lt8912_init_i2c before it is ever used. I think you made a mistake here. The lt8912_init_i2c function only initializes the lt->i2c_client[1]. (You can check the if condition). > > A related issue is that this device has two I2C addresses, which > doesn't fit super neatly into the I2C driver model[1]. Since both > addresses can't be provided by the DT, method one doesn't fit this > driver. Method 2 doesn't fit well either, since the I2C bus isn't > internal to the device. And 3&4 are not applicable. I'd say the two > options are #1 splitting this driver, #2 keeping it like it is. I'm > leaning towards #2 since both addresses correspond to a physical chip. > > [1] https://www.kernel.org/doc/html/latest/i2c/instantiating-devices.html > > > + > > + ret = lt8912_parse_dt(lt); > > + if (ret) > > + goto err_dt_parse; > > + > > + ret = lt8912_init_i2c(lt); > > + if (ret) > > + goto err_i2c; > > + > > + i2c_set_clientdata(client, lt); > > + > > + lt->bridge.funcs = <8912_bridge_funcs; > > + lt->bridge.of_node = dev->of_node; > > + drm_bridge_add(<->bridge); > > + > > + return 0; > > + > > +err_i2c: > > + lt8912_put_dt(lt); > > +err_dt_parse: > > + return ret; > > +} > > + > > +static int lt8912_remove(struct i2c_client *client) > > +{ > > + struct lt8912 *lt = i2c_get_clientdata(client); > > + > > + lt8912_bridge_detach(<->bridge); > > + drm_bridge_remove(<->bridge); > > + lt8912_free_i2c(lt); > > + lt8912_put_dt(lt); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id lt8912_dt_match[] = { > > + {.compatible = "lontium,lt8912"}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, lt8912_dt_match); > > + > > +static const struct i2c_device_id lt8912_id[] = { > > + {"lt8912", 0}, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(i2c, lt8912_id); > > + > > +static struct i2c_driver lt8912_i2c_driver = { > > + .driver = { > > + .name = "lt8912", > > + .of_match_table = lt8912_dt_match, > > + .owner = THIS_MODULE, > > The .owner assignment is redundant as it is later re-assigned in[1]. > > module_i2c_driver -> i2c_add_driver -> i2c_register_driver > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L1796 > > > + }, > > + .probe = lt8912_probe, > > + .remove = lt8912_remove, > > + .id_table = lt8912_id, > > +}; > > +module_i2c_driver(lt8912_i2c_driver); > > + > > +MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>"); > > +MODULE_DESCRIPTION("lt8912 drm driver"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 2.25.1 > > I will send a new version with the fixes. Thanks again, Adrien Grassein ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge 2021-02-18 20:35 ` Adrien Grassein @ 2021-02-18 21:26 ` Robert Foss 0 siblings, 0 replies; 8+ messages in thread From: Robert Foss @ 2021-02-18 21:26 UTC (permalink / raw) To: Adrien Grassein Cc: David Airlie, Daniel Vetter, Andrzej Hajda, Rob Herring, Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, dri-devel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Thu, 18 Feb 2021 at 21:36, Adrien Grassein <adrien.grassein@gmail.com> wrote: > > Hey Robert, > > Thank you for your time and for your review. > > Le mer. 17 févr. 2021 à 18:32, Robert Foss <robert.foss@linaro.org> a écrit : > > > > Hey Adrien, > > > > Thanks for submitting this! > > > > On Mon, 15 Feb 2021 at 00:31, Adrien Grassein <adrien.grassein@gmail.com> wrote: > > > > > > Lontium Lt8912 is a DSI to HDMI bridge. > > > > > > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> > > > --- > > > MAINTAINERS | 1 + > > > drivers/gpu/drm/bridge/Kconfig | 14 + > > > drivers/gpu/drm/bridge/Makefile | 1 + > > > drivers/gpu/drm/bridge/lontium-lt8912.c | 719 ++++++++++++++++++++++++ > > > 4 files changed, 735 insertions(+) > > > create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index f010b485aa7c..8b3d64b689c7 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -10444,6 +10444,7 @@ LONTIUM LT8912 MIPI TO HDMI BRIDGE > > > M: Adrien Grassein <adrien.grassein@gmail.com> > > > S: Maintained > > > F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml > > > +F: drivers/gpu/drm/bridge/lontium-lt8912.c > > > > > > LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) > > > M: Sathya Prakash <sathya.prakash@broadcom.com> > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > > index e4110d6ca7b3..5b36d4b86e3c 100644 > > > --- a/drivers/gpu/drm/bridge/Kconfig > > > +++ b/drivers/gpu/drm/bridge/Kconfig > > > @@ -48,6 +48,20 @@ config DRM_DISPLAY_CONNECTOR > > > on ARM-based platforms. Saying Y here when this driver is not needed > > > will not cause any issue. > > > > > > +config DRM_LONTIUM_LT8912 > > > + tristate "Lontium LT8912 DSI/HDMI bridge" > > > + depends on OF > > > + select DRM_PANEL_BRIDGE > > > + select DRM_KMS_HELPER > > > + select REGMAP_I2C > > > + help > > > + Driver for Lontium LT8912 DSI to HDMI bridge > > > + chip driver. > > > + Please say Y if you have such hardware. > > > + > > > + Say M here if you want to support this hardware as a module. > > > + The module will be named "lontium-lt8912". > > > + > > > config DRM_LONTIUM_LT9611 > > > tristate "Lontium LT9611 DSI/HDMI bridge" > > > select SND_SOC_HDMI_CODEC if SND_SOC > > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > > > index 86e7acc76f8d..5a1b201cea1f 100644 > > > --- a/drivers/gpu/drm/bridge/Makefile > > > +++ b/drivers/gpu/drm/bridge/Makefile > > > @@ -2,6 +2,7 @@ > > > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > > > obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o > > > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > > > +obj-$(CONFIG_DRM_LONTIUM_LT8912) += lontium-lt8912.o > > > obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o > > > obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o > > > obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > > > diff --git a/drivers/gpu/drm/bridge/lontium-lt8912.c b/drivers/gpu/drm/bridge/lontium-lt8912.c > > > new file mode 100644 > > > index 000000000000..d713d3365c3e > > > --- /dev/null > > > +++ b/drivers/gpu/drm/bridge/lontium-lt8912.c > > > @@ -0,0 +1,719 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > > + */ > > > + > > > +#include <linux/device.h> > > > +#include <linux/delay.h> > > > +#include <linux/i2c.h> > > > +#include <linux/gpio.h> > > > +#include <linux/of_gpio.h> > > > +#include <linux/regmap.h> > > > + > > > +#include <drm/drm_probe_helper.h> > > > +#include <drm/drm_atomic_helper.h> > > > +#include <drm/drm_mipi_dsi.h> > > > +#include <drm/drm_of.h> > > > + > > > +#include <video/videomode.h> > > > + > > > +#define I2C_MAIN 0 > > > +#define I2C_ADDR_MAIN 0x48 > > > + > > > +#define I2C_CEC_DSI 1 > > > +#define I2C_ADDR_CEC_DSI 0x49 > > > + > > > +#define I2C_MAX_IDX 2 > > > + > > > +#define HPD_WORK_DELAY_MS 1000 > > > + > > > +struct lt8912 { > > > + struct device *dev; > > > + struct drm_bridge bridge; > > > + struct drm_connector connector; > > > + > > > + struct i2c_client *i2c_client[I2C_MAX_IDX]; > > > + struct regmap *regmap[I2C_MAX_IDX]; > > > + > > > + struct device_node *host_node; > > > + struct drm_bridge *hdmi_port; > > > + > > > + struct mipi_dsi_device *dsi; > > > + > > > + struct gpio_desc *gp_reset; > > > + > > > + struct videomode mode; > > > + > > > + u8 data_lanes; > > > + bool is_power_on; > > > + bool is_attached; > > > +}; > > > + > > > +static int lt8912_write_init_config(struct lt8912 *lt) > > > +{ > > > > For the following register accesses it would be nice to see the > > registers named and the value written to them documented. If you don't > > have access to all of that information, not adding it is acceptable, > > but it will make the driver harder to maintain & extend going forward. > > > > So if possible, document registers in the same way it was done for lt9611 [1]. > > > > [1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/lontium-lt9611.c#L444 > > > > Sorry, I don't have any documentation about this chip. > I can't document these registers. Ack, let's work with what we've got then :) > > > > + const struct reg_sequence seq[] = { > > > + /* Digital clock en*/ > > > + {0x08, 0xff}, > > > + {0x09, 0xff}, > > > + {0x0a, 0xff}, > > > + {0x0b, 0x7c}, > > > + {0x0c, 0xff}, > > > + {0x42, 0x04}, > > > + > > > + /*Tx Analog*/ > > > + {0x31, 0xb1}, > > > + {0x32, 0xb1}, > > > + {0x33, 0x0e}, > > > + {0x37, 0x00}, > > > + {0x38, 0x22}, > > > + {0x60, 0x82}, > > > + > > > + /*Cbus Analog*/ > > > + {0x39, 0x45}, > > > + {0x3a, 0x00}, > > > + {0x3b, 0x00}, > > > + > > > + /*HDMI Pll Analog*/ > > > + {0x44, 0x31}, > > > + {0x55, 0x44}, > > > + {0x57, 0x01}, > > > + {0x5a, 0x02}, > > > + > > > + /*MIPI Analog*/ > > > + {0x3e, 0xd6}, > > > + {0x3f, 0xd4}, > > > + {0x41, 0x3c}, > > > + {0xB2, 0x00}, > > > + }; > > > + > > > + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq)); > > > +} > > > + > > > +static int lt8912_write_mipi_basic_config(struct lt8912 *lt) > > > +{ > > > + const struct reg_sequence seq[] = { > > > + {0x12, 0x04}, > > > + {0x14, 0x00}, > > > + {0x15, 0x00}, > > > + {0x1a, 0x03}, > > > + {0x1b, 0x03}, > > > + }; > > > + > > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > > +}; > > > + > > > +static int lt8912_write_dds_config(struct lt8912 *lt) > > > +{ > > > + const struct reg_sequence seq[] = { > > > + {0x4e, 0xff}, > > > + {0x4f, 0x56}, > > > + {0x50, 0x69}, > > > + {0x51, 0x80}, > > > + {0x1f, 0x5e}, > > > + {0x20, 0x01}, > > > + {0x21, 0x2c}, > > > + {0x22, 0x01}, > > > + {0x23, 0xfa}, > > > + {0x24, 0x00}, > > > + {0x25, 0xc8}, > > > + {0x26, 0x00}, > > > + {0x27, 0x5e}, > > > + {0x28, 0x01}, > > > + {0x29, 0x2c}, > > > + {0x2a, 0x01}, > > > + {0x2b, 0xfa}, > > > + {0x2c, 0x00}, > > > + {0x2d, 0xc8}, > > > + {0x2e, 0x00}, > > > + {0x42, 0x64}, > > > + {0x43, 0x00}, > > > + {0x44, 0x04}, > > > + {0x45, 0x00}, > > > + {0x46, 0x59}, > > > + {0x47, 0x00}, > > > + {0x48, 0xf2}, > > > + {0x49, 0x06}, > > > + {0x4a, 0x00}, > > > + {0x4b, 0x72}, > > > + {0x4c, 0x45}, > > > + {0x4d, 0x00}, > > > + {0x52, 0x08}, > > > + {0x53, 0x00}, > > > + {0x54, 0xb2}, > > > + {0x55, 0x00}, > > > + {0x56, 0xe4}, > > > + {0x57, 0x0d}, > > > + {0x58, 0x00}, > > > + {0x59, 0xe4}, > > > + {0x5a, 0x8a}, > > > + {0x5b, 0x00}, > > > + {0x5c, 0x34}, > > > + {0x1e, 0x4f}, > > > + {0x51, 0x00}, > > > + }; > > > + > > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > > +} > > > + > > > +static int lt8912_write_rxlogicres_config(struct lt8912 *lt) > > > +{ > > > + int ret; > > > + > > > + ret = regmap_write(lt->regmap[I2C_MAIN], 0x03, 0x7f); > > > + usleep_range(10000, 20000); > > > + ret |= regmap_write(lt->regmap[I2C_MAIN], 0x03, 0xff); > > > + > > > + return ret; > > > +}; > > > + > > > +static int lt8912_write_lvds_config(struct lt8912 *lt) > > > +{ > > > + const struct reg_sequence seq[] = { > > > + {0x44, 0x30}, > > > + {0x51, 0x05}, > > > + {0x50, 0x24}, > > > + {0x51, 0x2d}, > > > + {0x52, 0x04}, > > > + {0x69, 0x0e}, > > > + {0x69, 0x8e}, > > > + {0x6a, 0x00}, > > > + {0x6c, 0xb8}, > > > + {0x6b, 0x51}, > > > + {0x04, 0xfb}, > > > + {0x04, 0xff}, > > > + {0x7f, 0x00}, > > > + {0xa8, 0x13}, > > > + {0x02, 0xf7}, > > > + {0x02, 0xff}, > > > + {0x03, 0xcf}, > > > + {0x03, 0xff}, > > > + }; > > > + > > > + return regmap_multi_reg_write(lt->regmap[I2C_CEC_DSI], seq, ARRAY_SIZE(seq)); > > > +}; > > > + > > > +static inline struct lt8912 *bridge_to_lt8912(struct drm_bridge *b) > > > +{ > > > + return container_of(b, struct lt8912, bridge); > > > +} > > > + > > > +static inline struct lt8912 *connector_to_lt8912(struct drm_connector *c) > > > +{ > > > + return container_of(c, struct lt8912, connector); > > > +} > > > + > > > +static const struct regmap_config lt8912_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > + .max_register = 0xff, > > > +}; > > > + > > > +static int lt8912_init_i2c(struct lt8912 *lt) > > > +{ > > > + unsigned int i; > > > + int ret; > > > + struct i2c_board_info info[] = { > > > + { I2C_BOARD_INFO("lt8912p0", I2C_ADDR_MAIN), }, > > > + { I2C_BOARD_INFO("lt8912p1", I2C_ADDR_CEC_DSI), }, > > > + }; > > > + > > > + if (!lt) > > > + return -ENODEV; > > > + > > > + for (i = 0; i < ARRAY_SIZE(info); i++) { > > > + if (i > 0) { > > > + lt->i2c_client[i] = i2c_new_dummy_device(lt->i2c_client[0]->adapter, > > > + info[i].addr); > > > > Bad indentation. Some of these bad indentations look ok for > > tab-width=4, however tab-width=8 is what the style guide dictates. > > > > > + if (!lt->i2c_client[i]) > > > + return -ENODEV; > > > + } > > > + lt->regmap[i] = devm_regmap_init_i2c(lt->i2c_client[i], > > > + <8912_regmap_config); > > > > Bad indentation. > > > > > + if (IS_ERR(lt->regmap[i])) { > > > + ret = PTR_ERR(lt->regmap[i]); > > > + return ret; > > > + } > > > + } > > > + return 0; > > > +} > > > + > > > +static int lt8912_free_i2c(struct lt8912 *lt) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 1; i < I2C_MAX_IDX; i++) > > > + i2c_unregister_device(lt->i2c_client[i]); > > > + > > > + return 0; > > > +} > > > + > > > +static int lt8912_hard_power_on(struct lt8912 *lt) > > > +{ > > > + gpiod_set_value_cansleep(lt->gp_reset, 0); > > > + msleep(20); > > > + > > > + return 0; > > > +} > > > + > > > +static void lt8912_hard_power_off(struct lt8912 *lt) > > > +{ > > > + gpiod_set_value_cansleep(lt->gp_reset, 1); > > > + msleep(20); > > > + lt->is_power_on = false; > > > +} > > > + > > > +static int lt8912_video_setup(struct lt8912 *lt) > > > +{ > > > + u32 hactive, h_total, hpw, hfp, hbp; > > > + u32 vactive, v_total, vpw, vfp, vbp; > > > + u8 settle = 0x08; > > > + int ret; > > > + > > > + if (!lt) > > > + return -EINVAL; > > > + > > > + hactive = lt->mode.hactive; > > > + hfp = lt->mode.hfront_porch; > > > + hpw = lt->mode.hsync_len; > > > + hbp = lt->mode.hback_porch; > > > + h_total = hactive + hfp + hpw + hbp; > > > + > > > + vactive = lt->mode.vactive; > > > + vfp = lt->mode.vfront_porch; > > > + vpw = lt->mode.vsync_len; > > > + vbp = lt->mode.vback_porch; > > > + v_total = vactive + vfp + vpw + vbp; > > > + > > > + if (vactive <= 600) > > > + settle = 0x04; > > > + else if (vactive == 1080) > > > + settle = 0x0a; > > > + > > > + ret = regmap_write(lt->regmap[I2C_CEC_DSI], 0x10, 0x01); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x11, settle); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x18, hpw); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x19, vpw); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1c, hactive & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x1d, hactive >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x2f, 0x0c); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x34, h_total & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x35, h_total >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x36, v_total & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x37, v_total >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x38, vbp & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x39, vbp >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3a, vfp & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3b, vfp >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3c, hbp & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3d, hbp >> 8); > > > + > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3e, hfp & 0xff); > > > + ret |= regmap_write(lt->regmap[I2C_CEC_DSI], 0x3f, hfp >> 8); > > > + > > > + return ret; > > > +} > > > + > > > +static int lt8912_soft_power_on(struct lt8912 *lt) > > > +{ > > > + if (!lt->is_power_on) { > > > + u32 lanes = lt->data_lanes; > > > + > > > + lt8912_write_init_config(lt); > > > + regmap_write(lt->regmap[I2C_CEC_DSI], 0x13, lanes & 3); > > > + > > > + lt8912_write_mipi_basic_config(lt); > > > + > > > + lt->is_power_on = true; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int lt8912_video_on(struct lt8912 *lt) > > > +{ > > > + int ret; > > > + > > > + ret = lt8912_video_setup(lt); > > > + if (ret < 0) > > > + goto end; > > > + > > > + ret = lt8912_write_dds_config(lt); > > > + if (ret < 0) > > > + goto end; > > > + > > > + ret = lt8912_write_rxlogicres_config(lt); > > > + if (ret < 0) > > > + goto end; > > > + > > > + ret = lt8912_write_lvds_config(lt); > > > + if (ret < 0) > > > + goto end; > > > + > > > +end: > > > + return ret; > > > +} > > > + > > > +static enum drm_connector_status > > > +lt8912_connector_detect(struct drm_connector *connector, bool force) > > > +{ > > > + struct lt8912 *lt = connector_to_lt8912(connector); > > > + > > > + return drm_bridge_detect(lt->hdmi_port); > > > +} > > > + > > > +static const struct drm_connector_funcs lt8912_connector_funcs = { > > > + .detect = lt8912_connector_detect, > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > + .destroy = drm_connector_cleanup, > > > + .reset = drm_atomic_helper_connector_reset, > > > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > > +}; > > > + > > > +static enum drm_mode_status > > > +lt8912_connector_mode_valid(struct drm_connector *connector, > > > + struct drm_display_mode *mode) > > > +{ > > > + if (mode->clock > 150000) > > > + return MODE_CLOCK_HIGH; > > > + > > > + if (mode->hdisplay > 1920) > > > + return MODE_BAD_HVALUE; > > > + > > > + if (mode->vdisplay > 1080) > > > + return MODE_BAD_VVALUE; > > > + > > > + return MODE_OK; > > > +} > > > + > > > +static int lt8912_connector_get_modes(struct drm_connector *connector) > > > +{ > > > + struct edid *edid; > > > + int ret, num = 0; > > > + struct lt8912 *lt = connector_to_lt8912(connector); > > > + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; > > > + > > > + edid = drm_bridge_get_edid(lt->hdmi_port, connector); > > > > Memory leak, edid is never free'd. > > > > > + if (edid) { > > > + drm_connector_update_edid_property(connector, edid); > > > + num = drm_add_edid_modes(connector, edid); > > > + } else > > > + return ret; > > > + > > > + ret = drm_display_info_set_bus_formats(&connector->display_info, > > > + &bus_format, 1); > > > > Bad indentation. > > > > > + if (ret) > > > + return ret; > > > + > > > + return num; > > > +} > > > + > > > +static const struct drm_connector_helper_funcs lt8912_connector_helper_funcs = { > > > + .get_modes = lt8912_connector_get_modes, > > > + .mode_valid = lt8912_connector_mode_valid, > > > +}; > > > + > > > +static void lt8912_bridge_mode_set(struct drm_bridge *bridge, > > > + const struct drm_display_mode *mode, > > > + const struct drm_display_mode *adj) > > > +{ > > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > > + > > > + drm_display_mode_to_videomode(adj, <->mode); > > > +} > > > + > > > +static void lt8912_bridge_enable(struct drm_bridge *bridge) > > > +{ > > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > > + > > > + lt8912_video_on(lt); > > > +} > > > + > > > +static int lt8912_attach_dsi(struct lt8912 *lt) > > > +{ > > > + struct device *dev = lt->dev; > > > + struct mipi_dsi_host *host; > > > + struct mipi_dsi_device *dsi; > > > + int ret = -1; > > > + const struct mipi_dsi_device_info info = { .type = "lt8912", > > > + .channel = 0, > > > + .node = NULL, > > > + }; > > > + > > > + host = of_find_mipi_dsi_host_by_node(lt->host_node); > > > + if (!host) { > > > + dev_err(dev, "failed to find dsi host\n"); > > > + return -EPROBE_DEFER; > > > + } > > > + > > > + dsi = mipi_dsi_device_register_full(host, &info); > > > + if (IS_ERR(dsi)) { > > > + ret = PTR_ERR(dsi); > > > + dev_err(dev, "failed to create dsi device (%d)\n", ret); > > > + goto err_dsi_device; > > > + } > > > + > > > + lt->dsi = dsi; > > > + > > > + dsi->lanes = lt->data_lanes; > > > + dsi->format = MIPI_DSI_FMT_RGB888; > > > + > > > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | > > > + MIPI_DSI_MODE_VIDEO_BURST | > > > + MIPI_DSI_MODE_LPM | > > > + MIPI_DSI_MODE_EOT_PACKET; > > > > Maybe align the 3 last lines with where the 1st one starts. > > > > > + > > > + ret = mipi_dsi_attach(dsi); > > > + if (ret < 0) { > > > + dev_err(dev, "failed to attach dsi to host\n"); > > > + goto err_dsi_attach; > > > + } > > > + > > > + return 0; > > > + > > > +err_dsi_attach: > > > + mipi_dsi_device_unregister(dsi); > > > +err_dsi_device: > > > + return ret; > > > +} > > > + > > > +static void lt8912_detach_dsi(struct lt8912 *lt) > > > +{ > > > + mipi_dsi_detach(lt->dsi); > > > + mipi_dsi_device_unregister(lt->dsi); > > > +} > > > + > > > +static int lt8912_bridge_init(struct drm_bridge *bridge) > > > +{ > > > + int ret; > > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > > + struct drm_connector *connector = <->connector; > > > + > > > + > > > + if (lt->hdmi_port->ops & DRM_BRIDGE_OP_DETECT) { > > > + connector->polled = DRM_CONNECTOR_POLL_HPD; > > > + } else { > > > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | > > > + DRM_CONNECTOR_POLL_DISCONNECT; > > > > Bad indentation. > > > > > + } > > > + > > > + > > > + ret = drm_connector_init(bridge->dev, connector, > > > + <8912_connector_funcs, > > > + lt->hdmi_port->type); > > > + if (ret) > > > + goto exit; > > > + > > > + drm_connector_helper_add(connector, <8912_connector_helper_funcs); > > > + > > > + connector->dpms = DRM_MODE_DPMS_OFF; > > > + drm_connector_attach_encoder(connector, bridge->encoder); > > > + > > > + > > > + ret = lt8912_hard_power_on(lt); > > > + if (ret) > > > + goto exit; > > > + > > > + ret = lt8912_soft_power_on(lt); > > > + > > > +exit: > > > + return ret; > > > +} > > > + > > > +static int lt8912_bridge_attach(struct drm_bridge *bridge, > > > + enum drm_bridge_attach_flags flags) > > > > Bad indentation. > > > > > +{ > > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > > + int ret; > > > + > > > + /* No need to check the return of this function */ > > > + drm_bridge_attach(lt->bridge.encoder, > > > + lt->hdmi_port, > > > + <->bridge, > > > + flags); > > > > Bad indentation. > > > > > + > > > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { > > > + ret = lt8912_bridge_init(bridge); > > > + if (ret) { > > > + dev_err(lt->dev, "Failed to init bridge ! (%d)\n", ret); > > > + return ret; > > > + } > > > + } > > > + > > > + ret = lt8912_attach_dsi(lt); > > > + if (ret) > > > + return ret; > > > + > > > + lt->is_attached = true; > > > + > > > + return ret; > > > +} > > > + > > > +static void lt8912_bridge_detach(struct drm_bridge *bridge) > > > +{ > > > + struct lt8912 *lt = bridge_to_lt8912(bridge); > > > + > > > + if (lt->is_attached) { > > > + lt8912_detach_dsi(lt); > > > + lt8912_hard_power_off(lt); > > > + } > > > +} > > > + > > > +static const struct drm_bridge_funcs lt8912_bridge_funcs = { > > > + .attach = lt8912_bridge_attach, > > > + .detach = lt8912_bridge_detach, > > > + .mode_set = lt8912_bridge_mode_set, > > > + .enable = lt8912_bridge_enable, > > > > Have a look at the documentation around in drm_bridge_funcs[1], some > > additional functions have to be implemented. I think you can look at > > lt9611[2] & lt9611uxc[3] for inspiration. > > > > [1] https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridge-operations > > [2] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611.c#L947 > > [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/lontium-lt9611uxc.c#L542 > > > > > > > +}; > > > + > > > +static int lt8912_parse_dt(struct lt8912 *lt) > > > +{ > > > + struct gpio_desc *gp_reset; > > > + struct device *dev = lt->dev; > > > + int ret = 0; > > > + struct device_node *port_node; > > > + struct device_node *endpoint; > > > + > > > + gp_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > > + if (IS_ERR(gp_reset)) { > > > + ret = PTR_ERR(gp_reset); > > > + if (ret != -EPROBE_DEFER) > > > + dev_err(dev, "Failed to get reset gpio: %d\n", ret); > > > + return ret; > > > + } > > > + lt->gp_reset = gp_reset; > > > + > > > + > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); > > > + if (IS_ERR(endpoint)) { > > > + ret = PTR_ERR(endpoint); > > > + goto end; > > > + } > > > + > > > + lt->data_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > > > + of_node_put(endpoint); > > > + > > > + lt->host_node = of_graph_get_remote_node(dev->of_node, 0, -1); > > > + if (!lt->host_node) { > > > + dev_err(lt->dev, "%s: Failed to get remote port\n", __func__); > > > + ret = -ENODEV; > > > + goto end; > > > + } > > > + > > > + port_node = of_graph_get_remote_node(dev->of_node, 1, -1); > > > + if (!port_node) { > > > + dev_err(lt->dev, "%s: Failed to get connector port\n", __func__); > > > + ret = -ENODEV; > > > + goto err_free_host_node; > > > + } > > > + > > > + lt->hdmi_port = of_drm_find_bridge(port_node); > > > + if (IS_ERR(lt->hdmi_port)) { > > > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > > > + ret = PTR_ERR(lt->hdmi_port); > > > + of_node_put(lt->host_node); > > > + goto end; > > > + } > > > + > > > + if (!of_device_is_compatible(port_node, "hdmi-connector")) { > > > + dev_err(lt->dev, "%s: Failed to get hdmi port\n", __func__); > > > + ret = -EINVAL; > > > + } > > > + > > > + of_node_put(port_node); > > > + > > > +end: > > > + return ret; > > > + > > > +err_free_host_node: > > > + of_node_put(lt->host_node); > > > + return ret; > > > +} > > > + > > > +static int lt8912_put_dt(struct lt8912 *lt) > > > +{ > > > + of_node_put(lt->host_node); > > > + return 0; > > > +} > > > + > > > +static int lt8912_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + static struct lt8912 *lt; > > > + int ret = 0; > > > + struct device *dev = &client->dev; > > > + > > > + lt = devm_kzalloc(dev, sizeof(struct lt8912), GFP_KERNEL); > > > + if (!lt) > > > + return -ENOMEM; > > > + > > > + lt->dev = dev; > > > + lt->i2c_client[0] = client; > > > > The i2c_client that is created from the DT address and passed into > > lt8912_probe will be overwritten by the i2c clients created in > > lt8912_init_i2c before it is ever used. > > I think you made a mistake here. > The lt8912_init_i2c function only initializes the lt->i2c_client[1]. > (You can check the if condition). You are 100% correct. > > > > > A related issue is that this device has two I2C addresses, which > > doesn't fit super neatly into the I2C driver model[1]. Since both > > addresses can't be provided by the DT, method one doesn't fit this > > driver. Method 2 doesn't fit well either, since the I2C bus isn't > > internal to the device. And 3&4 are not applicable. I'd say the two > > options are #1 splitting this driver, #2 keeping it like it is. I'm > > leaning towards #2 since both addresses correspond to a physical chip. > > > > [1] https://www.kernel.org/doc/html/latest/i2c/instantiating-devices.html > > > > > + > > > + ret = lt8912_parse_dt(lt); > > > + if (ret) > > > + goto err_dt_parse; > > > + > > > + ret = lt8912_init_i2c(lt); > > > + if (ret) > > > + goto err_i2c; > > > + > > > + i2c_set_clientdata(client, lt); > > > + > > > + lt->bridge.funcs = <8912_bridge_funcs; > > > + lt->bridge.of_node = dev->of_node; > > > + drm_bridge_add(<->bridge); > > > + > > > + return 0; > > > + > > > +err_i2c: > > > + lt8912_put_dt(lt); > > > +err_dt_parse: > > > + return ret; > > > +} > > > + > > > +static int lt8912_remove(struct i2c_client *client) > > > +{ > > > + struct lt8912 *lt = i2c_get_clientdata(client); > > > + > > > + lt8912_bridge_detach(<->bridge); > > > + drm_bridge_remove(<->bridge); > > > + lt8912_free_i2c(lt); > > > + lt8912_put_dt(lt); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id lt8912_dt_match[] = { > > > + {.compatible = "lontium,lt8912"}, > > > + {} > > > +}; > > > +MODULE_DEVICE_TABLE(of, lt8912_dt_match); > > > + > > > +static const struct i2c_device_id lt8912_id[] = { > > > + {"lt8912", 0}, > > > + {}, > > > +}; > > > +MODULE_DEVICE_TABLE(i2c, lt8912_id); > > > + > > > +static struct i2c_driver lt8912_i2c_driver = { > > > + .driver = { > > > + .name = "lt8912", > > > + .of_match_table = lt8912_dt_match, > > > + .owner = THIS_MODULE, > > > > The .owner assignment is redundant as it is later re-assigned in[1]. > > > > module_i2c_driver -> i2c_add_driver -> i2c_register_driver > > > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L1796 > > > > > + }, > > > + .probe = lt8912_probe, > > > + .remove = lt8912_remove, > > > + .id_table = lt8912_id, > > > +}; > > > +module_i2c_driver(lt8912_i2c_driver); > > > + > > > +MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>"); > > > +MODULE_DESCRIPTION("lt8912 drm driver"); > > > +MODULE_LICENSE("GPL v2"); > > > -- > > > 2.25.1 > > > > > I will send a new version with the fixes. > > Thanks again, > Adrien Grassein ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-18 21:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-14 23:29 [PATCH v2 0/2] Add support of Lontium lt8912 MIPI to HDMI bridge Adrien Grassein 2021-02-14 23:29 ` [PATCH v2 1/2] dt-bindings: display: bridge: Add documentation for LT8912 Adrien Grassein 2021-02-17 20:24 ` Rob Herring 2021-02-17 22:16 ` Rob Herring 2021-02-14 23:29 ` [PATCH v2 2/2] drm/bridge: Introduce LT8912 DSI to HDMI bridge Adrien Grassein 2021-02-17 17:32 ` Robert Foss 2021-02-18 20:35 ` Adrien Grassein 2021-02-18 21:26 ` Robert Foss
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox