Hi,
This series adds driver and bindings for Lontium LT9611 bridge chip which takes MIPI DSI as input and HDMI as output.
This chip can be found in 96boards RB3 platform [1] commonly called DB845c.
[1]: https://www.96boards.org/product/rb3-platform/
Vinod Koul (3): dt-bindings: vendor-prefixes: Add Lontium vendor prefix dt-bindings: display: bridge: Add documentation for LT9611 drm/bridge: Introduce LT9611 DSI to HDMI bridge
.../display/bridge/lontium,lt9611.yaml | 178 +++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/gpu/drm/bridge/Kconfig | 13 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lt9611.c | 1113 +++++++++++++++++ 5 files changed, 1307 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml create mode 100644 drivers/gpu/drm/bridge/lt9611.c
Add prefix for Lontium Semiconductor Corporation
Signed-off-by: Vinod Koul vkoul@kernel.org --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index d3891386d671..7294852bc47b 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -579,6 +579,8 @@ patternProperties: description: Logic Technologies Limited "^longcheer,.*": description: Longcheer Technology (Shanghai) Co., Ltd. + "^lontium,.*": + description: Lontium Semiconductor Corporation "^loongson,.*": description: Loongson Technology Corporation Limited "^lsi,.*":
On Wed, 13 May 2020 15:35:31 +0530, Vinod Koul wrote:
Add prefix for Lontium Semiconductor Corporation
Signed-off-by: Vinod Koul vkoul@kernel.org
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+)
Acked-by: Rob Herring robh@kernel.org
Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports and I2S port as input and one HDMI port as output
Signed-off-by: Vinod Koul vkoul@kernel.org --- .../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml new file mode 100644 index 000000000000..77ee8cc35cd8 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml @@ -0,0 +1,178 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Lontium LT9611 2 Port MIPI to HDMI Bridge + +maintainers: + - Vinod Koul vkoul@kernel.org + +description: | + The LT9611 is a bridge device which converts DSI to HDMI + +properties: + compatible: + enum: + - lontium,lt9611 + + reg: + maxItems: 1 + description: base I2C address of the device. + + "#sound-dai-cells": + const: 1 + + interrupts: + maxItems: 1 + description: interrupt line for the chip + + reset-gpios: + maxItems: 1 + description: GPIO connected to active high RESET pin. + + vdd-supply: + description: Regulator for 1.8V MIPI phy power. + + vcc-supply: + description: Regulator for 3.3V IO power. + + ports: + type: object + + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + port@0: + type: object + additionalProperties: false + + description: | + HDMI port for HDMI output + + properties: + reg: + const: 0 + + patternProperties: + endpoint: + type: object + additionalProperties: false + + properties: + remote-endpoint: true + + required: + - reg + + port@1: + type: object + additionalProperties: false + + description: | + MIPI port-1 for MIPI input + + properties: + reg: + const: 1 + + patternProperties: + endpoint: + type: object + additionalProperties: false + + properties: + remote-endpoint: true + + required: + - reg + + port@2: + type: object + additionalProperties: false + + description: | + MIPI port-2 for MIPI input + + properties: + reg: + const: 2 + + patternProperties: + endpoint: + type: object + additionalProperties: false + + properties: + remote-endpoint: true + + required: + - reg + + required: + - "#address-cells" + - "#size-cells" + - port@0 + - port@1 + +required: + - compatible + - reg + - interrupts + - vdd-supply + - vcc-supply + - ports + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + i2c10 { + #address-cells = <1>; + #size-cells = <0>; + + lt9611_codec: hdmi-bridge@3b { + compatible = "lontium,lt9611"; + reg = <0x3b>; + + reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>; + interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>; + + vdd-supply = <<9611_1v8>; + vcc-supply = <<9611_3v3>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lt9611_out: endpoint { + remote-endpoint = <&hdmi_con>; + }; + }; + + port@1 { + reg = <1>; + lt9611_a: endpoint { + remote-endpoint = <&dsi0_out>; + }; + }; + + port@2 { + reg = <2>; + lt9611_b: endpoint { + remote-endpoint = <&dsi1_out>; + }; + }; + }; + }; + };
On Wed, 13 May 2020 15:35:32 +0530, Vinod Koul wrote:
Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports and I2S port as input and one HDMI port as output
Signed-off-by: Vinod Koul vkoul@kernel.org
.../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
Reviewed-by: Rob Herring robh@kernel.org
Hi Vinod,
Thank you for the patch.
On Wed, May 13, 2020 at 03:35:32PM +0530, Vinod Koul wrote:
Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports and I2S port as input and one HDMI port as output
Signed-off-by: Vinod Koul vkoul@kernel.org
.../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml new file mode 100644 index 000000000000..77ee8cc35cd8 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml @@ -0,0 +1,178 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Lontium LT9611 2 Port MIPI to HDMI Bridge
+maintainers:
- Vinod Koul vkoul@kernel.org
+description: |
- The LT9611 is a bridge device which converts DSI to HDMI
+properties:
- compatible:
- enum:
- lontium,lt9611- reg:
- maxItems: 1
- description: base I2C address of the device.
- "#sound-dai-cells":
- const: 1
- interrupts:
- maxItems: 1
- description: interrupt line for the chip
I think you could drop the descriptions for the reg and interrupt properties, they don't add much.
- reset-gpios:
- maxItems: 1
- description: GPIO connected to active high RESET pin.
- vdd-supply:
- description: Regulator for 1.8V MIPI phy power.
- vcc-supply:
- description: Regulator for 3.3V IO power.
- ports:
- type: object
- properties:
"#address-cells":const: 1"#size-cells":const: 0port@0:type: objectadditionalProperties: falsedescription: |HDMI port for HDMI output
The usual practice is to have the input ports first, followed by the output ports. Is there a reason not to follow that rule ?
properties:reg:const: 0patternProperties:endpoint:
If you want to use patternProperties, this should be
"^endpoint@[0-9]+$":
(including the quotes). Same below.
type: objectadditionalProperties: falseproperties:remote-endpoint: true
How about
remote-endpoint: $ref: /schemas/types.yaml#/definitions/phandle
and the same below ?
You also need a reg property if multiple endpoints are present.
required:- regport@1:type: objectadditionalProperties: falsedescription: |MIPI port-1 for MIPI inputproperties:reg:const: 1patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- regport@2:type: objectadditionalProperties: falsedescription: |MIPI port-2 for MIPI input
A description of how the two MIPI inputs differ would be useful. In particular, are both mandatory, or is it valid to connect only one of the two ? If using a single input is supported, can it be either, or does it have to be the first one ? When using both inputs, what should be connected to them ?
properties:reg:const: 2patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- reg- required:
- "#address-cells"- "#size-cells"- port@0- port@1+required:
- compatible
- reg
- interrupts
- vdd-supply
- vcc-supply
- ports
+additionalProperties: false
+examples:
- |
- #include <dt-bindings/gpio/gpio.h>
- #include <dt-bindings/interrupt-controller/irq.h>
- i2c10 {
#address-cells = <1>;#size-cells = <0>;lt9611_codec: hdmi-bridge@3b {
Please drop unused labels.
compatible = "lontium,lt9611";reg = <0x3b>;reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;vdd-supply = <<9611_1v8>;vcc-supply = <<9611_3v3>;ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lt9611_out: endpoint {remote-endpoint = <&hdmi_con>;};};port@1 {reg = <1>;lt9611_a: endpoint {remote-endpoint = <&dsi0_out>;};};port@2 {reg = <2>;lt9611_b: endpoint {remote-endpoint = <&dsi1_out>;};};};};- };
It's customary to end YAML schema files with ... on a separate line.
Hi Laurent,
Sorry for late reply, I was out last week.
On 28-05-20, 04:48, Laurent Pinchart wrote:
- interrupts:
- maxItems: 1
- description: interrupt line for the chip
I think you could drop the descriptions for the reg and interrupt properties, they don't add much.
Sure, will do
- reset-gpios:
- maxItems: 1
- description: GPIO connected to active high RESET pin.
- vdd-supply:
- description: Regulator for 1.8V MIPI phy power.
- vcc-supply:
- description: Regulator for 3.3V IO power.
- ports:
- type: object
- properties:
"#address-cells":const: 1"#size-cells":const: 0port@0:type: objectadditionalProperties: falsedescription: |HDMI port for HDMI outputThe usual practice is to have the input ports first, followed by the output ports. Is there a reason not to follow that rule ?
I was not aware of this rule, is it documented somewhere? Nevertheless will update..
properties:reg:const: 0patternProperties:endpoint:If you want to use patternProperties, this should be
"^endpoint@[0-9]+$":(including the quotes). Same below.
Ok
type: objectadditionalProperties: falseproperties:remote-endpoint: trueHow about
remote-endpoint: $ref: /schemas/types.yaml#/definitions/phandleand the same below ?
Ok
You also need a reg property if multiple endpoints are present.
Will update
required:- regport@1:type: objectadditionalProperties: falsedescription: |MIPI port-1 for MIPI inputproperties:reg:const: 1patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- regport@2:type: objectadditionalProperties: falsedescription: |MIPI port-2 for MIPI inputA description of how the two MIPI inputs differ would be useful. In particular, are both mandatory, or is it valid to connect only one of the two ? If using a single input is supported, can it be either, or does it have to be the first one ? When using both inputs, what should be connected to them ?
Sure I will add details. port-1 is mandatory and port-2 optional. port-2 is used in combination with port-1 to drive displays for higher resolution like 4k
properties:reg:const: 2patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- reg- required:
- "#address-cells"- "#size-cells"- port@0- port@1+required:
- compatible
- reg
- interrupts
- vdd-supply
- vcc-supply
- ports
+additionalProperties: false
+examples:
- |
- #include <dt-bindings/gpio/gpio.h>
- #include <dt-bindings/interrupt-controller/irq.h>
- i2c10 {
#address-cells = <1>;#size-cells = <0>;lt9611_codec: hdmi-bridge@3b {Please drop unused labels.
ok
compatible = "lontium,lt9611";reg = <0x3b>;reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;vdd-supply = <<9611_1v8>;vcc-supply = <<9611_3v3>;ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lt9611_out: endpoint {remote-endpoint = <&hdmi_con>;};};port@1 {reg = <1>;lt9611_a: endpoint {remote-endpoint = <&dsi0_out>;};};port@2 {reg = <2>;lt9611_b: endpoint {remote-endpoint = <&dsi1_out>;};};};};- };
It's customary to end YAML schema files with ... on a separate line.
Will update
Thanks for the review
Hi Vinod,
On Thu, Jun 04, 2020 at 12:48:59PM +0530, Vinod Koul wrote:
Hi Laurent,
Sorry for late reply, I was out last week.
No worries.
On 28-05-20, 04:48, Laurent Pinchart wrote:
- interrupts:
- maxItems: 1
- description: interrupt line for the chip
I think you could drop the descriptions for the reg and interrupt properties, they don't add much.
Sure, will do
- reset-gpios:
- maxItems: 1
- description: GPIO connected to active high RESET pin.
- vdd-supply:
- description: Regulator for 1.8V MIPI phy power.
- vcc-supply:
- description: Regulator for 3.3V IO power.
- ports:
- type: object
- properties:
"#address-cells":const: 1"#size-cells":const: 0port@0:type: objectadditionalProperties: falsedescription: |HDMI port for HDMI outputThe usual practice is to have the input ports first, followed by the output ports. Is there a reason not to follow that rule ?
I was not aware of this rule, is it documented somewhere? Nevertheless will update..
I don't think it's documented, no. It's just a common practice.
properties:reg:const: 0patternProperties:endpoint:If you want to use patternProperties, this should be
"^endpoint@[0-9]+$":(including the quotes). Same below.
Ok
type: objectadditionalProperties: falseproperties:remote-endpoint: trueHow about
remote-endpoint: $ref: /schemas/types.yaml#/definitions/phandleand the same below ?
Ok
You also need a reg property if multiple endpoints are present.
Will update
required:- regport@1:type: objectadditionalProperties: falsedescription: |MIPI port-1 for MIPI inputproperties:reg:const: 1patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- regport@2:type: objectadditionalProperties: falsedescription: |MIPI port-2 for MIPI inputA description of how the two MIPI inputs differ would be useful. In particular, are both mandatory, or is it valid to connect only one of the two ? If using a single input is supported, can it be either, or does it have to be the first one ? When using both inputs, what should be connected to them ?
Sure I will add details. port-1 is mandatory and port-2 optional. port-2 is used in combination with port-1 to drive displays for higher resolution like 4k
properties:reg:const: 2patternProperties:endpoint:type: objectadditionalProperties: falseproperties:remote-endpoint: truerequired:- reg- required:
- "#address-cells"- "#size-cells"- port@0- port@1+required:
- compatible
- reg
- interrupts
- vdd-supply
- vcc-supply
- ports
+additionalProperties: false
+examples:
- |
- #include <dt-bindings/gpio/gpio.h>
- #include <dt-bindings/interrupt-controller/irq.h>
- i2c10 {
#address-cells = <1>;#size-cells = <0>;lt9611_codec: hdmi-bridge@3b {Please drop unused labels.
ok
compatible = "lontium,lt9611";reg = <0x3b>;reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;vdd-supply = <<9611_1v8>;vcc-supply = <<9611_3v3>;ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lt9611_out: endpoint {remote-endpoint = <&hdmi_con>;};};port@1 {reg = <1>;lt9611_a: endpoint {remote-endpoint = <&dsi0_out>;};};port@2 {reg = <2>;lt9611_b: endpoint {remote-endpoint = <&dsi1_out>;};};};};- };
It's customary to end YAML schema files with ... on a separate line.
Will update
Thanks for the review
Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and I2S port as an input and HDMI port as output
Co-developed-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/gpu/drm/bridge/Kconfig | 13 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lt9611.c | 1113 +++++++++++++++++++++++++++++++ 3 files changed, 1127 insertions(+) create mode 100644 drivers/gpu/drm/bridge/lt9611.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index aaed2347ace9..5cac15ce2027 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -38,6 +38,19 @@ 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_LT9611 + tristate "Lontium LT9611 DSI/HDMI bridge" + select SND_SOC_HDMI_CODEC if SND_SOC + depends on OF + select DRM_PANEL_BRIDGE + select DRM_KMS_HELPER + select REGMAP_I2C + help + Driver for Lontium LT9611 DSI to HDMI bridge + chip driver that converts dual DSI and I2S to + HDMI signals + Please say Y if you have such hardware. + config DRM_LVDS_CODEC tristate "Transparent LVDS encoders and decoders support" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 6fb062b5b0f0..d2a696e8ca5d 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o +obj-$(CONFIG_DRM_LONTIUM_LT9611) += lt9611.o obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o diff --git a/drivers/gpu/drm/bridge/lt9611.c b/drivers/gpu/drm/bridge/lt9611.c new file mode 100644 index 000000000000..e6e7ce43980d --- /dev/null +++ b/drivers/gpu/drm/bridge/lt9611.c @@ -0,0 +1,1113 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2019-2020. Linaro Limited. + */ + +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/interrupt.h> +#include <linux/of_graph.h> +#include <linux/regulator/consumer.h> +#include <drm/drm_probe_helper.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_print.h> + +#define EDID_SEG_SIZE 256 + +#define LT9611_4LANES 0 + +struct lt9611 { + struct device *dev; + struct drm_bridge bridge; + struct drm_connector connector; + + struct regmap *regmap; + + struct device_node *dsi0_node; + struct device_node *dsi1_node; + struct mipi_dsi_device *dsi0; + struct mipi_dsi_device *dsi1; + + bool ac_mode; + + struct gpio_desc *reset_gpio; + struct gpio_desc *enable_gpio; + + bool power_on; + bool sleep; + + struct regulator_bulk_data supplies[2]; + + struct i2c_client *client; + + enum drm_connector_status status; + + u8 edid_buf[EDID_SEG_SIZE]; + u32 vic; +}; + +#define LT9611_PAGE_CONTROL 0xff + +static const struct regmap_range_cfg lt9611_ranges[] = { + { + .name = "register_range", + .range_min = 0, + .range_max = 0x85ff, + .selector_reg = LT9611_PAGE_CONTROL, + .selector_mask = 0xff, + .selector_shift = 0, + .window_start = 0, + .window_len = 0x100, + }, +}; + +static const struct regmap_config lt9611_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0xffff, + .ranges = lt9611_ranges, + .num_ranges = ARRAY_SIZE(lt9611_ranges), +}; + +struct lt9611_mode { + u16 hdisplay; + u16 vdisplay; + u8 fps; + u8 lanes; + u8 intfs; +}; + +static struct lt9611_mode lt9611_modes[] = { + { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */ + { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */ + { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */ + { 1920, 1080, 24, 3, 1 }, + { 720, 480, 60, 4, 1 }, + { 720, 576, 50, 2, 1 }, + { 640, 480, 60, 2, 1 }, +}; + +static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge) +{ + return container_of(bridge, struct lt9611, bridge); +} + +static struct lt9611 *connector_to_lt9611(struct drm_connector *connector) +{ + return container_of(connector, struct lt9611, connector); +} + +static int lt9611_mipi_input_analog(struct lt9611 *lt9611) +{ + struct reg_sequence reg_cfg[] = { + { 0x8106, 0x40 }, /*port A rx current*/ + { 0x810a, 0xfe }, /*port A ldo voltage set*/ + { 0x810b, 0xbf }, /*enable port A lprx*/ + { 0x8111, 0x40 }, /*port B rx current*/ + { 0x8115, 0xfe }, /*port B ldo voltage set*/ + { 0x8116, 0xbf }, /*enable port B lprx*/ + + { 0x811c, 0x03 }, /*PortA clk lane no-LP mode*/ + { 0x8120, 0x03 }, /*PortB clk lane with-LP mode*/ + }; + + regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg)); + + return 0; +} + +static int lt9611_mipi_input_digital(struct lt9611 *lt9611, + const struct drm_display_mode *mode) +{ + regmap_write(lt9611->regmap, 0x8300, LT9611_4LANES); + + if (mode->hdisplay == 3840) + regmap_write(lt9611->regmap, 0x830a, 0x03); + else + regmap_write(lt9611->regmap, 0x830a, 0x00); + + regmap_write(lt9611->regmap, 0x824f, 0x80); + regmap_write(lt9611->regmap, 0x8250, 0x10); + regmap_write(lt9611->regmap, 0x8302, 0x0a); + regmap_write(lt9611->regmap, 0x8306, 0x0a); + + return 0; +} + +static void lt9611_mipi_video_setup(struct lt9611 *lt9611, + const struct drm_display_mode *mode) +{ + u32 h_total, h_act, hpw, hfp, hss; + u32 v_total, v_act, vpw, vfp, vss; + + h_total = mode->htotal; + v_total = mode->vtotal; + + h_act = mode->hdisplay; + hpw = mode->hsync_end - mode->hsync_start; + hfp = mode->hsync_start - mode->hdisplay; + hss = (mode->hsync_end - mode->hsync_start) + + (mode->htotal - mode->hsync_end); + + v_act = mode->vdisplay; + vpw = mode->vsync_end - mode->vsync_start; + vfp = mode->vsync_start - mode->vdisplay; + vss = (mode->vsync_end - mode->vsync_start) + + (mode->vtotal - mode->vsync_end); + + regmap_write(lt9611->regmap, 0x830d, (u8)(v_total / 256)); + regmap_write(lt9611->regmap, 0x830e, (u8)(v_total % 256)); + + regmap_write(lt9611->regmap, 0x830f, (u8)(v_act / 256)); + regmap_write(lt9611->regmap, 0x8310, (u8)(v_act % 256)); + + regmap_write(lt9611->regmap, 0x8311, (u8)(h_total / 256)); + regmap_write(lt9611->regmap, 0x8312, (u8)(h_total % 256)); + + regmap_write(lt9611->regmap, 0x8313, (u8)(h_act / 256)); + regmap_write(lt9611->regmap, 0x8314, (u8)(h_act % 256)); + + regmap_write(lt9611->regmap, 0x8315, (u8)(vpw % 256)); + regmap_write(lt9611->regmap, 0x8316, (u8)(hpw % 256)); + + regmap_write(lt9611->regmap, 0x8317, (u8)(vfp % 256)); + + regmap_write(lt9611->regmap, 0x8318, (u8)(vss % 256)); + + regmap_write(lt9611->regmap, 0x8319, (u8)(hfp % 256)); + + regmap_write(lt9611->regmap, 0x831a, (u8)(hss / 256)); + regmap_write(lt9611->regmap, 0x831b, (u8)(hss % 256)); +} + +static int lt9611_pcr_setup(struct lt9611 *lt9611, + const struct drm_display_mode *mode) +{ + struct reg_sequence reg_cfg[] = { + { 0x830b, 0x01 }, + { 0x830c, 0x10 }, + { 0x8348, 0x00 }, + { 0x8349, 0x81 }, + + /* stage 1 */ + { 0x8321, 0x4a }, + { 0x8324, 0x71 }, + { 0x8325, 0x30 }, + { 0x832a, 0x01 }, + + /* stage 2 */ + { 0x834a, 0x40 }, + { 0x831d, 0x10 }, + + /* MK limit */ + { 0x832d, 0x38 }, + { 0x8331, 0x08 }, + }; + + regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg)); + + switch (mode->hdisplay) { + case 640: + regmap_write(lt9611->regmap, 0x8326, 0x14); + break; + case 1920: + regmap_write(lt9611->regmap, 0x8326, 0x37); + break; + case 3840: + regmap_write(lt9611->regmap, 0x830b, 0x03); + regmap_write(lt9611->regmap, 0x830c, 0xd0); + regmap_write(lt9611->regmap, 0x8348, 0x03); + regmap_write(lt9611->regmap, 0x8349, 0xe0); + regmap_write(lt9611->regmap, 0x8324, 0x72); + regmap_write(lt9611->regmap, 0x8325, 0x00); + regmap_write(lt9611->regmap, 0x832a, 0x01); + regmap_write(lt9611->regmap, 0x834a, 0x10); + regmap_write(lt9611->regmap, 0x831d, 0x10); + regmap_write(lt9611->regmap, 0x8326, 0x37); + break; + } + + /* pcr rst */ + regmap_write(lt9611->regmap, 0x8011, 0x5a); + regmap_write(lt9611->regmap, 0x8011, 0xfa); + + return 0; +} + +static int lt9611_pll_setup(struct lt9611 *lt9611, + const struct drm_display_mode *mode) +{ + unsigned int pclk = mode->clock; + struct reg_sequence reg_cfg[] = { + /* txpll init */ + { 0x8123, 0x40 }, + { 0x8124, 0x64 }, + { 0x8125, 0x80 }, + { 0x8126, 0x55 }, + { 0x812c, 0x37 }, + { 0x812f, 0x01 }, + { 0x8126, 0x55 }, + { 0x8127, 0x66 }, + { 0x8128, 0x88 }, + }; + + regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg)); + + if (pclk > 150000) + regmap_write(lt9611->regmap, 0x812d, 0x88); + else if (pclk > 70000) + regmap_write(lt9611->regmap, 0x812d, 0x99); + else + regmap_write(lt9611->regmap, 0x812d, 0xaa); + + regmap_write(lt9611->regmap, 0x82e3, pclk >> 17); /* pclk[19:16] */ + regmap_write(lt9611->regmap, 0x82e4, pclk >> 9); /* pclk[15:8] */ + regmap_write(lt9611->regmap, 0x82e5, pclk >> 1); /* pclk[7:0] */ + + regmap_write(lt9611->regmap, 0x82de, 0x20); + regmap_write(lt9611->regmap, 0x82de, 0xe0); + + regmap_write(lt9611->regmap, 0x8016, 0xf1); + regmap_write(lt9611->regmap, 0x8016, 0xf3); + + return 0; +} + +static int lt9611_video_check(struct lt9611 *lt9611) +{ + u32 v_total, v_act, h_act_a, h_act_b, h_total_sysclk; + unsigned int temp; + int ret; + + /* top module video check */ + + /* v_act */ + ret = regmap_read(lt9611->regmap, 0x8282, &temp); + if (ret) + goto end; + + v_act = temp << 8; + ret = regmap_read(lt9611->regmap, 0x8283, &temp); + if (ret) + goto end; + v_act = v_act + temp; + + /* v_total */ + ret = regmap_read(lt9611->regmap, 0x826c, &temp); + if (ret) + goto end; + v_total = temp << 8; + ret = regmap_read(lt9611->regmap, 0x826d, &temp); + if (ret) + goto end; + v_total = v_total + temp; + + /* h_total_sysclk */ + ret = regmap_read(lt9611->regmap, 0x8286, &temp); + if (ret) + goto end; + h_total_sysclk = temp << 8; + ret = regmap_read(lt9611->regmap, 0x8287, &temp); + if (ret) + goto end; + h_total_sysclk = h_total_sysclk + temp; + + /* h_act_a */ + ret = regmap_read(lt9611->regmap, 0x8382, &temp); + if (ret) + goto end; + h_act_a = temp << 8; + ret = regmap_read(lt9611->regmap, 0x8383, &temp); + if (ret) + goto end; + h_act_a = (h_act_a + temp) / 3; + + /* h_act_b */ + ret = regmap_read(lt9611->regmap, 0x8386, &temp); + if (ret) + goto end; + h_act_b = temp << 8; + ret = regmap_read(lt9611->regmap, 0x8387, &temp); + if (ret) + goto end; + h_act_b = (h_act_b + temp) / 3; + + dev_info(lt9611->dev, + "video check: h_act_a=%d, h_act_b=%d, v_act=%d, v_total=%d, h_total_sysclk=%d\n", + h_act_a, h_act_b, v_act, v_total, h_total_sysclk); + + return 0; + +end: + dev_err(lt9611->dev, "read video check error\n"); + return ret; +} + +static void lt9611_hdmi_tx_digital(struct lt9611 *lt9611) +{ + regmap_write(lt9611->regmap, 0x8443, 0x46 - lt9611->vic); + regmap_write(lt9611->regmap, 0x8447, lt9611->vic); + regmap_write(lt9611->regmap, 0x843d, 0x0a); /* UD1 infoframe */ + + regmap_write(lt9611->regmap, 0x82d6, 0x8c); + regmap_write(lt9611->regmap, 0x82d7, 0x04); +} + +static void lt9611_hdmi_tx_phy(struct lt9611 *lt9611) +{ + struct reg_sequence reg_cfg[] = { + { 0x8130, 0x6a }, + { 0x8131, 0x44 }, /* HDMI DC mode */ + { 0x8132, 0x4a }, + { 0x8133, 0x0b }, + { 0x8134, 0x00 }, + { 0x8135, 0x00 }, + { 0x8136, 0x00 }, + { 0x8137, 0x44 }, + { 0x813f, 0x0f }, + { 0x8140, 0xa0 }, + { 0x8141, 0xa0 }, + { 0x8142, 0xa0 }, + { 0x8143, 0xa0 }, + { 0x8144, 0x0a }, + }; + + /* HDMI AC mode */ + if (lt9611->ac_mode) + reg_cfg[2].def = 0x73; + + regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg)); +} + +static irqreturn_t lt9611_irq_thread_handler(int irq, void *dev_id) +{ + struct lt9611 *lt9611 = dev_id; + unsigned int irq_flag0 = 0; + unsigned int irq_flag3 = 0; + + regmap_read(lt9611->regmap, 0x820f, &irq_flag3); + regmap_read(lt9611->regmap, 0x820c, &irq_flag0); + + pr_debug("%s() irq_flag0: %#x irq_flag3: %#x\n", + __func__, irq_flag0, irq_flag3); + + /* hpd changed low */ + if (irq_flag3 & 0x80) { + dev_info(lt9611->dev, "hdmi cable disconnected\n"); + + regmap_write(lt9611->regmap, 0x8207, 0xbf); + regmap_write(lt9611->regmap, 0x8207, 0x3f); + } + /* hpd changed high */ + if (irq_flag3 & 0x40) { + dev_info(lt9611->dev, "hdmi cable connected\n"); + + regmap_write(lt9611->regmap, 0x8207, 0x7f); + regmap_write(lt9611->regmap, 0x8207, 0x3f); + } + + if (irq_flag3 & 0xc0) + drm_kms_helper_hotplug_event(lt9611->bridge.dev); + + /* video input changed */ + if (irq_flag0 & 0x01) { + dev_info(lt9611->dev, "video input changed\n"); + regmap_write(lt9611->regmap, 0x829e, 0xff); + regmap_write(lt9611->regmap, 0x829e, 0xf7); + regmap_write(lt9611->regmap, 0x8204, 0xff); + regmap_write(lt9611->regmap, 0x8204, 0xfe); + } + + return IRQ_HANDLED; +} + +static void lt9611_enable_hpd_interrupts(struct lt9611 *lt9611) +{ + unsigned int val; + + dev_dbg(lt9611->dev, "enabling hpd interrupts\n"); + + regmap_read(lt9611->regmap, 0x8203, &val); + + val &= ~0xc0; + regmap_write(lt9611->regmap, 0x8203, val); + regmap_write(lt9611->regmap, 0x8207, 0xff); //clear + regmap_write(lt9611->regmap, 0x8207, 0x3f); +} + +static void lt9611_sleep_setup(struct lt9611 *lt9611) +{ + struct reg_sequence sleep_setup[] = { + { 0x8024, 0x76 }, + { 0x8023, 0x01 }, + { 0x8157, 0x03 }, //set addr pin as output + { 0x8149, 0x0b }, + { 0x8151, 0x30 }, //disable IRQ + { 0x8102, 0x48 }, //MIPI Rx power down + { 0x8123, 0x80 }, + { 0x8130, 0x00 }, + { 0x8100, 0x01 }, //bandgap power down + { 0x8101, 0x00 }, //system clk power down + }; + + dev_dbg(lt9611->dev, "sleep\n"); + + regmap_multi_reg_write(lt9611->regmap, + sleep_setup, ARRAY_SIZE(sleep_setup)); + lt9611->sleep = true; +} + +static int lt9611_power_on(struct lt9611 *lt9611) +{ + int ret; + const struct reg_sequence seq[] = { + /* LT9611_System_Init */ + { 0x8101, 0x18 }, /* sel xtal clock */ + + /* timer for frequency meter */ + { 0x821b, 0x69 }, /*timer 2*/ + { 0x821c, 0x78 }, + { 0x82cb, 0x69 }, /*timer 1 */ + { 0x82cc, 0x78 }, + + /* irq init */ + { 0x8251, 0x01 }, + { 0x8258, 0x0a }, /* hpd irq */ + { 0x8259, 0x80 }, /* hpd debounce width */ + { 0x829e, 0xf7 }, /* video check irq */ + + /* power consumption for work */ + { 0x8004, 0xf0 }, + { 0x8006, 0xf0 }, + { 0x800a, 0x80 }, + { 0x800b, 0x40 }, + { 0x800d, 0xef }, + { 0x8011, 0xfa }, + }; + + if (lt9611->power_on) + return 0; + + dev_dbg(lt9611->dev, "power on\n"); + + ret = regmap_multi_reg_write(lt9611->regmap, seq, ARRAY_SIZE(seq)); + if (!ret) + lt9611->power_on = true; + + return ret; +} + +static int lt9611_power_off(struct lt9611 *lt9611) +{ + int ret; + + dev_dbg(lt9611->dev, "power off\n"); + + ret = regmap_write(lt9611->regmap, 0x8130, 0x6a); + if (!ret) + lt9611->power_on = false; + + return ret; +} + +static void lt9611_reset(struct lt9611 *lt9611) +{ + gpiod_set_value_cansleep(lt9611->reset_gpio, 1); + msleep(20); + gpiod_set_value_cansleep(lt9611->reset_gpio, 0); + msleep(20); + gpiod_set_value_cansleep(lt9611->reset_gpio, 1); + msleep(100); +} + +static void lt9611_assert_5v(struct lt9611 *lt9611) +{ + if (!lt9611->enable_gpio) + return; + + gpiod_set_value_cansleep(lt9611->enable_gpio, 1); + msleep(20); +} + +static int lt9611_regulator_init(struct lt9611 *lt9611) +{ + int ret; + + lt9611->supplies[0].supply = "vdd"; + lt9611->supplies[1].supply = "vcc"; + ret = devm_regulator_bulk_get(lt9611->dev, 2, lt9611->supplies); + if (ret < 0) + return ret; + + return regulator_set_load(lt9611->supplies[0].consumer, 300000); +} + +static int lt9611_regulator_enable(struct lt9611 *lt9611) +{ + int ret; + + ret = regulator_enable(lt9611->supplies[0].consumer); + if (ret < 0) + return ret; + + usleep_range(1000, 10000); + + ret = regulator_enable(lt9611->supplies[1].consumer); + if (ret < 0) { + regulator_disable(lt9611->supplies[0].consumer); + return ret; + } + + return 0; +} + +static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) { + if (lt9611_modes[i].hdisplay == mode->hdisplay && + lt9611_modes[i].vdisplay == mode->vdisplay && + lt9611_modes[i].fps == drm_mode_vrefresh(mode)) { + return <9611_modes[i]; + } + } + + return NULL; +} + +/* connector funcs */ +static enum drm_connector_status +lt9611_connector_detect(struct drm_connector *connector, bool force) +{ + struct lt9611 *lt9611 = connector_to_lt9611(connector); + unsigned int reg_val = 0; + int connected = 0; + + regmap_read(lt9611->regmap, 0x825e, ®_val); + connected = (reg_val & BIT(2)); + dev_dbg(lt9611->dev, "connected = %x\n", connected); + + lt9611->status = connected ? connector_status_connected : + connector_status_disconnected; + + return lt9611->status; +} + +static int lt9611_read_edid(struct lt9611 *lt9611) +{ + unsigned int temp; + int ret = 0; + int i, j; + + memset(lt9611->edid_buf, 0, EDID_SEG_SIZE); + + regmap_write(lt9611->regmap, 0x8503, 0xc9); + + /* 0xA0 is EDID device address */ + regmap_write(lt9611->regmap, 0x8504, 0xa0); + /* 0x00 is EDID offset address */ + regmap_write(lt9611->regmap, 0x8505, 0x00); + /* length for read */ + regmap_write(lt9611->regmap, 0x8506, 0x20); + regmap_write(lt9611->regmap, 0x8514, 0x7f); + + for (i = 0 ; i < 8 ; i++) { + /* offset address */ + regmap_write(lt9611->regmap, 0x8505, i * 32); + regmap_write(lt9611->regmap, 0x8507, 0x36); + regmap_write(lt9611->regmap, 0x8507, 0x31); + regmap_write(lt9611->regmap, 0x8507, 0x37); + usleep_range(5000, 10000); + + regmap_read(lt9611->regmap, 0x8540, &temp); + + if (temp & 0x02) { /*KEY_DDC_ACCS_DONE=1*/ + for (j = 0; j < 32; j++) { + regmap_read(lt9611->regmap, 0x8583, &temp); + lt9611->edid_buf[i * 32 + j] = temp; + } + } else if (temp & 0x50) { /* DDC No Ack or Abitration lost */ + dev_err(lt9611->dev, "read edid failed: no ack\n"); + ret = -EIO; + goto end; + } else { + dev_err(lt9611->dev, + "read edid failed: access not done\n"); + ret = -EIO; + goto end; + } + } + + dev_dbg(lt9611->dev, "read edid succeeded, checksum = 0x%x\n", + lt9611->edid_buf[255]); + +end: + regmap_write(lt9611->regmap, 0x8507, 0x1f); + return ret; +} + +/* TODO: add support for more extension blocks */ +static int +lt9611_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len) +{ + struct lt9611 *lt9611 = data; + int ret; + + dev_dbg(lt9611->dev, "get edid block: block=%d, len=%d\n", + block, (int)len); + + if (len > 128) + return -EINVAL; + + /* support up to 1 extension block */ + if (block > 1) + return -EINVAL; + + if (block == 0) { + /* always read 2 edid blocks once */ + ret = lt9611_read_edid(lt9611); + if (ret) { + dev_err(lt9611->dev, "edid read failed\n"); + return ret; + } + } + + if (block % 2 == 0) + memcpy(buf, lt9611->edid_buf, len); + else + memcpy(buf, lt9611->edid_buf + 128, len); + + return 0; +} + +static int lt9611_connector_get_modes(struct drm_connector *connector) +{ + struct lt9611 *lt9611 = connector_to_lt9611(connector); + unsigned int count; + struct edid *edid; + + dev_dbg(lt9611->dev, "get modes\n"); + + lt9611_power_on(lt9611); + edid = drm_do_get_edid(connector, lt9611_get_edid_block, lt9611); + drm_connector_update_edid_property(connector, edid); + count = drm_add_edid_modes(connector, edid); + kfree(edid); + + return count; +} + +static enum drm_mode_status +lt9611_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) +{ + struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode); + + return lt9611_mode ? MODE_OK : MODE_BAD; +} + +/* bridge funcs */ +static void lt9611_bridge_enable(struct drm_bridge *bridge) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + + dev_dbg(lt9611->dev, "bridge enable\n"); + + if (lt9611_power_on(lt9611)) { + dev_err(lt9611->dev, "power on failed\n"); + return; + } + + dev_dbg(lt9611->dev, "video on\n"); + + lt9611_mipi_input_analog(lt9611); + lt9611_hdmi_tx_digital(lt9611); + lt9611_hdmi_tx_phy(lt9611); + + msleep(500); + + lt9611_video_check(lt9611); + + /* Enable HDMI output */ + regmap_write(lt9611->regmap, 0x8130, 0xea); +} + +static void lt9611_bridge_disable(struct drm_bridge *bridge) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + int ret; + + dev_dbg(lt9611->dev, "bridge disable\n"); + + /* Disable HDMI output */ + ret = regmap_write(lt9611->regmap, 0x8130, 0x6a); + if (ret) { + dev_err(lt9611->dev, "video on failed\n"); + return; + } + + if (lt9611_power_off(lt9611)) { + dev_err(lt9611->dev, "power on failed\n"); + return; + } +} + +static struct +drm_connector_helper_funcs lt9611_bridge_connector_helper_funcs = { + .get_modes = lt9611_connector_get_modes, + .mode_valid = lt9611_connector_mode_valid, +}; + +static const struct drm_connector_funcs lt9611_bridge_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .detect = lt9611_connector_detect, + .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 struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611, + struct device_node *dsi_node) +{ + const struct mipi_dsi_device_info info = { "lt9611", 0, NULL }; + struct mipi_dsi_device *dsi; + struct mipi_dsi_host *host; + int ret; + + host = of_find_mipi_dsi_host_by_node(dsi_node); + if (!host) { + dev_err(lt9611->dev, "failed to find dsi host\n"); + return ERR_PTR(-EPROBE_DEFER); + } + + dsi = mipi_dsi_device_register_full(host, &info); + if (IS_ERR(dsi)) { + dev_err(lt9611->dev, "failed to create dsi device\n"); + return dsi; + } + + dsi->lanes = 4; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | + MIPI_DSI_MODE_VIDEO_HSE; + + ret = mipi_dsi_attach(dsi); + if (ret < 0) { + dev_err(lt9611->dev, "failed to attach dsi to host\n"); + mipi_dsi_device_unregister(dsi); + return ERR_PTR(ret); + } + + return dsi; +} + +static int lt9611_bridge_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + int ret; + + dev_dbg(lt9611->dev, "bridge attach\n"); + + ret = drm_connector_init(bridge->dev, <9611->connector, + <9611_bridge_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); + if (ret) { + DRM_ERROR("Failed to initialize connector with drm\n"); + return ret; + } + + drm_connector_helper_add(<9611->connector, + <9611_bridge_connector_helper_funcs); + drm_connector_attach_encoder(<9611->connector, bridge->encoder); + + if (!bridge->encoder) { + DRM_ERROR("Parent encoder object not found"); + return -ENODEV; + } + + /* Attach primary DSI */ + lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node); + if (IS_ERR(lt9611->dsi0)) + return PTR_ERR(lt9611->dsi0); + + /* Attach secondary DSI, if specified */ + if (lt9611->dsi1_node) { + lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node); + if (IS_ERR(lt9611->dsi1)) { + ret = PTR_ERR(lt9611->dsi1); + goto err_unregister_dsi0; + } + } + + return 0; + +err_unregister_dsi0: + mipi_dsi_device_unregister(lt9611->dsi0); + + return ret; +} + +static void lt9611_bridge_detach(struct drm_bridge *bridge) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + + if (lt9611->dsi1) { + mipi_dsi_detach(lt9611->dsi1); + mipi_dsi_device_unregister(lt9611->dsi1); + } + + mipi_dsi_detach(lt9611->dsi0); + mipi_dsi_device_unregister(lt9611->dsi0); +} + +static enum drm_mode_status +lt9611_bridge_mode_valid(struct drm_bridge *bridge, + const struct drm_display_mode *mode) +{ + struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode); + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + + if (lt9611_mode->intfs > 1 && !lt9611->dsi1) + return MODE_PANEL; + else + return MODE_OK; +} + +static void lt9611_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + + dev_dbg(lt9611->dev, "bridge pre_enable\n"); + + if (!lt9611->sleep) + return; + + lt9611_reset(lt9611); + regmap_write(lt9611->regmap, 0x80ee, 0x01); + + lt9611->sleep = false; +} + +static void lt9611_bridge_post_disable(struct drm_bridge *bridge) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + + dev_dbg(lt9611->dev, "bridge post_disable\n"); + + lt9611_sleep_setup(lt9611); +} + +static void lt9611_bridge_mode_set(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + const struct drm_display_mode *adj_mode) +{ + struct lt9611 *lt9611 = bridge_to_lt9611(bridge); + struct hdmi_avi_infoframe avi_frame; + int ret; + + dev_dbg(lt9611->dev, "bridge mode_set: hdisplay=%d, vdisplay=%d, vrefresh=%d, clock=%d\n", + adj_mode->hdisplay, adj_mode->vdisplay, + adj_mode->vrefresh, adj_mode->clock); + + lt9611_bridge_pre_enable(bridge); + + lt9611_mipi_input_digital(lt9611, mode); + lt9611_pll_setup(lt9611, mode); + lt9611_mipi_video_setup(lt9611, mode); + lt9611_pcr_setup(lt9611, mode); + + ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame, + <9611->connector, + mode); + if (!ret) + lt9611->vic = avi_frame.video_code; +} + +static const struct drm_bridge_funcs lt9611_bridge_funcs = { + .attach = lt9611_bridge_attach, + .detach = lt9611_bridge_detach, + .mode_valid = lt9611_bridge_mode_valid, + .enable = lt9611_bridge_enable, + .disable = lt9611_bridge_disable, + .post_disable = lt9611_bridge_post_disable, + .mode_set = lt9611_bridge_mode_set, +}; + +static int lt9611_parse_dt(struct device *dev, + struct lt9611 *lt9611) +{ + lt9611->dsi0_node = of_graph_get_remote_node(dev->of_node, 1, -1); + if (!lt9611->dsi0_node) { + DRM_DEV_ERROR(dev, "failed to get remote node for primary dsi\n"); + return -ENODEV; + } + + lt9611->dsi1_node = of_graph_get_remote_node(dev->of_node, 2, -1); + + lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode"); + dev_dbg(lt9611->dev, "ac_mode=%d\n", lt9611->ac_mode); + + return 0; +} + +static int lt9611_gpio_init(struct lt9611 *lt9611) +{ + struct device *dev = lt9611->dev; + + lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(lt9611->reset_gpio)) { + dev_err(dev, "failed to acquire reset gpio\n"); + return PTR_ERR(lt9611->reset_gpio); + } + + lt9611->enable_gpio = devm_gpiod_get_optional(dev, "enable", + GPIOD_OUT_LOW); + if (IS_ERR(lt9611->enable_gpio)) { + dev_err(dev, "failed to acquire enable gpio\n"); + return PTR_ERR(lt9611->enable_gpio); + } + + return 0; +} + +static int lt9611_read_device_rev(struct lt9611 *lt9611) +{ + unsigned int rev; + int ret; + + regmap_write(lt9611->regmap, 0x80ee, 0x01); + ret = regmap_read(lt9611->regmap, 0x8002, &rev); + if (ret) + dev_err(lt9611->dev, "failed to read revision: %d\n", ret); + + dev_info(lt9611->dev, "LT9611 revision: 0x%x\n", rev); + + return ret; +} + +static int lt9611_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct lt9611 *lt9611; + struct device *dev = &client->dev; + int ret; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { + dev_err(dev, "device doesn't support I2C\n"); + return -ENODEV; + } + + lt9611 = devm_kzalloc(dev, sizeof(*lt9611), GFP_KERNEL); + if (!lt9611) + return -ENOMEM; + + lt9611->dev = &client->dev; + lt9611->client = client; + lt9611->sleep = false; + + lt9611->regmap = devm_regmap_init_i2c(client, <9611_regmap_config); + if (IS_ERR(lt9611->regmap)) { + DRM_ERROR("regmap i2c init failed\n"); + return PTR_ERR(lt9611->regmap); + } + + ret = lt9611_parse_dt(&client->dev, lt9611); + if (ret) { + dev_err(dev, "failed to parse device tree\n"); + return ret; + } + + ret = lt9611_gpio_init(lt9611); + if (ret < 0) + return ret; + + ret = lt9611_regulator_init(lt9611); + if (ret < 0) + return ret; + + lt9611_assert_5v(lt9611); + + ret = lt9611_regulator_enable(lt9611); + if (ret) + return ret; + + lt9611_reset(lt9611); + + ret = lt9611_read_device_rev(lt9611); + if (ret) { + dev_err(dev, "failed to read chip rev\n"); + goto err_disable_regulators; + } + + ret = devm_request_threaded_irq(dev, client->irq, NULL, + lt9611_irq_thread_handler, + IRQF_ONESHOT, "lt9611", lt9611); + if (ret) { + dev_err(dev, "failed to request irq\n"); + goto err_disable_regulators; + } + + i2c_set_clientdata(client, lt9611); + + lt9611->bridge.funcs = <9611_bridge_funcs; + lt9611->bridge.of_node = client->dev.of_node; + + drm_bridge_add(<9611->bridge); + + lt9611_enable_hpd_interrupts(lt9611); + + return 0; + +err_disable_regulators: + regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies); + + of_node_put(lt9611->dsi0_node); + of_node_put(lt9611->dsi1_node); + + return ret; +} + +static int lt9611_remove(struct i2c_client *client) +{ + struct lt9611 *lt9611 = i2c_get_clientdata(client); + + disable_irq(client->irq); + drm_bridge_remove(<9611->bridge); + + regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies); + + of_node_put(lt9611->dsi0_node); + of_node_put(lt9611->dsi1_node); + + return 0; +} + +static struct i2c_device_id lt9611_id[] = { + { "lontium,lt9611", 0}, + {} +}; + +static const struct of_device_id lt9611_match_table[] = { + {.compatible = "lontium,lt9611"}, + {} +}; +MODULE_DEVICE_TABLE(of, lt9611_match_table); + +static struct i2c_driver lt9611_driver = { + .driver = { + .name = "lt9611", + .of_match_table = lt9611_match_table, + }, + .probe = lt9611_probe, + .remove = lt9611_remove, + .id_table = lt9611_id, +}; +module_i2c_driver(lt9611_driver); + +MODULE_LICENSE("GPL v2");
Hi Vinod,
I love your patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next] [also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip linus/master v5.7-rc5 next-20200512] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vinod-Koul/Add-LT9611-DSI-to-HDMI-b... base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: nios2-allyesconfig (attached as .config) compiler: nios2-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/bridge/lt9611.c: In function 'lt9611_reset': drivers/gpu/drm/bridge/lt9611.c:518:2: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration] 518 | gpiod_set_value_cansleep(lt9611->reset_gpio, 1); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c: In function 'lt9611_gpio_init': drivers/gpu/drm/bridge/lt9611.c:963:23: error: implicit declaration of function 'devm_gpiod_get' [-Werror=implicit-function-declaration] 963 | lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); | ^~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:963:52: error: 'GPIOD_OUT_HIGH' undeclared (first use in this function) 963 | lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); | ^~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:963:52: note: each undeclared identifier is reported only once for each function it appears in drivers/gpu/drm/bridge/lt9611.c:969:24: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_regulator_get_optional'? [-Werror=implicit-function-declaration] 969 | lt9611->enable_gpio = devm_gpiod_get_optional(dev, "enable", | ^~~~~~~~~~~~~~~~~~~~~~~ | devm_regulator_get_optional drivers/gpu/drm/bridge/lt9611.c:970:13: error: 'GPIOD_OUT_LOW' undeclared (first use in this function) 970 | GPIOD_OUT_LOW); | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c: At top level: drivers/gpu/drm/bridge/lt9611.c:1100:1: warning: data definition has no type or storage class 1100 | MODULE_DEVICE_TABLE(of, lt9611_match_table); | ^~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1100:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int] drivers/gpu/drm/bridge/lt9611.c:1100:1: warning: parameter names (without types) in function declaration In file included from include/linux/device.h:31, from include/linux/platform_device.h:13, from drivers/gpu/drm/bridge/lt9611.c:7: include/linux/device/driver.h:263:1: warning: data definition has no type or storage class 263 | module_init(__driver##_init); | ^~~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ include/linux/device/driver.h:263:1: error: type defaults to 'int' in declaration of 'module_init' [-Werror=implicit-int] 263 | module_init(__driver##_init); | ^~~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ In file included from include/linux/linkage.h:7, from include/linux/kernel.h:8, from include/asm-generic/bug.h:19, from ./arch/nios2/include/generated/asm/bug.h:1, from include/linux/bug.h:5, from include/linux/thread_info.h:12, from include/asm-generic/current.h:5, from ./arch/nios2/include/generated/asm/current.h:1, from include/linux/sched.h:12, from include/linux/ratelimit.h:6, from include/linux/dev_printk.h:16, from include/linux/device.h:15, from include/linux/platform_device.h:13, from drivers/gpu/drm/bridge/lt9611.c:7:
include/linux/export.h:19:30: warning: parameter names (without types) in function declaration
19 | #define THIS_MODULE ((struct module *)0) | ^~~~~~ include/linux/i2c.h:855:22: note: in expansion of macro 'THIS_MODULE' 855 | i2c_register_driver(THIS_MODULE, driver) | ^~~~~~~~~~~ include/linux/device/driver.h:261:9: note: in expansion of macro 'i2c_add_driver' 261 | return __register(&(__driver) , ##__VA_ARGS__); | ^~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ In file included from include/linux/device.h:31, from include/linux/platform_device.h:13, from drivers/gpu/drm/bridge/lt9611.c:7: include/linux/device/driver.h:268:1: warning: data definition has no type or storage class 268 | module_exit(__driver##_exit); | ^~~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ include/linux/device/driver.h:268:1: error: type defaults to 'int' in declaration of 'module_exit' [-Werror=implicit-int] 268 | module_exit(__driver##_exit); | ^~~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ In file included from include/linux/linkage.h:7, from include/linux/kernel.h:8, from include/asm-generic/bug.h:19, from ./arch/nios2/include/generated/asm/bug.h:1, from include/linux/bug.h:5, from include/linux/thread_info.h:12, from include/asm-generic/current.h:5, from ./arch/nios2/include/generated/asm/current.h:1, from include/linux/sched.h:12, from include/linux/ratelimit.h:6, from include/linux/dev_printk.h:16, from include/linux/device.h:15, from include/linux/platform_device.h:13, from drivers/gpu/drm/bridge/lt9611.c:7:
include/linux/export.h:19:30: warning: parameter names (without types) in function declaration
19 | #define THIS_MODULE ((struct module *)0) | ^~~~~~ include/linux/i2c.h:855:22: note: in expansion of macro 'THIS_MODULE' 855 | i2c_register_driver(THIS_MODULE, driver) | ^~~~~~~~~~~ include/linux/device/driver.h:261:9: note: in expansion of macro 'i2c_add_driver' 261 | return __register(&(__driver) , ##__VA_ARGS__); | ^~~~~~~~~~ include/linux/i2c.h:923:2: note: in expansion of macro 'module_driver' 923 | module_driver(__i2c_driver, i2c_add_driver, | ^~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1113:16: error: expected declaration specifiers or '...' before string constant 1113 | MODULE_LICENSE("GPL v2"); | ^~~~~~~~ In file included from include/linux/device.h:31, from include/linux/platform_device.h:13, from drivers/gpu/drm/bridge/lt9611.c:7: drivers/gpu/drm/bridge/lt9611.c:1111:19: warning: 'lt9611_driver_init' defined but not used [-Wunused-function] 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~ include/linux/device/driver.h:259:19: note: in definition of macro 'module_driver' 259 | static int __init __driver##_init(void) | ^~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:1111:1: note: in expansion of macro 'module_i2c_driver' 1111 | module_i2c_driver(lt9611_driver); | ^~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors
vim +19 include/linux/export.h
b67067f1176df6 Nicholas Piggin 2016-08-24 4 f50169324df4ad Paul Gortmaker 2011-05-23 5 /* f50169324df4ad Paul Gortmaker 2011-05-23 6 * Export symbols from the kernel to modules. Forked from module.h f50169324df4ad Paul Gortmaker 2011-05-23 7 * to reduce the amount of pointless cruft we feed to gcc when only f50169324df4ad Paul Gortmaker 2011-05-23 8 * exporting a simple symbol or two. f50169324df4ad Paul Gortmaker 2011-05-23 9 * b92021b09df70c Rusty Russell 2013-03-15 10 * Try not to add #includes here. It slows compilation and makes kernel b92021b09df70c Rusty Russell 2013-03-15 11 * hackers place grumpy comments in header files. f50169324df4ad Paul Gortmaker 2011-05-23 12 */ f50169324df4ad Paul Gortmaker 2011-05-23 13 b92021b09df70c Rusty Russell 2013-03-15 14 #ifndef __ASSEMBLY__ f50169324df4ad Paul Gortmaker 2011-05-23 15 #ifdef MODULE f50169324df4ad Paul Gortmaker 2011-05-23 16 extern struct module __this_module; f50169324df4ad Paul Gortmaker 2011-05-23 17 #define THIS_MODULE (&__this_module) f50169324df4ad Paul Gortmaker 2011-05-23 18 #else f50169324df4ad Paul Gortmaker 2011-05-23 @19 #define THIS_MODULE ((struct module *)0) f50169324df4ad Paul Gortmaker 2011-05-23 20 #endif f50169324df4ad Paul Gortmaker 2011-05-23 21
:::::: The code at line 19 was first introduced by commit :::::: f50169324df4ad942e544386d136216c8617636a module.h: split out the EXPORT_SYMBOL into export.h
:::::: TO: Paul Gortmaker paul.gortmaker@windriver.com :::::: CC: Paul Gortmaker paul.gortmaker@windriver.com
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Vinod,
I love your patch! Yet something to improve:
[auto build test ERROR on robh/for-next] [also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip linus/master v5.7-rc5 next-20200512] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vinod-Koul/Add-LT9611-DSI-to-HDMI-b... base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: i386-allyesconfig (attached as .config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=i386
If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/bridge/lt9611.c: In function 'lt9611_reset':
drivers/gpu/drm/bridge/lt9611.c:518:2: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
gpiod_set_value_cansleep(lt9611->reset_gpio, 1); ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c: In function 'lt9611_gpio_init':
drivers/gpu/drm/bridge/lt9611.c:963:23: error: implicit declaration of function 'devm_gpiod_get'; did you mean 'devm_iounmap'? [-Werror=implicit-function-declaration]
lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); ^~~~~~~~~~~~~~ devm_iounmap
drivers/gpu/drm/bridge/lt9611.c:963:52: error: 'GPIOD_OUT_HIGH' undeclared (first use in this function)
lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); ^~~~~~~~~~~~~~ drivers/gpu/drm/bridge/lt9611.c:963:52: note: each undeclared identifier is reported only once for each function it appears in
drivers/gpu/drm/bridge/lt9611.c:969:24: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_regulator_get_optional'? [-Werror=implicit-function-declaration]
lt9611->enable_gpio = devm_gpiod_get_optional(dev, "enable", ^~~~~~~~~~~~~~~~~~~~~~~ devm_regulator_get_optional
drivers/gpu/drm/bridge/lt9611.c:970:13: error: 'GPIOD_OUT_LOW' undeclared (first use in this function); did you mean 'GPIOD_OUT_HIGH'?
GPIOD_OUT_LOW); ^~~~~~~~~~~~~ GPIOD_OUT_HIGH cc1: some warnings being treated as errors
vim +/gpiod_set_value_cansleep +518 drivers/gpu/drm/bridge/lt9611.c
515 516 static void lt9611_reset(struct lt9611 *lt9611) 517 {
518 gpiod_set_value_cansleep(lt9611->reset_gpio, 1);
519 msleep(20); 520 gpiod_set_value_cansleep(lt9611->reset_gpio, 0); 521 msleep(20); 522 gpiod_set_value_cansleep(lt9611->reset_gpio, 1); 523 msleep(100); 524 } 525
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Vinod,
Few high-level comments: - handful of functions always return 0 and the return value is never checked - switch to return void - annotate all (nearly) arrays as static const - consistently use multi_reg_write - in some cases non-const array will be fine, overwriting a few entries as needed - there is very partial comments about the registers/values - missing docs or?
Personally I'm in favour of using symbolic names, instead of hex+comment. Considering how partial the comments are, current approach is perfectly fine.
On Wed, 13 May 2020 at 11:06, Vinod Koul vkoul@kernel.org wrote:
Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and I2S port as an input and HDMI port as output
Co-developed-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Vinod Koul vkoul@kernel.org
--- /dev/null +++ b/drivers/gpu/drm/bridge/lt9611.c
Please add a vendor prefix to the filename.
@@ -0,0 +1,1113 @@
+struct lt9611_mode {
u16 hdisplay;u16 vdisplay;u8 fps;
We all enjoy the odd fps game, but let's use vrefresh here.
u8 lanes;u8 intfs;+};
+static int lt9611_mipi_input_digital(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
regmap_write(lt9611->regmap, 0x8300, LT9611_4LANES);if (mode->hdisplay == 3840)regmap_write(lt9611->regmap, 0x830a, 0x03);elseregmap_write(lt9611->regmap, 0x830a, 0x00);regmap_write(lt9611->regmap, 0x824f, 0x80);regmap_write(lt9611->regmap, 0x8250, 0x10);regmap_write(lt9611->regmap, 0x8302, 0x0a);regmap_write(lt9611->regmap, 0x8306, 0x0a);
Create an (non-const) array, overwriting the [1] entry for 3840 mode?
return 0;
Kill return type.
+}
+static int lt9611_pcr_setup(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
struct reg_sequence reg_cfg[] = {
static const?
{ 0x830b, 0x01 },{ 0x830c, 0x10 },{ 0x8348, 0x00 },{ 0x8349, 0x81 },/* stage 1 */{ 0x8321, 0x4a },{ 0x8324, 0x71 },{ 0x8325, 0x30 },{ 0x832a, 0x01 },/* stage 2 */{ 0x834a, 0x40 },{ 0x831d, 0x10 },/* MK limit */{ 0x832d, 0x38 },{ 0x8331, 0x08 },};regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg));switch (mode->hdisplay) {case 640:regmap_write(lt9611->regmap, 0x8326, 0x14);break;case 1920:regmap_write(lt9611->regmap, 0x8326, 0x37);break;case 3840:regmap_write(lt9611->regmap, 0x830b, 0x03);regmap_write(lt9611->regmap, 0x830c, 0xd0);regmap_write(lt9611->regmap, 0x8348, 0x03);regmap_write(lt9611->regmap, 0x8349, 0xe0);regmap_write(lt9611->regmap, 0x8324, 0x72);regmap_write(lt9611->regmap, 0x8325, 0x00);regmap_write(lt9611->regmap, 0x832a, 0x01);regmap_write(lt9611->regmap, 0x834a, 0x10);regmap_write(lt9611->regmap, 0x831d, 0x10);regmap_write(lt9611->regmap, 0x8326, 0x37);
Throw this in another const array?
break;}/* pcr rst */regmap_write(lt9611->regmap, 0x8011, 0x5a);regmap_write(lt9611->regmap, 0x8011, 0xfa);return 0;+}
regmap_write(lt9611->regmap, 0x82e3, pclk >> 17); /* pclk[19:16] */regmap_write(lt9611->regmap, 0x82e4, pclk >> 9); /* pclk[15:8] */regmap_write(lt9611->regmap, 0x82e5, pclk >> 1); /* pclk[7:0] */
Comment does not match the code. We're discarding the LSB, so we cannot realistically be writing pclk[7:0]. Similar applies for the other two.
/* v_act */ret = regmap_read(lt9611->regmap, 0x8282, &temp);if (ret)goto end;v_act = temp << 8;ret = regmap_read(lt9611->regmap, 0x8283, &temp);if (ret)goto end;v_act = v_act + temp;
Having a helper for the above "result = read(x) << 8 | read(x+1)" would be great. This way one doesn't have to repeat the pattern 4-5 times.
+static int lt9611_read_edid(struct lt9611 *lt9611) +{
unsigned int temp;int ret = 0;int i, j;memset(lt9611->edid_buf, 0, EDID_SEG_SIZE);
How about: memset(lt9611->edid_buf, 0, sizeof(lt9611->edid_buf));
Then again, do we need the memset()? We are allocating the memory with devm_kzalloc()
regmap_write(lt9611->regmap, 0x8503, 0xc9);/* 0xA0 is EDID device address */regmap_write(lt9611->regmap, 0x8504, 0xa0);/* 0x00 is EDID offset address */regmap_write(lt9611->regmap, 0x8505, 0x00);/* length for read */regmap_write(lt9611->regmap, 0x8506, 0x20);
Is this the same 32 as seen in the loops below? #define and use consistently?
regmap_write(lt9611->regmap, 0x8514, 0x7f);for (i = 0 ; i < 8 ; i++) {
Add a #define for the magic 8
/* offset address */regmap_write(lt9611->regmap, 0x8505, i * 32);regmap_write(lt9611->regmap, 0x8507, 0x36);regmap_write(lt9611->regmap, 0x8507, 0x31);regmap_write(lt9611->regmap, 0x8507, 0x37);usleep_range(5000, 10000);regmap_read(lt9611->regmap, 0x8540, &temp);if (temp & 0x02) { /*KEY_DDC_ACCS_DONE=1*/
Use #define KEY_DDC_ACCS_DONE 0x02
for (j = 0; j < 32; j++) {
Another #define for 32
regmap_read(lt9611->regmap, 0x8583, &temp);lt9611->edid_buf[i * 32 + j] = temp;}} else if (temp & 0x50) { /* DDC No Ack or Abitration lost */dev_err(lt9611->dev, "read edid failed: no ack\n");ret = -EIO;goto end;} else {dev_err(lt9611->dev,"read edid failed: access not done\n");ret = -EIO;goto end;}}dev_dbg(lt9611->dev, "read edid succeeded, checksum = 0x%x\n",lt9611->edid_buf[255]);+end:
regmap_write(lt9611->regmap, 0x8507, 0x1f);return ret;+}
+/* TODO: add support for more extension blocks */ +static int +lt9611_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len) +{
struct lt9611 *lt9611 = data;int ret;dev_dbg(lt9611->dev, "get edid block: block=%d, len=%d\n",block, (int)len);if (len > 128)return -EINVAL;/* support up to 1 extension block */
Move the TODO here?
if (block > 1)return -EINVAL;if (block == 0) {/* always read 2 edid blocks once */
Please mention why that's a good idea. From memory - there aren't many other drivers that do this.
ret = lt9611_read_edid(lt9611);if (ret) {dev_err(lt9611->dev, "edid read failed\n");return ret;}}if (block % 2 == 0)memcpy(buf, lt9611->edid_buf, len);elsememcpy(buf, lt9611->edid_buf + 128, len);
The above can be written as: memcpy(buf, lt9611->edid_buf + (block * 128), len);
return 0;+}
+static int lt9611_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)+{
/* Attach secondary DSI, if specified */if (lt9611->dsi1_node) {lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node);if (IS_ERR(lt9611->dsi1)) {ret = PTR_ERR(lt9611->dsi1);goto err_unregister_dsi0;}}return 0;+err_unregister_dsi0:
Missing detach? If possible directly use lt9611_bridge_detach().
mipi_dsi_device_unregister(lt9611->dsi0);return ret;+}
+static int lt9611_read_device_rev(struct lt9611 *lt9611) +{
unsigned int rev;int ret;regmap_write(lt9611->regmap, 0x80ee, 0x01);ret = regmap_read(lt9611->regmap, 0x8002, &rev);if (ret)dev_err(lt9611->dev, "failed to read revision: %d\n", ret);
The "failed" message will be followed by printing random kernel memory. Initialize rev to some dummy number or omit the dev_info.
dev_info(lt9611->dev, "LT9611 revision: 0x%x\n", rev);return ret;+}
+static int lt9611_probe(struct i2c_client *client,
const struct i2c_device_id *id)+{
ret = lt9611_parse_dt(&client->dev, lt9611);if (ret) {dev_err(dev, "failed to parse device tree\n");return ret;}ret = lt9611_gpio_init(lt9611);if (ret < 0)
Missing of_node_put() here and for the next few error paths.
return ret;ret = lt9611_regulator_init(lt9611);if (ret < 0)return ret;lt9611_assert_5v(lt9611);ret = lt9611_regulator_enable(lt9611);if (ret)return ret;
return 0;+err_disable_regulators:
regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);of_node_put(lt9611->dsi0_node);of_node_put(lt9611->dsi1_node);
Use the inverse order wrt the get() operation.
return ret;+}
+static int lt9611_remove(struct i2c_client *client) +{
struct lt9611 *lt9611 = i2c_get_clientdata(client);disable_irq(client->irq);drm_bridge_remove(<9611->bridge);regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);of_node_put(lt9611->dsi0_node);of_node_put(lt9611->dsi1_node);
Flip the order - dsi1, then dsi0
return 0;+}
+static struct i2c_device_id lt9611_id[] = {
{ "lontium,lt9611", 0},{}+};
+static const struct of_device_id lt9611_match_table[] = {
{.compatible = "lontium,lt9611"},
In the above two - add space after { and before }. Pretty sure ./scripts/checkpatch.pl will complain about those. Might want to double-check for other issues reported by said tool.
-Emil
Hello Emil,
Thanks for the comments.
On 13-05-20, 20:20, Emil Velikov wrote:
Hi Vinod,
Few high-level comments:
- handful of functions always return 0 and the return value is never
checked - switch to return void
Sure makes sense, will do
- annotate all (nearly) arrays as static const
Will do
- consistently use multi_reg_write - in some cases non-const array
will be fine, overwriting a few entries as needed
Okay that makes sense
- there is very partial comments about the registers/values - missing docs or?
yeah am not a big fan either, problem is documentation.
Well the spec I have doesn't have register names and few registers are missing :( I have few name created but naming registers turned nasty super quick.. Do let me know if you have suggestions, I will give it one more shot though
Personally I'm in favour of using symbolic names, instead of hex+comment. Considering how partial the comments are, current approach is perfectly fine.
On Wed, 13 May 2020 at 11:06, Vinod Koul vkoul@kernel.org wrote:
Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and I2S port as an input and HDMI port as output
Co-developed-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Vinod Koul vkoul@kernel.org
--- /dev/null +++ b/drivers/gpu/drm/bridge/lt9611.c
Please add a vendor prefix to the filename.
Okay
+struct lt9611_mode {
u16 hdisplay;u16 vdisplay;u8 fps;We all enjoy the odd fps game, but let's use vrefresh here.
Sure will change
+static int lt9611_mipi_input_digital(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
regmap_write(lt9611->regmap, 0x8300, LT9611_4LANES);if (mode->hdisplay == 3840)regmap_write(lt9611->regmap, 0x830a, 0x03);elseregmap_write(lt9611->regmap, 0x830a, 0x00);regmap_write(lt9611->regmap, 0x824f, 0x80);regmap_write(lt9611->regmap, 0x8250, 0x10);regmap_write(lt9611->regmap, 0x8302, 0x0a);regmap_write(lt9611->regmap, 0x8306, 0x0a);Create an (non-const) array, overwriting the [1] entry for 3840 mode?
So array is the recommendation, I dont have much liking for them but I can see they would be useful here, so will change this and other instances and we can use regmap_multi_reg_write() while taking care of static const for non modified arrays
return 0;Kill return type.
Yup, here and other places
regmap_write(lt9611->regmap, 0x82e3, pclk >> 17); /* pclk[19:16] */regmap_write(lt9611->regmap, 0x82e4, pclk >> 9); /* pclk[15:8] */regmap_write(lt9611->regmap, 0x82e5, pclk >> 1); /* pclk[7:0] */Comment does not match the code. We're discarding the LSB, so we cannot realistically be writing pclk[7:0]. Similar applies for the other two.
Thanks for pointing, will fix it up
/* v_act */ret = regmap_read(lt9611->regmap, 0x8282, &temp);if (ret)goto end;v_act = temp << 8;ret = regmap_read(lt9611->regmap, 0x8283, &temp);if (ret)goto end;v_act = v_act + temp;Having a helper for the above "result = read(x) << 8 | read(x+1)" would be great. This way one doesn't have to repeat the pattern 4-5 times.
will add
+static int lt9611_read_edid(struct lt9611 *lt9611) +{
unsigned int temp;int ret = 0;int i, j;memset(lt9611->edid_buf, 0, EDID_SEG_SIZE);How about: memset(lt9611->edid_buf, 0, sizeof(lt9611->edid_buf));
Then again, do we need the memset()? We are allocating the memory with devm_kzalloc()
Yes but lt9611_read_edid() is called multiple times so would make sense to memset it, will modify this to sizeof.
regmap_write(lt9611->regmap, 0x8503, 0xc9);/* 0xA0 is EDID device address */regmap_write(lt9611->regmap, 0x8504, 0xa0);/* 0x00 is EDID offset address */regmap_write(lt9611->regmap, 0x8505, 0x00);/* length for read */regmap_write(lt9611->regmap, 0x8506, 0x20);Is this the same 32 as seen in the loops below? #define and use consistently?
Sure will use defines here and other places
if (block > 1)return -EINVAL;if (block == 0) {/* always read 2 edid blocks once */Please mention why that's a good idea. From memory - there aren't many other drivers that do this.
Okay will find the reason for this and
ret = lt9611_read_edid(lt9611);if (ret) {dev_err(lt9611->dev, "edid read failed\n");return ret;}}if (block % 2 == 0)memcpy(buf, lt9611->edid_buf, len);elsememcpy(buf, lt9611->edid_buf + 128, len);The above can be written as: memcpy(buf, lt9611->edid_buf + (block * 128), len);
correct
/* Attach secondary DSI, if specified */if (lt9611->dsi1_node) {lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node);if (IS_ERR(lt9611->dsi1)) {ret = PTR_ERR(lt9611->dsi1);goto err_unregister_dsi0;}}return 0;+err_unregister_dsi0:
Missing detach? If possible directly use lt9611_bridge_detach().
will update
+static int lt9611_read_device_rev(struct lt9611 *lt9611) +{
unsigned int rev;int ret;regmap_write(lt9611->regmap, 0x80ee, 0x01);ret = regmap_read(lt9611->regmap, 0x8002, &rev);if (ret)dev_err(lt9611->dev, "failed to read revision: %d\n", ret);The "failed" message will be followed by printing random kernel memory. Initialize rev to some dummy number or omit the dev_info.
Am printing the 'ret' error code here and not the uninitialized rev, so I guess this one should be fine
ret = lt9611_parse_dt(&client->dev, lt9611);if (ret) {dev_err(dev, "failed to parse device tree\n");return ret;}ret = lt9611_gpio_init(lt9611);if (ret < 0)Missing of_node_put() here and for the next few error paths.
Yes this should be replaced by jump to of_node_put below
+static const struct of_device_id lt9611_match_table[] = {
{.compatible = "lontium,lt9611"},In the above two - add space after { and before }. Pretty sure
Correct, will fix this.
./scripts/checkpatch.pl will complain about those. Might want to double-check for other issues reported by said tool.
Somehow that was not the case :( I always run checkpatch.pl with --strict option. I have 1 warn about 80 char limit for a error message which I have ignored :)
Hi Vinod,
Thank you for the patch.
On Wed, May 13, 2020 at 03:35:33PM +0530, Vinod Koul wrote:
Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and I2S port as an input and HDMI port as output
Co-developed-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/gpu/drm/bridge/Kconfig | 13 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/lt9611.c | 1113 +++++++++++++++++++++++++++++++ 3 files changed, 1127 insertions(+) create mode 100644 drivers/gpu/drm/bridge/lt9611.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index aaed2347ace9..5cac15ce2027 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -38,6 +38,19 @@ 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_LT9611
- tristate "Lontium LT9611 DSI/HDMI bridge"
- select SND_SOC_HDMI_CODEC if SND_SOC
- depends on OF
- select DRM_PANEL_BRIDGE
- select DRM_KMS_HELPER
- select REGMAP_I2C
- help
Driver for Lontium LT9611 DSI to HDMI bridgechip driver that converts dual DSI and I2S toHDMI signalsPlease say Y if you have such hardware.config DRM_LVDS_CODEC tristate "Transparent LVDS encoders and decoders support" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 6fb062b5b0f0..d2a696e8ca5d 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o +obj-$(CONFIG_DRM_LONTIUM_LT9611) += lt9611.o obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o diff --git a/drivers/gpu/drm/bridge/lt9611.c b/drivers/gpu/drm/bridge/lt9611.c new file mode 100644 index 000000000000..e6e7ce43980d --- /dev/null +++ b/drivers/gpu/drm/bridge/lt9611.c @@ -0,0 +1,1113 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2018, The Linux Foundation. All rights reserved.
- Copyright (c) 2019-2020. Linaro Limited.
- */
+#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/interrupt.h> +#include <linux/of_graph.h> +#include <linux/regulator/consumer.h> +#include <drm/drm_probe_helper.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_print.h>
+#define EDID_SEG_SIZE 256
+#define LT9611_4LANES 0
+struct lt9611 {
- struct device *dev;
- struct drm_bridge bridge;
- struct drm_connector connector;
- struct regmap *regmap;
- struct device_node *dsi0_node;
- struct device_node *dsi1_node;
- struct mipi_dsi_device *dsi0;
- struct mipi_dsi_device *dsi1;
- bool ac_mode;
- struct gpio_desc *reset_gpio;
- struct gpio_desc *enable_gpio;
- bool power_on;
- bool sleep;
- struct regulator_bulk_data supplies[2];
- struct i2c_client *client;
- enum drm_connector_status status;
- u8 edid_buf[EDID_SEG_SIZE];
- u32 vic;
+};
+#define LT9611_PAGE_CONTROL 0xff
+static const struct regmap_range_cfg lt9611_ranges[] = {
- {
.name = "register_range",.range_min = 0,.range_max = 0x85ff,.selector_reg = LT9611_PAGE_CONTROL,.selector_mask = 0xff,.selector_shift = 0,.window_start = 0,.window_len = 0x100,- },
+};
+static const struct regmap_config lt9611_regmap_config = {
- .reg_bits = 8,
- .val_bits = 8,
- .max_register = 0xffff,
- .ranges = lt9611_ranges,
- .num_ranges = ARRAY_SIZE(lt9611_ranges),
+};
+struct lt9611_mode {
- u16 hdisplay;
- u16 vdisplay;
- u8 fps;
- u8 lanes;
- u8 intfs;
+};
+static struct lt9611_mode lt9611_modes[] = {
- { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
- { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
- { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
- { 1920, 1080, 24, 3, 1 },
- { 720, 480, 60, 4, 1 },
- { 720, 576, 50, 2, 1 },
- { 640, 480, 60, 2, 1 },
+};
+static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge) +{
- return container_of(bridge, struct lt9611, bridge);
+}
+static struct lt9611 *connector_to_lt9611(struct drm_connector *connector) +{
- return container_of(connector, struct lt9611, connector);
+}
+static int lt9611_mipi_input_analog(struct lt9611 *lt9611) +{
- struct reg_sequence reg_cfg[] = {
{ 0x8106, 0x40 }, /*port A rx current*/{ 0x810a, 0xfe }, /*port A ldo voltage set*/{ 0x810b, 0xbf }, /*enable port A lprx*/{ 0x8111, 0x40 }, /*port B rx current*/{ 0x8115, 0xfe }, /*port B ldo voltage set*/{ 0x8116, 0xbf }, /*enable port B lprx*/{ 0x811c, 0x03 }, /*PortA clk lane no-LP mode*/{ 0x8120, 0x03 }, /*PortB clk lane with-LP mode*/- };
- regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg));
- return 0;
+}
+static int lt9611_mipi_input_digital(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
- regmap_write(lt9611->regmap, 0x8300, LT9611_4LANES);
- if (mode->hdisplay == 3840)
regmap_write(lt9611->regmap, 0x830a, 0x03);- else
regmap_write(lt9611->regmap, 0x830a, 0x00);- regmap_write(lt9611->regmap, 0x824f, 0x80);
- regmap_write(lt9611->regmap, 0x8250, 0x10);
- regmap_write(lt9611->regmap, 0x8302, 0x0a);
- regmap_write(lt9611->regmap, 0x8306, 0x0a);
- return 0;
+}
+static void lt9611_mipi_video_setup(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
- u32 h_total, h_act, hpw, hfp, hss;
- u32 v_total, v_act, vpw, vfp, vss;
- h_total = mode->htotal;
- v_total = mode->vtotal;
- h_act = mode->hdisplay;
- hpw = mode->hsync_end - mode->hsync_start;
- hfp = mode->hsync_start - mode->hdisplay;
- hss = (mode->hsync_end - mode->hsync_start) +
(mode->htotal - mode->hsync_end);- v_act = mode->vdisplay;
- vpw = mode->vsync_end - mode->vsync_start;
- vfp = mode->vsync_start - mode->vdisplay;
- vss = (mode->vsync_end - mode->vsync_start) +
(mode->vtotal - mode->vsync_end);- regmap_write(lt9611->regmap, 0x830d, (u8)(v_total / 256));
- regmap_write(lt9611->regmap, 0x830e, (u8)(v_total % 256));
- regmap_write(lt9611->regmap, 0x830f, (u8)(v_act / 256));
- regmap_write(lt9611->regmap, 0x8310, (u8)(v_act % 256));
- regmap_write(lt9611->regmap, 0x8311, (u8)(h_total / 256));
- regmap_write(lt9611->regmap, 0x8312, (u8)(h_total % 256));
- regmap_write(lt9611->regmap, 0x8313, (u8)(h_act / 256));
- regmap_write(lt9611->regmap, 0x8314, (u8)(h_act % 256));
- regmap_write(lt9611->regmap, 0x8315, (u8)(vpw % 256));
- regmap_write(lt9611->regmap, 0x8316, (u8)(hpw % 256));
- regmap_write(lt9611->regmap, 0x8317, (u8)(vfp % 256));
- regmap_write(lt9611->regmap, 0x8318, (u8)(vss % 256));
- regmap_write(lt9611->regmap, 0x8319, (u8)(hfp % 256));
- regmap_write(lt9611->regmap, 0x831a, (u8)(hss / 256));
- regmap_write(lt9611->regmap, 0x831b, (u8)(hss % 256));
+}
+static int lt9611_pcr_setup(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
- struct reg_sequence reg_cfg[] = {
{ 0x830b, 0x01 },{ 0x830c, 0x10 },{ 0x8348, 0x00 },{ 0x8349, 0x81 },/* stage 1 */{ 0x8321, 0x4a },{ 0x8324, 0x71 },{ 0x8325, 0x30 },{ 0x832a, 0x01 },/* stage 2 */{ 0x834a, 0x40 },{ 0x831d, 0x10 },/* MK limit */{ 0x832d, 0x38 },{ 0x8331, 0x08 },- };
- regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg));
- switch (mode->hdisplay) {
- case 640:
regmap_write(lt9611->regmap, 0x8326, 0x14);break;- case 1920:
regmap_write(lt9611->regmap, 0x8326, 0x37);break;- case 3840:
regmap_write(lt9611->regmap, 0x830b, 0x03);regmap_write(lt9611->regmap, 0x830c, 0xd0);regmap_write(lt9611->regmap, 0x8348, 0x03);regmap_write(lt9611->regmap, 0x8349, 0xe0);regmap_write(lt9611->regmap, 0x8324, 0x72);regmap_write(lt9611->regmap, 0x8325, 0x00);regmap_write(lt9611->regmap, 0x832a, 0x01);regmap_write(lt9611->regmap, 0x834a, 0x10);regmap_write(lt9611->regmap, 0x831d, 0x10);regmap_write(lt9611->regmap, 0x8326, 0x37);break;- }
- /* pcr rst */
- regmap_write(lt9611->regmap, 0x8011, 0x5a);
- regmap_write(lt9611->regmap, 0x8011, 0xfa);
- return 0;
+}
+static int lt9611_pll_setup(struct lt9611 *lt9611,
const struct drm_display_mode *mode)+{
- unsigned int pclk = mode->clock;
- struct reg_sequence reg_cfg[] = {
/* txpll init */{ 0x8123, 0x40 },{ 0x8124, 0x64 },{ 0x8125, 0x80 },{ 0x8126, 0x55 },{ 0x812c, 0x37 },{ 0x812f, 0x01 },{ 0x8126, 0x55 },{ 0x8127, 0x66 },{ 0x8128, 0x88 },- };
- regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg));
- if (pclk > 150000)
regmap_write(lt9611->regmap, 0x812d, 0x88);- else if (pclk > 70000)
regmap_write(lt9611->regmap, 0x812d, 0x99);- else
regmap_write(lt9611->regmap, 0x812d, 0xaa);- regmap_write(lt9611->regmap, 0x82e3, pclk >> 17); /* pclk[19:16] */
- regmap_write(lt9611->regmap, 0x82e4, pclk >> 9); /* pclk[15:8] */
- regmap_write(lt9611->regmap, 0x82e5, pclk >> 1); /* pclk[7:0] */
- regmap_write(lt9611->regmap, 0x82de, 0x20);
- regmap_write(lt9611->regmap, 0x82de, 0xe0);
- regmap_write(lt9611->regmap, 0x8016, 0xf1);
- regmap_write(lt9611->regmap, 0x8016, 0xf3);
- return 0;
+}
+static int lt9611_video_check(struct lt9611 *lt9611) +{
- u32 v_total, v_act, h_act_a, h_act_b, h_total_sysclk;
- unsigned int temp;
- int ret;
- /* top module video check */
- /* v_act */
- ret = regmap_read(lt9611->regmap, 0x8282, &temp);
- if (ret)
goto end;- v_act = temp << 8;
- ret = regmap_read(lt9611->regmap, 0x8283, &temp);
- if (ret)
goto end;- v_act = v_act + temp;
- /* v_total */
- ret = regmap_read(lt9611->regmap, 0x826c, &temp);
- if (ret)
goto end;- v_total = temp << 8;
- ret = regmap_read(lt9611->regmap, 0x826d, &temp);
- if (ret)
goto end;- v_total = v_total + temp;
- /* h_total_sysclk */
- ret = regmap_read(lt9611->regmap, 0x8286, &temp);
- if (ret)
goto end;- h_total_sysclk = temp << 8;
- ret = regmap_read(lt9611->regmap, 0x8287, &temp);
- if (ret)
goto end;- h_total_sysclk = h_total_sysclk + temp;
- /* h_act_a */
- ret = regmap_read(lt9611->regmap, 0x8382, &temp);
- if (ret)
goto end;- h_act_a = temp << 8;
- ret = regmap_read(lt9611->regmap, 0x8383, &temp);
- if (ret)
goto end;- h_act_a = (h_act_a + temp) / 3;
- /* h_act_b */
- ret = regmap_read(lt9611->regmap, 0x8386, &temp);
- if (ret)
goto end;- h_act_b = temp << 8;
- ret = regmap_read(lt9611->regmap, 0x8387, &temp);
- if (ret)
goto end;- h_act_b = (h_act_b + temp) / 3;
- dev_info(lt9611->dev,
"video check: h_act_a=%d, h_act_b=%d, v_act=%d, v_total=%d, h_total_sysclk=%d\n",h_act_a, h_act_b, v_act, v_total, h_total_sysclk);- return 0;
+end:
- dev_err(lt9611->dev, "read video check error\n");
- return ret;
+}
+static void lt9611_hdmi_tx_digital(struct lt9611 *lt9611) +{
- regmap_write(lt9611->regmap, 0x8443, 0x46 - lt9611->vic);
- regmap_write(lt9611->regmap, 0x8447, lt9611->vic);
- regmap_write(lt9611->regmap, 0x843d, 0x0a); /* UD1 infoframe */
- regmap_write(lt9611->regmap, 0x82d6, 0x8c);
- regmap_write(lt9611->regmap, 0x82d7, 0x04);
+}
+static void lt9611_hdmi_tx_phy(struct lt9611 *lt9611) +{
- struct reg_sequence reg_cfg[] = {
{ 0x8130, 0x6a },{ 0x8131, 0x44 }, /* HDMI DC mode */{ 0x8132, 0x4a },{ 0x8133, 0x0b },{ 0x8134, 0x00 },{ 0x8135, 0x00 },{ 0x8136, 0x00 },{ 0x8137, 0x44 },{ 0x813f, 0x0f },{ 0x8140, 0xa0 },{ 0x8141, 0xa0 },{ 0x8142, 0xa0 },{ 0x8143, 0xa0 },{ 0x8144, 0x0a },- };
- /* HDMI AC mode */
- if (lt9611->ac_mode)
reg_cfg[2].def = 0x73;- regmap_multi_reg_write(lt9611->regmap, reg_cfg, ARRAY_SIZE(reg_cfg));
+}
+static irqreturn_t lt9611_irq_thread_handler(int irq, void *dev_id) +{
- struct lt9611 *lt9611 = dev_id;
- unsigned int irq_flag0 = 0;
- unsigned int irq_flag3 = 0;
- regmap_read(lt9611->regmap, 0x820f, &irq_flag3);
- regmap_read(lt9611->regmap, 0x820c, &irq_flag0);
- pr_debug("%s() irq_flag0: %#x irq_flag3: %#x\n",
__func__, irq_flag0, irq_flag3);/* hpd changed low */- if (irq_flag3 & 0x80) {
dev_info(lt9611->dev, "hdmi cable disconnected\n");regmap_write(lt9611->regmap, 0x8207, 0xbf);regmap_write(lt9611->regmap, 0x8207, 0x3f);- }
/* hpd changed high */- if (irq_flag3 & 0x40) {
dev_info(lt9611->dev, "hdmi cable connected\n");regmap_write(lt9611->regmap, 0x8207, 0x7f);regmap_write(lt9611->regmap, 0x8207, 0x3f);- }
- if (irq_flag3 & 0xc0)
drm_kms_helper_hotplug_event(lt9611->bridge.dev);- /* video input changed */
- if (irq_flag0 & 0x01) {
dev_info(lt9611->dev, "video input changed\n");regmap_write(lt9611->regmap, 0x829e, 0xff);regmap_write(lt9611->regmap, 0x829e, 0xf7);regmap_write(lt9611->regmap, 0x8204, 0xff);regmap_write(lt9611->regmap, 0x8204, 0xfe);- }
- return IRQ_HANDLED;
+}
+static void lt9611_enable_hpd_interrupts(struct lt9611 *lt9611) +{
- unsigned int val;
- dev_dbg(lt9611->dev, "enabling hpd interrupts\n");
- regmap_read(lt9611->regmap, 0x8203, &val);
- val &= ~0xc0;
- regmap_write(lt9611->regmap, 0x8203, val);
- regmap_write(lt9611->regmap, 0x8207, 0xff); //clear
- regmap_write(lt9611->regmap, 0x8207, 0x3f);
+}
+static void lt9611_sleep_setup(struct lt9611 *lt9611) +{
- struct reg_sequence sleep_setup[] = {
{ 0x8024, 0x76 },{ 0x8023, 0x01 },{ 0x8157, 0x03 }, //set addr pin as output{ 0x8149, 0x0b },{ 0x8151, 0x30 }, //disable IRQ{ 0x8102, 0x48 }, //MIPI Rx power down{ 0x8123, 0x80 },{ 0x8130, 0x00 },{ 0x8100, 0x01 }, //bandgap power down{ 0x8101, 0x00 }, //system clk power down- };
- dev_dbg(lt9611->dev, "sleep\n");
- regmap_multi_reg_write(lt9611->regmap,
sleep_setup, ARRAY_SIZE(sleep_setup));- lt9611->sleep = true;
+}
+static int lt9611_power_on(struct lt9611 *lt9611) +{
- int ret;
- const struct reg_sequence seq[] = {
/* LT9611_System_Init */{ 0x8101, 0x18 }, /* sel xtal clock *//* timer for frequency meter */{ 0x821b, 0x69 }, /*timer 2*/{ 0x821c, 0x78 },{ 0x82cb, 0x69 }, /*timer 1 */{ 0x82cc, 0x78 },/* irq init */{ 0x8251, 0x01 },{ 0x8258, 0x0a }, /* hpd irq */{ 0x8259, 0x80 }, /* hpd debounce width */{ 0x829e, 0xf7 }, /* video check irq *//* power consumption for work */{ 0x8004, 0xf0 },{ 0x8006, 0xf0 },{ 0x800a, 0x80 },{ 0x800b, 0x40 },{ 0x800d, 0xef },{ 0x8011, 0xfa },- };
- if (lt9611->power_on)
return 0;- dev_dbg(lt9611->dev, "power on\n");
- ret = regmap_multi_reg_write(lt9611->regmap, seq, ARRAY_SIZE(seq));
- if (!ret)
lt9611->power_on = true;- return ret;
+}
+static int lt9611_power_off(struct lt9611 *lt9611) +{
- int ret;
- dev_dbg(lt9611->dev, "power off\n");
- ret = regmap_write(lt9611->regmap, 0x8130, 0x6a);
- if (!ret)
lt9611->power_on = false;- return ret;
+}
+static void lt9611_reset(struct lt9611 *lt9611) +{
- gpiod_set_value_cansleep(lt9611->reset_gpio, 1);
- msleep(20);
- gpiod_set_value_cansleep(lt9611->reset_gpio, 0);
- msleep(20);
- gpiod_set_value_cansleep(lt9611->reset_gpio, 1);
- msleep(100);
+}
+static void lt9611_assert_5v(struct lt9611 *lt9611) +{
- if (!lt9611->enable_gpio)
return;- gpiod_set_value_cansleep(lt9611->enable_gpio, 1);
- msleep(20);
+}
+static int lt9611_regulator_init(struct lt9611 *lt9611) +{
- int ret;
- lt9611->supplies[0].supply = "vdd";
- lt9611->supplies[1].supply = "vcc";
- ret = devm_regulator_bulk_get(lt9611->dev, 2, lt9611->supplies);
- if (ret < 0)
return ret;- return regulator_set_load(lt9611->supplies[0].consumer, 300000);
+}
+static int lt9611_regulator_enable(struct lt9611 *lt9611) +{
- int ret;
- ret = regulator_enable(lt9611->supplies[0].consumer);
- if (ret < 0)
return ret;- usleep_range(1000, 10000);
- ret = regulator_enable(lt9611->supplies[1].consumer);
- if (ret < 0) {
regulator_disable(lt9611->supplies[0].consumer);return ret;- }
- return 0;
+}
+static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode) +{
- int i;
- for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
if (lt9611_modes[i].hdisplay == mode->hdisplay &<9611_modes[i].vdisplay == mode->vdisplay &<9611_modes[i].fps == drm_mode_vrefresh(mode)) {return <9611_modes[i];}- }
- return NULL;
+}
+/* connector funcs */ +static enum drm_connector_status +lt9611_connector_detect(struct drm_connector *connector, bool force) +{
- struct lt9611 *lt9611 = connector_to_lt9611(connector);
- unsigned int reg_val = 0;
- int connected = 0;
- regmap_read(lt9611->regmap, 0x825e, ®_val);
- connected = (reg_val & BIT(2));
- dev_dbg(lt9611->dev, "connected = %x\n", connected);
- lt9611->status = connected ? connector_status_connected :
connector_status_disconnected;- return lt9611->status;
+}
+static int lt9611_read_edid(struct lt9611 *lt9611) +{
- unsigned int temp;
- int ret = 0;
- int i, j;
- memset(lt9611->edid_buf, 0, EDID_SEG_SIZE);
- regmap_write(lt9611->regmap, 0x8503, 0xc9);
- /* 0xA0 is EDID device address */
- regmap_write(lt9611->regmap, 0x8504, 0xa0);
- /* 0x00 is EDID offset address */
- regmap_write(lt9611->regmap, 0x8505, 0x00);
- /* length for read */
- regmap_write(lt9611->regmap, 0x8506, 0x20);
- regmap_write(lt9611->regmap, 0x8514, 0x7f);
- for (i = 0 ; i < 8 ; i++) {
/* offset address */regmap_write(lt9611->regmap, 0x8505, i * 32);regmap_write(lt9611->regmap, 0x8507, 0x36);regmap_write(lt9611->regmap, 0x8507, 0x31);regmap_write(lt9611->regmap, 0x8507, 0x37);usleep_range(5000, 10000);regmap_read(lt9611->regmap, 0x8540, &temp);if (temp & 0x02) { /*KEY_DDC_ACCS_DONE=1*/for (j = 0; j < 32; j++) {regmap_read(lt9611->regmap, 0x8583, &temp);lt9611->edid_buf[i * 32 + j] = temp;}} else if (temp & 0x50) { /* DDC No Ack or Abitration lost */dev_err(lt9611->dev, "read edid failed: no ack\n");ret = -EIO;goto end;} else {dev_err(lt9611->dev,"read edid failed: access not done\n");ret = -EIO;goto end;}- }
- dev_dbg(lt9611->dev, "read edid succeeded, checksum = 0x%x\n",
lt9611->edid_buf[255]);+end:
- regmap_write(lt9611->regmap, 0x8507, 0x1f);
- return ret;
+}
+/* TODO: add support for more extension blocks */ +static int +lt9611_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len) +{
- struct lt9611 *lt9611 = data;
- int ret;
- dev_dbg(lt9611->dev, "get edid block: block=%d, len=%d\n",
block, (int)len);- if (len > 128)
return -EINVAL;- /* support up to 1 extension block */
- if (block > 1)
return -EINVAL;- if (block == 0) {
/* always read 2 edid blocks once */ret = lt9611_read_edid(lt9611);if (ret) {dev_err(lt9611->dev, "edid read failed\n");return ret;}- }
- if (block % 2 == 0)
memcpy(buf, lt9611->edid_buf, len);- else
memcpy(buf, lt9611->edid_buf + 128, len);- return 0;
+}
+static int lt9611_connector_get_modes(struct drm_connector *connector) +{
- struct lt9611 *lt9611 = connector_to_lt9611(connector);
- unsigned int count;
- struct edid *edid;
- dev_dbg(lt9611->dev, "get modes\n");
- lt9611_power_on(lt9611);
- edid = drm_do_get_edid(connector, lt9611_get_edid_block, lt9611);
- drm_connector_update_edid_property(connector, edid);
- count = drm_add_edid_modes(connector, edid);
- kfree(edid);
- return count;
+}
+static enum drm_mode_status +lt9611_connector_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)+{
- struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
- return lt9611_mode ? MODE_OK : MODE_BAD;
+}
+/* bridge funcs */ +static void lt9611_bridge_enable(struct drm_bridge *bridge) +{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- dev_dbg(lt9611->dev, "bridge enable\n");
- if (lt9611_power_on(lt9611)) {
dev_err(lt9611->dev, "power on failed\n");return;- }
- dev_dbg(lt9611->dev, "video on\n");
- lt9611_mipi_input_analog(lt9611);
- lt9611_hdmi_tx_digital(lt9611);
- lt9611_hdmi_tx_phy(lt9611);
- msleep(500);
- lt9611_video_check(lt9611);
- /* Enable HDMI output */
- regmap_write(lt9611->regmap, 0x8130, 0xea);
+}
+static void lt9611_bridge_disable(struct drm_bridge *bridge) +{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- int ret;
- dev_dbg(lt9611->dev, "bridge disable\n");
- /* Disable HDMI output */
- ret = regmap_write(lt9611->regmap, 0x8130, 0x6a);
- if (ret) {
dev_err(lt9611->dev, "video on failed\n");return;- }
- if (lt9611_power_off(lt9611)) {
dev_err(lt9611->dev, "power on failed\n");return;- }
+}
+static struct +drm_connector_helper_funcs lt9611_bridge_connector_helper_funcs = {
- .get_modes = lt9611_connector_get_modes,
- .mode_valid = lt9611_connector_mode_valid,
+};
+static const struct drm_connector_funcs lt9611_bridge_connector_funcs = {
- .fill_modes = drm_helper_probe_single_connector_modes,
- .detect = lt9611_connector_detect,
- .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 struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
struct device_node *dsi_node)+{
- const struct mipi_dsi_device_info info = { "lt9611", 0, NULL };
- struct mipi_dsi_device *dsi;
- struct mipi_dsi_host *host;
- int ret;
- host = of_find_mipi_dsi_host_by_node(dsi_node);
- if (!host) {
dev_err(lt9611->dev, "failed to find dsi host\n");return ERR_PTR(-EPROBE_DEFER);- }
- dsi = mipi_dsi_device_register_full(host, &info);
- if (IS_ERR(dsi)) {
dev_err(lt9611->dev, "failed to create dsi device\n");return dsi;- }
- dsi->lanes = 4;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
MIPI_DSI_MODE_VIDEO_HSE;- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
dev_err(lt9611->dev, "failed to attach dsi to host\n");mipi_dsi_device_unregister(dsi);return ERR_PTR(ret);- }
- return dsi;
+}
+static int lt9611_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)+{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- int ret;
- dev_dbg(lt9611->dev, "bridge attach\n");
Connector creation in bridge drivers is deprecated. Please at least add support for the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, to make connector creation optional. Ideally the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case should not be implemented at all. This will require the display controller driver to use DRM_BRIDGE_ATTACH_NO_CONNECTOR. Which display controller(s) do you use this driver with ?
- ret = drm_connector_init(bridge->dev, <9611->connector,
<9611_bridge_connector_funcs,DRM_MODE_CONNECTOR_HDMIA);- if (ret) {
DRM_ERROR("Failed to initialize connector with drm\n");return ret;- }
- drm_connector_helper_add(<9611->connector,
<9611_bridge_connector_helper_funcs);- drm_connector_attach_encoder(<9611->connector, bridge->encoder);
- if (!bridge->encoder) {
DRM_ERROR("Parent encoder object not found");return -ENODEV;- }
- /* Attach primary DSI */
- lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node);
- if (IS_ERR(lt9611->dsi0))
return PTR_ERR(lt9611->dsi0);- /* Attach secondary DSI, if specified */
- if (lt9611->dsi1_node) {
lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node);if (IS_ERR(lt9611->dsi1)) {ret = PTR_ERR(lt9611->dsi1);goto err_unregister_dsi0;}- }
- return 0;
+err_unregister_dsi0:
- mipi_dsi_device_unregister(lt9611->dsi0);
- return ret;
+}
+static void lt9611_bridge_detach(struct drm_bridge *bridge) +{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- if (lt9611->dsi1) {
mipi_dsi_detach(lt9611->dsi1);mipi_dsi_device_unregister(lt9611->dsi1);- }
- mipi_dsi_detach(lt9611->dsi0);
- mipi_dsi_device_unregister(lt9611->dsi0);
+}
+static enum drm_mode_status +lt9611_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)+{
- struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
return MODE_PANEL;- else
return MODE_OK;+}
+static void lt9611_bridge_pre_enable(struct drm_bridge *bridge) +{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- dev_dbg(lt9611->dev, "bridge pre_enable\n");
- if (!lt9611->sleep)
return;- lt9611_reset(lt9611);
- regmap_write(lt9611->regmap, 0x80ee, 0x01);
- lt9611->sleep = false;
+}
+static void lt9611_bridge_post_disable(struct drm_bridge *bridge) +{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- dev_dbg(lt9611->dev, "bridge post_disable\n");
- lt9611_sleep_setup(lt9611);
+}
+static void lt9611_bridge_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode,const struct drm_display_mode *adj_mode)+{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- struct hdmi_avi_infoframe avi_frame;
- int ret;
- dev_dbg(lt9611->dev, "bridge mode_set: hdisplay=%d, vdisplay=%d, vrefresh=%d, clock=%d\n",
adj_mode->hdisplay, adj_mode->vdisplay,adj_mode->vrefresh, adj_mode->clock);- lt9611_bridge_pre_enable(bridge);
- lt9611_mipi_input_digital(lt9611, mode);
- lt9611_pll_setup(lt9611, mode);
- lt9611_mipi_video_setup(lt9611, mode);
- lt9611_pcr_setup(lt9611, mode);
- ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame,
<9611->connector,mode);- if (!ret)
lt9611->vic = avi_frame.video_code;+}
+static const struct drm_bridge_funcs lt9611_bridge_funcs = {
- .attach = lt9611_bridge_attach,
- .detach = lt9611_bridge_detach,
- .mode_valid = lt9611_bridge_mode_valid,
- .enable = lt9611_bridge_enable,
- .disable = lt9611_bridge_disable,
- .post_disable = lt9611_bridge_post_disable,
- .mode_set = lt9611_bridge_mode_set,
+};
+static int lt9611_parse_dt(struct device *dev,
struct lt9611 *lt9611)+{
- lt9611->dsi0_node = of_graph_get_remote_node(dev->of_node, 1, -1);
- if (!lt9611->dsi0_node) {
DRM_DEV_ERROR(dev, "failed to get remote node for primary dsi\n");return -ENODEV;- }
- lt9611->dsi1_node = of_graph_get_remote_node(dev->of_node, 2, -1);
- lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode");
- dev_dbg(lt9611->dev, "ac_mode=%d\n", lt9611->ac_mode);
- return 0;
+}
+static int lt9611_gpio_init(struct lt9611 *lt9611) +{
- struct device *dev = lt9611->dev;
- lt9611->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(lt9611->reset_gpio)) {
dev_err(dev, "failed to acquire reset gpio\n");return PTR_ERR(lt9611->reset_gpio);- }
- lt9611->enable_gpio = devm_gpiod_get_optional(dev, "enable",
GPIOD_OUT_LOW);- if (IS_ERR(lt9611->enable_gpio)) {
dev_err(dev, "failed to acquire enable gpio\n");return PTR_ERR(lt9611->enable_gpio);- }
- return 0;
+}
+static int lt9611_read_device_rev(struct lt9611 *lt9611) +{
- unsigned int rev;
- int ret;
- regmap_write(lt9611->regmap, 0x80ee, 0x01);
- ret = regmap_read(lt9611->regmap, 0x8002, &rev);
- if (ret)
dev_err(lt9611->dev, "failed to read revision: %d\n", ret);- dev_info(lt9611->dev, "LT9611 revision: 0x%x\n", rev);
- return ret;
+}
+static int lt9611_probe(struct i2c_client *client,
const struct i2c_device_id *id)+{
- struct lt9611 *lt9611;
- struct device *dev = &client->dev;
- int ret;
- if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(dev, "device doesn't support I2C\n");return -ENODEV;- }
- lt9611 = devm_kzalloc(dev, sizeof(*lt9611), GFP_KERNEL);
- if (!lt9611)
return -ENOMEM;- lt9611->dev = &client->dev;
- lt9611->client = client;
- lt9611->sleep = false;
- lt9611->regmap = devm_regmap_init_i2c(client, <9611_regmap_config);
- if (IS_ERR(lt9611->regmap)) {
DRM_ERROR("regmap i2c init failed\n");return PTR_ERR(lt9611->regmap);- }
- ret = lt9611_parse_dt(&client->dev, lt9611);
- if (ret) {
dev_err(dev, "failed to parse device tree\n");return ret;- }
- ret = lt9611_gpio_init(lt9611);
- if (ret < 0)
return ret;- ret = lt9611_regulator_init(lt9611);
- if (ret < 0)
return ret;- lt9611_assert_5v(lt9611);
- ret = lt9611_regulator_enable(lt9611);
- if (ret)
return ret;- lt9611_reset(lt9611);
- ret = lt9611_read_device_rev(lt9611);
- if (ret) {
dev_err(dev, "failed to read chip rev\n");goto err_disable_regulators;- }
- ret = devm_request_threaded_irq(dev, client->irq, NULL,
lt9611_irq_thread_handler,IRQF_ONESHOT, "lt9611", lt9611);- if (ret) {
dev_err(dev, "failed to request irq\n");goto err_disable_regulators;- }
- i2c_set_clientdata(client, lt9611);
- lt9611->bridge.funcs = <9611_bridge_funcs;
- lt9611->bridge.of_node = client->dev.of_node;
- drm_bridge_add(<9611->bridge);
- lt9611_enable_hpd_interrupts(lt9611);
- return 0;
+err_disable_regulators:
- regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);
- of_node_put(lt9611->dsi0_node);
- of_node_put(lt9611->dsi1_node);
- return ret;
+}
+static int lt9611_remove(struct i2c_client *client) +{
- struct lt9611 *lt9611 = i2c_get_clientdata(client);
- disable_irq(client->irq);
- drm_bridge_remove(<9611->bridge);
- regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);
- of_node_put(lt9611->dsi0_node);
- of_node_put(lt9611->dsi1_node);
- return 0;
+}
+static struct i2c_device_id lt9611_id[] = {
- { "lontium,lt9611", 0},
- {}
+};
+static const struct of_device_id lt9611_match_table[] = {
- {.compatible = "lontium,lt9611"},
- {}
+}; +MODULE_DEVICE_TABLE(of, lt9611_match_table);
+static struct i2c_driver lt9611_driver = {
- .driver = {
.name = "lt9611",.of_match_table = lt9611_match_table,- },
- .probe = lt9611_probe,
- .remove = lt9611_remove,
- .id_table = lt9611_id,
+}; +module_i2c_driver(lt9611_driver);
+MODULE_LICENSE("GPL v2");
Hi Laurent,
On 28-05-20, 04:52, Laurent Pinchart wrote:
+static int lt9611_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)+{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- int ret;
- dev_dbg(lt9611->dev, "bridge attach\n");
Connector creation in bridge drivers is deprecated. Please at least add
Okay what is the right way for connector creation? I can add support for that.
support for the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, to make connector creation optional. Ideally the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case
will add that
should not be implemented at all. This will require the display controller driver to use DRM_BRIDGE_ATTACH_NO_CONNECTOR. Which display controller(s) do you use this driver with ?
I am using with msm display driver, this was tested on dragon-board db845c board.
Thanks
Hi Vinod,
On Thu, Jun 04, 2020 at 12:55:48PM +0530, Vinod Koul wrote:
On 28-05-20, 04:52, Laurent Pinchart wrote:
+static int lt9611_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)+{
- struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
- int ret;
- dev_dbg(lt9611->dev, "bridge attach\n");
Connector creation in bridge drivers is deprecated. Please at least add
Okay what is the right way for connector creation? I can add support for that.
Historically bridge drivers have created connectors. With support for bridge chaining, this approach was considered not to scale. For instance, I have a board where the SoC has an internal LVDS encoder, and the board itself has an LVDS-to-DPI decoder followed by a DPI-to-HDMI encoder. All three components are supported by bridge drivers, and only the last one should create a connector. Furthermore, different operations of the connector may be implemented by different bridges, for instance with one bridge connected to the DDC lines to read EDID, and another bridge connected to the HPD line to detect hotplug.
To support these systems, we have deprecated connector creation in bridges, in favour of implementing new bridge callback functions for connector-related operations (see .get_modes(), .get_edid() and .detect() in struct drm_bridge_funcs). With this new model, each bridge implements the operations it supports, and the display controller driver binds the bridges together to create a connector that delegates the connector operations to the appropriate bridge. A helper function, drm_bridge_connector_init(), can be used to automate that.
To transition to this model, we require all new bridge to at least optionally support disabling connector creation (as requested by the DRM_BRIDGE_ATTACH_NO_CONNECTOR), and implement the drm_bridge_funcs functions related to connector operations. Existing bridges are also converted to the new model. Once all bridges used by a display controller support the new model, the display controller is then converted to use DRM_BRIDGE_ATTACH_NO_CONNECTOR and drm_bridge_connector_init() (or implement the latter manually if the helper doesn't support all the display controller's needs). Once all display controllers using a bridge have been converted to the new model, support for creating a connector (the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case) is removed from the bridge driver. Finally, once everybody will use the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, we will simply drop it.
support for the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, to make connector creation optional. Ideally the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case
will add that
should not be implemented at all. This will require the display controller driver to use DRM_BRIDGE_ATTACH_NO_CONNECTOR. Which display controller(s) do you use this driver with ?
I am using with msm display driver, this was tested on dragon-board db845c board.
dri-devel@lists.freedesktop.org
-
Emil Velikov -
kbuild test robot -
Laurent Pinchart -
Rob Herring -
Vinod Koul