* [RESEND v3 0/2] drm: bridge: adv7511: Add support For ADV7535 @ 2020-01-07 13:34 ` Bogdan Togorean 0 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, mark.rutland, a.hajda, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, gregkh, tglx, sam, alexander.deucher, matt.redfearn, robdclark, wsa+renesas, linux-kernel, Bogdan Togorean This patch-set add support for ADV7535 part in ADV7511 driver. ADV7535 and ADV7533 are pin to pin compatible parts but ADV7535 support TMDS clock upto 148.5Mhz and resolutions up to 1080p@60Hz. --- Changes in v3: - remove CONFIG_DRM_I2C_ADV7533 from Kconfig. Now driver support all chip versions - create macros for v1p2 config registers - remove dummy functions from header Bogdan Togorean (2): dt-bindings: drm: bridge: adv7511: Add ADV7535 support drm: bridge: adv7511: Add support for ADV7535 .../bindings/display/bridge/adi,adv7511.txt | 23 +++++----- drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++---------------- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++----- 5 files changed, 44 insertions(+), 74 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RESEND v3 0/2] drm: bridge: adv7511: Add support For ADV7535 @ 2020-01-07 13:34 ` Bogdan Togorean 0 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: mark.rutland, robdclark, jernej.skrabec, narmstrong, airlied, gregkh, jonas, wsa+renesas, linux-kernel, robh+dt, Laurent.pinchart, alexander.deucher, tglx, sam, matt.redfearn, Bogdan Togorean This patch-set add support for ADV7535 part in ADV7511 driver. ADV7535 and ADV7533 are pin to pin compatible parts but ADV7535 support TMDS clock upto 148.5Mhz and resolutions up to 1080p@60Hz. --- Changes in v3: - remove CONFIG_DRM_I2C_ADV7533 from Kconfig. Now driver support all chip versions - create macros for v1p2 config registers - remove dummy functions from header Bogdan Togorean (2): dt-bindings: drm: bridge: adv7511: Add ADV7535 support drm: bridge: adv7511: Add support for ADV7535 .../bindings/display/bridge/adi,adv7511.txt | 23 +++++----- drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++---------------- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++----- 5 files changed, 44 insertions(+), 74 deletions(-) -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RESEND v3 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support 2020-01-07 13:34 ` Bogdan Togorean @ 2020-01-07 13:34 ` Bogdan Togorean -1 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, mark.rutland, a.hajda, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, gregkh, tglx, sam, alexander.deucher, matt.redfearn, robdclark, wsa+renesas, linux-kernel, Bogdan Togorean, Laurent Pinchart ADV7535 is a part compatible with ADV7533 but it supports 1080p@60hz and v1p2 supply is fixed to 1.8V Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../bindings/display/bridge/adi,adv7511.txt | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index 2c887536258c..e8ddec5d9d91 100644 --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt @@ -1,10 +1,10 @@ -Analog Device ADV7511(W)/13/33 HDMI Encoders +Analog Device ADV7511(W)/13/33/35 HDMI Encoders ----------------------------------------- -The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters -compatible with HDMI 1.4 and DVI 1.0. They support color space conversion, -S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while -the others support RGB interface. +The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video +transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space +conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input +pixels, while the others support RGB interface. Required properties: @@ -13,6 +13,7 @@ Required properties: "adi,adv7511w" "adi,adv7513" "adi,adv7533" + "adi,adv7535" - reg: I2C slave addresses The ADV7511 internal registers are split into four pages exposed through @@ -52,14 +53,14 @@ The following input format properties are required except in "rgb 1x" and - bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is needed only for ADV7511. -The following properties are required for ADV7533: +The following properties are required for ADV7533 and ADV7535: - adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should be one of 1, 2, 3 or 4. - a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip. - v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip. - v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be - either 1.2V or 1.8V. + either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535. Optional properties: @@ -71,9 +72,9 @@ Optional properties: - adi,embedded-sync: The input uses synchronization signals embedded in the data stream (similar to BT.656). Defaults to separate H/V synchronization signals. -- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing - generator. The chip will rely on the sync signals in the DSI data lanes, - rather than generate its own timings for HDMI output. +- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the + internal timing generator. The chip will rely on the sync signals in the + DSI data lanes, rather than generate its own timings for HDMI output. - clocks: from common clock binding: reference to the CEC clock. - clock-names: from common clock binding: must be "cec". - reg-names : Names of maps with programmable addresses. @@ -85,7 +86,7 @@ Required nodes: The ADV7511 has two video ports. Their connections are modelled using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. -- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the +- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the remote endpoint phandle should be a reference to a valid mipi_dsi_host device node. - Video port 1 for the HDMI output -- 2.23.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND v3 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support @ 2020-01-07 13:34 ` Bogdan Togorean 0 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: mark.rutland, robdclark, jernej.skrabec, narmstrong, airlied, gregkh, jonas, wsa+renesas, linux-kernel, robh+dt, Laurent Pinchart, alexander.deucher, tglx, sam, matt.redfearn, Bogdan Togorean ADV7535 is a part compatible with ADV7533 but it supports 1080p@60hz and v1p2 supply is fixed to 1.8V Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../bindings/display/bridge/adi,adv7511.txt | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index 2c887536258c..e8ddec5d9d91 100644 --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt @@ -1,10 +1,10 @@ -Analog Device ADV7511(W)/13/33 HDMI Encoders +Analog Device ADV7511(W)/13/33/35 HDMI Encoders ----------------------------------------- -The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters -compatible with HDMI 1.4 and DVI 1.0. They support color space conversion, -S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while -the others support RGB interface. +The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video +transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space +conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input +pixels, while the others support RGB interface. Required properties: @@ -13,6 +13,7 @@ Required properties: "adi,adv7511w" "adi,adv7513" "adi,adv7533" + "adi,adv7535" - reg: I2C slave addresses The ADV7511 internal registers are split into four pages exposed through @@ -52,14 +53,14 @@ The following input format properties are required except in "rgb 1x" and - bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is needed only for ADV7511. -The following properties are required for ADV7533: +The following properties are required for ADV7533 and ADV7535: - adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should be one of 1, 2, 3 or 4. - a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip. - v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip. - v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be - either 1.2V or 1.8V. + either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535. Optional properties: @@ -71,9 +72,9 @@ Optional properties: - adi,embedded-sync: The input uses synchronization signals embedded in the data stream (similar to BT.656). Defaults to separate H/V synchronization signals. -- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing - generator. The chip will rely on the sync signals in the DSI data lanes, - rather than generate its own timings for HDMI output. +- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the + internal timing generator. The chip will rely on the sync signals in the + DSI data lanes, rather than generate its own timings for HDMI output. - clocks: from common clock binding: reference to the CEC clock. - clock-names: from common clock binding: must be "cec". - reg-names : Names of maps with programmable addresses. @@ -85,7 +86,7 @@ Required nodes: The ADV7511 has two video ports. Their connections are modelled using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. -- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the +- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the remote endpoint phandle should be a reference to a valid mipi_dsi_host device node. - Video port 1 for the HDMI output -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 2020-01-07 13:34 ` Bogdan Togorean @ 2020-01-07 13:34 ` Bogdan Togorean -1 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, mark.rutland, a.hajda, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, gregkh, tglx, sam, alexander.deucher, matt.redfearn, robdclark, wsa+renesas, linux-kernel, Bogdan Togorean ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V or 1.8V and is configurable in a register. Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> --- drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ 4 files changed, 32 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig index 8a56ff81f4fb..47d4eb9e845d 100644 --- a/drivers/gpu/drm/bridge/adv7511/Kconfig +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 depends on OF select DRM_KMS_HELPER select REGMAP_I2C + select DRM_MIPI_DSI help - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. config DRM_I2C_ADV7511_AUDIO bool "ADV7511 HDMI Audio driver" @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO Support the ADV7511 HDMI Audio interface. This is used in conjunction with the AV7511 HDMI driver. -config DRM_I2C_ADV7533 - bool "ADV7533 encoder" - depends on DRM_I2C_ADV7511 - select DRM_MIPI_DSI - default y - help - Support for the Analog Devices ADV7533 DSI to HDMI encoder. - config DRM_I2C_ADV7511_CEC - bool "ADV7511/33 HDMI CEC driver" + bool "ADV7511/33/35 HDMI CEC driver" depends on DRM_I2C_ADV7511 select CEC_CORE default y diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile index b46ebeb35fd4..d8ceb534b51f 100644 --- a/drivers/gpu/drm/bridge/adv7511/Makefile +++ b/drivers/gpu/drm/bridge/adv7511/Makefile @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -adv7511-y := adv7511_drv.o +adv7511-y := adv7511_drv.o adv7533.o adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 52b2adfdc877..ed9cfd944098 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -220,6 +220,10 @@ #define ADV7533_REG_CEC_OFFSET 0x70 +#define ADV7533_REG_SUPPLY_SELECT 0xe4 + +#define ADV7533_V1P2_ENABLE BIT(7) + enum adv7511_input_clock { ADV7511_INPUT_CLOCK_1X, ADV7511_INPUT_CLOCK_2X, @@ -320,6 +324,7 @@ struct adv7511_video_config { enum adv7511_type { ADV7511, ADV7533, + ADV7535, }; #define ADV7511_MAX_ADDRS 3 @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) } #endif -#ifdef CONFIG_DRM_I2C_ADV7533 void adv7533_dsi_power_on(struct adv7511 *adv); void adv7533_dsi_power_off(struct adv7511 *adv); void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); -#else -static inline void adv7533_dsi_power_on(struct adv7511 *adv) -{ -} - -static inline void adv7533_dsi_power_off(struct adv7511 *adv) -{ -} - -static inline void adv7533_mode_set(struct adv7511 *adv, - const struct drm_display_mode *mode) -{ -} - -static inline int adv7533_patch_registers(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline int adv7533_attach_dsi(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline void adv7533_detach_dsi(struct adv7511 *adv) -{ -} - -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) -{ - return -ENODEV; -} -#endif #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 9e13e466e72c..35595472e771 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) */ regcache_sync(adv7511->regmap); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_dsi_power_on(adv7511); adv7511->powered = true; } @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) static void adv7511_power_off(struct adv7511 *adv7511) { __adv7511_power_off(adv7511); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_dsi_power_off(adv7511); adv7511->powered = false; } @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, regmap_update_bits(adv7511->regmap, 0x17, 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_mode_set(adv7511, adj_mode); drm_mode_copy(&adv7511->curr_mode, adj_mode); @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) &adv7511_connector_helper_funcs); drm_connector_attach_encoder(&adv->connector, bridge->encoder); - if (adv->type == ADV7533) + if (adv->type == ADV7533 || adv->type == ADV7535) ret = adv7533_attach_dsi(adv); if (adv->i2c_main->irq) @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { "dvdd-3v", }; +/* The order of entries is important. If changed update hardcoded indices */ static const char * const adv7533_supply_names[] = { "avdd", "dvdd", @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) struct i2c_client *i2c = to_i2c_client(dev); struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) reg -= ADV7533_REG_CEC_OFFSET; switch (reg) { @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) goto err; } - if (adv->type == ADV7533) { + if (adv->type == ADV7533 || adv->type == ADV7535) { ret = adv7533_patch_cec_registers(adv); if (ret) goto err; @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) struct adv7511_link_config link_config; struct adv7511 *adv7511; struct device *dev = &i2c->dev; + struct regulator *reg_v1p2; unsigned int val; - int ret; + int ret, reg_v1p2_uV; if (!dev->of_node) return -EINVAL; @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) goto uninit_regulators; + if (adv7511->type == ADV7533) { + reg_v1p2 = adv7511->supplies[5].consumer; + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); + + if (reg_v1p2_uV == 1200000) { + regmap_update_bits(adv7511->regmap, + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, + ADV7533_V1P2_ENABLE); + } + } + adv7511_packet_disable(adv7511, 0xffff); adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) { struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_detach_dsi(adv7511); i2c_unregister_device(adv7511->i2c_cec); if (adv7511->cec_clk) @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { { "adv7511", ADV7511 }, { "adv7511w", ADV7511 }, { "adv7513", ADV7511 }, -#ifdef CONFIG_DRM_I2C_ADV7533 { "adv7533", ADV7533 }, -#endif + { "adv7535", ADV7535 }, { } }; MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, -#ifdef CONFIG_DRM_I2C_ADV7533 { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, -#endif + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, { } }; MODULE_DEVICE_TABLE(of, adv7511_of_ids); -- 2.23.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 @ 2020-01-07 13:34 ` Bogdan Togorean 0 siblings, 0 replies; 12+ messages in thread From: Bogdan Togorean @ 2020-01-07 13:34 UTC (permalink / raw) To: dri-devel Cc: mark.rutland, robdclark, jernej.skrabec, narmstrong, airlied, gregkh, jonas, wsa+renesas, linux-kernel, robh+dt, Laurent.pinchart, alexander.deucher, tglx, sam, matt.redfearn, Bogdan Togorean ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V or 1.8V and is configurable in a register. Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> --- drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ 4 files changed, 32 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig index 8a56ff81f4fb..47d4eb9e845d 100644 --- a/drivers/gpu/drm/bridge/adv7511/Kconfig +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 depends on OF select DRM_KMS_HELPER select REGMAP_I2C + select DRM_MIPI_DSI help - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. config DRM_I2C_ADV7511_AUDIO bool "ADV7511 HDMI Audio driver" @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO Support the ADV7511 HDMI Audio interface. This is used in conjunction with the AV7511 HDMI driver. -config DRM_I2C_ADV7533 - bool "ADV7533 encoder" - depends on DRM_I2C_ADV7511 - select DRM_MIPI_DSI - default y - help - Support for the Analog Devices ADV7533 DSI to HDMI encoder. - config DRM_I2C_ADV7511_CEC - bool "ADV7511/33 HDMI CEC driver" + bool "ADV7511/33/35 HDMI CEC driver" depends on DRM_I2C_ADV7511 select CEC_CORE default y diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile index b46ebeb35fd4..d8ceb534b51f 100644 --- a/drivers/gpu/drm/bridge/adv7511/Makefile +++ b/drivers/gpu/drm/bridge/adv7511/Makefile @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -adv7511-y := adv7511_drv.o +adv7511-y := adv7511_drv.o adv7533.o adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 52b2adfdc877..ed9cfd944098 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -220,6 +220,10 @@ #define ADV7533_REG_CEC_OFFSET 0x70 +#define ADV7533_REG_SUPPLY_SELECT 0xe4 + +#define ADV7533_V1P2_ENABLE BIT(7) + enum adv7511_input_clock { ADV7511_INPUT_CLOCK_1X, ADV7511_INPUT_CLOCK_2X, @@ -320,6 +324,7 @@ struct adv7511_video_config { enum adv7511_type { ADV7511, ADV7533, + ADV7535, }; #define ADV7511_MAX_ADDRS 3 @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) } #endif -#ifdef CONFIG_DRM_I2C_ADV7533 void adv7533_dsi_power_on(struct adv7511 *adv); void adv7533_dsi_power_off(struct adv7511 *adv); void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); -#else -static inline void adv7533_dsi_power_on(struct adv7511 *adv) -{ -} - -static inline void adv7533_dsi_power_off(struct adv7511 *adv) -{ -} - -static inline void adv7533_mode_set(struct adv7511 *adv, - const struct drm_display_mode *mode) -{ -} - -static inline int adv7533_patch_registers(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline int adv7533_attach_dsi(struct adv7511 *adv) -{ - return -ENODEV; -} - -static inline void adv7533_detach_dsi(struct adv7511 *adv) -{ -} - -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) -{ - return -ENODEV; -} -#endif #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 9e13e466e72c..35595472e771 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) */ regcache_sync(adv7511->regmap); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_dsi_power_on(adv7511); adv7511->powered = true; } @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) static void adv7511_power_off(struct adv7511 *adv7511) { __adv7511_power_off(adv7511); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_dsi_power_off(adv7511); adv7511->powered = false; } @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, regmap_update_bits(adv7511->regmap, 0x17, 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_mode_set(adv7511, adj_mode); drm_mode_copy(&adv7511->curr_mode, adj_mode); @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) &adv7511_connector_helper_funcs); drm_connector_attach_encoder(&adv->connector, bridge->encoder); - if (adv->type == ADV7533) + if (adv->type == ADV7533 || adv->type == ADV7535) ret = adv7533_attach_dsi(adv); if (adv->i2c_main->irq) @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { "dvdd-3v", }; +/* The order of entries is important. If changed update hardcoded indices */ static const char * const adv7533_supply_names[] = { "avdd", "dvdd", @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) struct i2c_client *i2c = to_i2c_client(dev); struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) reg -= ADV7533_REG_CEC_OFFSET; switch (reg) { @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) goto err; } - if (adv->type == ADV7533) { + if (adv->type == ADV7533 || adv->type == ADV7535) { ret = adv7533_patch_cec_registers(adv); if (ret) goto err; @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) struct adv7511_link_config link_config; struct adv7511 *adv7511; struct device *dev = &i2c->dev; + struct regulator *reg_v1p2; unsigned int val; - int ret; + int ret, reg_v1p2_uV; if (!dev->of_node) return -EINVAL; @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) goto uninit_regulators; + if (adv7511->type == ADV7533) { + reg_v1p2 = adv7511->supplies[5].consumer; + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); + + if (reg_v1p2_uV == 1200000) { + regmap_update_bits(adv7511->regmap, + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, + ADV7533_V1P2_ENABLE); + } + } + adv7511_packet_disable(adv7511, 0xffff); adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) { struct adv7511 *adv7511 = i2c_get_clientdata(i2c); - if (adv7511->type == ADV7533) + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) adv7533_detach_dsi(adv7511); i2c_unregister_device(adv7511->i2c_cec); if (adv7511->cec_clk) @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { { "adv7511", ADV7511 }, { "adv7511w", ADV7511 }, { "adv7513", ADV7511 }, -#ifdef CONFIG_DRM_I2C_ADV7533 { "adv7533", ADV7533 }, -#endif + { "adv7535", ADV7535 }, { } }; MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, -#ifdef CONFIG_DRM_I2C_ADV7533 { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, -#endif + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, { } }; MODULE_DEVICE_TABLE(of, adv7511_of_ids); -- 2.23.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 2020-01-07 13:34 ` Bogdan Togorean @ 2020-01-17 10:27 ` Andrzej Hajda -1 siblings, 0 replies; 12+ messages in thread From: Andrzej Hajda @ 2020-01-17 10:27 UTC (permalink / raw) To: Bogdan Togorean, dri-devel Cc: airlied, daniel, robh+dt, mark.rutland, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, gregkh, tglx, sam, alexander.deucher, matt.redfearn, robdclark, wsa+renesas, linux-kernel On 07.01.2020 14:34, Bogdan Togorean wrote: > ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows > 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V > or 1.8V and is configurable in a register. > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> > --- > drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- > drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- > drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ > 4 files changed, 32 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig > index 8a56ff81f4fb..47d4eb9e845d 100644 > --- a/drivers/gpu/drm/bridge/adv7511/Kconfig > +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig > @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 > depends on OF > select DRM_KMS_HELPER > select REGMAP_I2C > + select DRM_MIPI_DSI > help > - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. > + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. > > config DRM_I2C_ADV7511_AUDIO > bool "ADV7511 HDMI Audio driver" > @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO > Support the ADV7511 HDMI Audio interface. This is used in > conjunction with the AV7511 HDMI driver. > > -config DRM_I2C_ADV7533 > - bool "ADV7533 encoder" > - depends on DRM_I2C_ADV7511 > - select DRM_MIPI_DSI > - default y > - help > - Support for the Analog Devices ADV7533 DSI to HDMI encoder. > - > config DRM_I2C_ADV7511_CEC > - bool "ADV7511/33 HDMI CEC driver" > + bool "ADV7511/33/35 HDMI CEC driver" > depends on DRM_I2C_ADV7511 > select CEC_CORE > default y > diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile > index b46ebeb35fd4..d8ceb534b51f 100644 > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > @@ -1,6 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > -adv7511-y := adv7511_drv.o > +adv7511-y := adv7511_drv.o adv7533.o > adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o > adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o > -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h > index 52b2adfdc877..ed9cfd944098 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > @@ -220,6 +220,10 @@ > > #define ADV7533_REG_CEC_OFFSET 0x70 > > +#define ADV7533_REG_SUPPLY_SELECT 0xe4 > + > +#define ADV7533_V1P2_ENABLE BIT(7) > + > enum adv7511_input_clock { > ADV7511_INPUT_CLOCK_1X, > ADV7511_INPUT_CLOCK_2X, > @@ -320,6 +324,7 @@ struct adv7511_video_config { > enum adv7511_type { > ADV7511, > ADV7533, > + ADV7535, > }; > > #define ADV7511_MAX_ADDRS 3 > @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) > } > #endif > > -#ifdef CONFIG_DRM_I2C_ADV7533 > void adv7533_dsi_power_on(struct adv7511 *adv); > void adv7533_dsi_power_off(struct adv7511 *adv); > void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); > @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); > int adv7533_attach_dsi(struct adv7511 *adv); > void adv7533_detach_dsi(struct adv7511 *adv); > int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); > -#else > -static inline void adv7533_dsi_power_on(struct adv7511 *adv) > -{ > -} > - > -static inline void adv7533_dsi_power_off(struct adv7511 *adv) > -{ > -} > - > -static inline void adv7533_mode_set(struct adv7511 *adv, > - const struct drm_display_mode *mode) > -{ > -} > - > -static inline int adv7533_patch_registers(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline int adv7533_attach_dsi(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline void adv7533_detach_dsi(struct adv7511 *adv) > -{ > -} > - > -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) > -{ > - return -ENODEV; > -} > -#endif > > #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 9e13e466e72c..35595472e771 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) > */ > regcache_sync(adv7511->regmap); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_dsi_power_on(adv7511); > adv7511->powered = true; > } > @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) > static void adv7511_power_off(struct adv7511 *adv7511) > { > __adv7511_power_off(adv7511); > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_dsi_power_off(adv7511); > adv7511->powered = false; > } > @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, > regmap_update_bits(adv7511->regmap, 0x17, > 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_mode_set(adv7511, adj_mode); > > drm_mode_copy(&adv7511->curr_mode, adj_mode); > @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) > &adv7511_connector_helper_funcs); > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > - if (adv->type == ADV7533) > + if (adv->type == ADV7533 || adv->type == ADV7535) > ret = adv7533_attach_dsi(adv); > > if (adv->i2c_main->irq) > @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { > "dvdd-3v", > }; > > +/* The order of entries is important. If changed update hardcoded indices */ > static const char * const adv7533_supply_names[] = { > "avdd", > "dvdd", > @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) > struct i2c_client *i2c = to_i2c_client(dev); > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > reg -= ADV7533_REG_CEC_OFFSET; > > switch (reg) { > @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) > goto err; > } > > - if (adv->type == ADV7533) { > + if (adv->type == ADV7533 || adv->type == ADV7535) { > ret = adv7533_patch_cec_registers(adv); > if (ret) > goto err; > @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) > struct adv7511_link_config link_config; > struct adv7511 *adv7511; > struct device *dev = &i2c->dev; > + struct regulator *reg_v1p2; > unsigned int val; > - int ret; > + int ret, reg_v1p2_uV; > > if (!dev->of_node) > return -EINVAL; > @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) > if (ret) > goto uninit_regulators; > > + if (adv7511->type == ADV7533) { > + reg_v1p2 = adv7511->supplies[5].consumer; > + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); > + > + if (reg_v1p2_uV == 1200000) { > + regmap_update_bits(adv7511->regmap, > + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, > + ADV7533_V1P2_ENABLE); > + } > + } > + In patch adding support for adv7535 you modifies adv7533 path. It looks suspicious, maybe it would be better to extract it to separate patch. Anyway this change requires tests from adv7533 users. Beside this the patch looks OK. Regards Andrzej > adv7511_packet_disable(adv7511, 0xffff); > > adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", > @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) > { > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_detach_dsi(adv7511); > i2c_unregister_device(adv7511->i2c_cec); > if (adv7511->cec_clk) > @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { > { "adv7511", ADV7511 }, > { "adv7511w", ADV7511 }, > { "adv7513", ADV7511 }, > -#ifdef CONFIG_DRM_I2C_ADV7533 > { "adv7533", ADV7533 }, > -#endif > + { "adv7535", ADV7535 }, > { } > }; > MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); > @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { > { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, > { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, > { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, > -#ifdef CONFIG_DRM_I2C_ADV7533 > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > -#endif > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > { } > }; > MODULE_DEVICE_TABLE(of, adv7511_of_ids); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 @ 2020-01-17 10:27 ` Andrzej Hajda 0 siblings, 0 replies; 12+ messages in thread From: Andrzej Hajda @ 2020-01-17 10:27 UTC (permalink / raw) To: Bogdan Togorean, dri-devel Cc: mark.rutland, robdclark, jernej.skrabec, narmstrong, airlied, gregkh, jonas, linux-kernel, wsa+renesas, robh+dt, Laurent.pinchart, alexander.deucher, tglx, sam, matt.redfearn On 07.01.2020 14:34, Bogdan Togorean wrote: > ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows > 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V > or 1.8V and is configurable in a register. > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> > --- > drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- > drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- > drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ > 4 files changed, 32 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig > index 8a56ff81f4fb..47d4eb9e845d 100644 > --- a/drivers/gpu/drm/bridge/adv7511/Kconfig > +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig > @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 > depends on OF > select DRM_KMS_HELPER > select REGMAP_I2C > + select DRM_MIPI_DSI > help > - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. > + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. > > config DRM_I2C_ADV7511_AUDIO > bool "ADV7511 HDMI Audio driver" > @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO > Support the ADV7511 HDMI Audio interface. This is used in > conjunction with the AV7511 HDMI driver. > > -config DRM_I2C_ADV7533 > - bool "ADV7533 encoder" > - depends on DRM_I2C_ADV7511 > - select DRM_MIPI_DSI > - default y > - help > - Support for the Analog Devices ADV7533 DSI to HDMI encoder. > - > config DRM_I2C_ADV7511_CEC > - bool "ADV7511/33 HDMI CEC driver" > + bool "ADV7511/33/35 HDMI CEC driver" > depends on DRM_I2C_ADV7511 > select CEC_CORE > default y > diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile > index b46ebeb35fd4..d8ceb534b51f 100644 > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > @@ -1,6 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > -adv7511-y := adv7511_drv.o > +adv7511-y := adv7511_drv.o adv7533.o > adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o > adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o > -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h > index 52b2adfdc877..ed9cfd944098 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > @@ -220,6 +220,10 @@ > > #define ADV7533_REG_CEC_OFFSET 0x70 > > +#define ADV7533_REG_SUPPLY_SELECT 0xe4 > + > +#define ADV7533_V1P2_ENABLE BIT(7) > + > enum adv7511_input_clock { > ADV7511_INPUT_CLOCK_1X, > ADV7511_INPUT_CLOCK_2X, > @@ -320,6 +324,7 @@ struct adv7511_video_config { > enum adv7511_type { > ADV7511, > ADV7533, > + ADV7535, > }; > > #define ADV7511_MAX_ADDRS 3 > @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) > } > #endif > > -#ifdef CONFIG_DRM_I2C_ADV7533 > void adv7533_dsi_power_on(struct adv7511 *adv); > void adv7533_dsi_power_off(struct adv7511 *adv); > void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); > @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); > int adv7533_attach_dsi(struct adv7511 *adv); > void adv7533_detach_dsi(struct adv7511 *adv); > int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); > -#else > -static inline void adv7533_dsi_power_on(struct adv7511 *adv) > -{ > -} > - > -static inline void adv7533_dsi_power_off(struct adv7511 *adv) > -{ > -} > - > -static inline void adv7533_mode_set(struct adv7511 *adv, > - const struct drm_display_mode *mode) > -{ > -} > - > -static inline int adv7533_patch_registers(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline int adv7533_attach_dsi(struct adv7511 *adv) > -{ > - return -ENODEV; > -} > - > -static inline void adv7533_detach_dsi(struct adv7511 *adv) > -{ > -} > - > -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) > -{ > - return -ENODEV; > -} > -#endif > > #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 9e13e466e72c..35595472e771 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) > */ > regcache_sync(adv7511->regmap); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_dsi_power_on(adv7511); > adv7511->powered = true; > } > @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) > static void adv7511_power_off(struct adv7511 *adv7511) > { > __adv7511_power_off(adv7511); > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_dsi_power_off(adv7511); > adv7511->powered = false; > } > @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, > regmap_update_bits(adv7511->regmap, 0x17, > 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_mode_set(adv7511, adj_mode); > > drm_mode_copy(&adv7511->curr_mode, adj_mode); > @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) > &adv7511_connector_helper_funcs); > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > - if (adv->type == ADV7533) > + if (adv->type == ADV7533 || adv->type == ADV7535) > ret = adv7533_attach_dsi(adv); > > if (adv->i2c_main->irq) > @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { > "dvdd-3v", > }; > > +/* The order of entries is important. If changed update hardcoded indices */ > static const char * const adv7533_supply_names[] = { > "avdd", > "dvdd", > @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) > struct i2c_client *i2c = to_i2c_client(dev); > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > reg -= ADV7533_REG_CEC_OFFSET; > > switch (reg) { > @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) > goto err; > } > > - if (adv->type == ADV7533) { > + if (adv->type == ADV7533 || adv->type == ADV7535) { > ret = adv7533_patch_cec_registers(adv); > if (ret) > goto err; > @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) > struct adv7511_link_config link_config; > struct adv7511 *adv7511; > struct device *dev = &i2c->dev; > + struct regulator *reg_v1p2; > unsigned int val; > - int ret; > + int ret, reg_v1p2_uV; > > if (!dev->of_node) > return -EINVAL; > @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) > if (ret) > goto uninit_regulators; > > + if (adv7511->type == ADV7533) { > + reg_v1p2 = adv7511->supplies[5].consumer; > + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); > + > + if (reg_v1p2_uV == 1200000) { > + regmap_update_bits(adv7511->regmap, > + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, > + ADV7533_V1P2_ENABLE); > + } > + } > + In patch adding support for adv7535 you modifies adv7533 path. It looks suspicious, maybe it would be better to extract it to separate patch. Anyway this change requires tests from adv7533 users. Beside this the patch looks OK. Regards Andrzej > adv7511_packet_disable(adv7511, 0xffff); > > adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", > @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) > { > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > - if (adv7511->type == ADV7533) > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > adv7533_detach_dsi(adv7511); > i2c_unregister_device(adv7511->i2c_cec); > if (adv7511->cec_clk) > @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { > { "adv7511", ADV7511 }, > { "adv7511w", ADV7511 }, > { "adv7513", ADV7511 }, > -#ifdef CONFIG_DRM_I2C_ADV7533 > { "adv7533", ADV7533 }, > -#endif > + { "adv7535", ADV7535 }, > { } > }; > MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); > @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { > { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, > { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, > { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, > -#ifdef CONFIG_DRM_I2C_ADV7533 > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > -#endif > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > { } > }; > MODULE_DEVICE_TABLE(of, adv7511_of_ids); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 2020-01-17 10:27 ` Andrzej Hajda @ 2020-01-17 10:55 ` Andrzej Hajda -1 siblings, 0 replies; 12+ messages in thread From: Andrzej Hajda @ 2020-01-17 10:55 UTC (permalink / raw) To: Bogdan Togorean, dri-devel Cc: airlied, daniel, robh+dt, mark.rutland, narmstrong, Laurent.pinchart, jonas, jernej.skrabec, gregkh, tglx, sam, alexander.deucher, matt.redfearn, robdclark, wsa+renesas, linux-kernel On 17.01.2020 11:27, Andrzej Hajda wrote: > On 07.01.2020 14:34, Bogdan Togorean wrote: >> ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows >> 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V >> or 1.8V and is configurable in a register. >> >> Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> >> --- >> drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- >> drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- >> drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- >> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ >> 4 files changed, 32 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig >> index 8a56ff81f4fb..47d4eb9e845d 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig >> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig >> @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 >> depends on OF >> select DRM_KMS_HELPER >> select REGMAP_I2C >> + select DRM_MIPI_DSI >> help >> - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. >> + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. >> >> config DRM_I2C_ADV7511_AUDIO >> bool "ADV7511 HDMI Audio driver" >> @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO >> Support the ADV7511 HDMI Audio interface. This is used in >> conjunction with the AV7511 HDMI driver. >> >> -config DRM_I2C_ADV7533 >> - bool "ADV7533 encoder" >> - depends on DRM_I2C_ADV7511 >> - select DRM_MIPI_DSI >> - default y >> - help >> - Support for the Analog Devices ADV7533 DSI to HDMI encoder. >> - >> config DRM_I2C_ADV7511_CEC >> - bool "ADV7511/33 HDMI CEC driver" >> + bool "ADV7511/33/35 HDMI CEC driver" >> depends on DRM_I2C_ADV7511 >> select CEC_CORE >> default y >> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile >> index b46ebeb35fd4..d8ceb534b51f 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/Makefile >> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile >> @@ -1,6 +1,5 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> -adv7511-y := adv7511_drv.o >> +adv7511-y := adv7511_drv.o adv7533.o >> adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o >> adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o >> -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o >> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> index 52b2adfdc877..ed9cfd944098 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> @@ -220,6 +220,10 @@ >> >> #define ADV7533_REG_CEC_OFFSET 0x70 >> >> +#define ADV7533_REG_SUPPLY_SELECT 0xe4 >> + >> +#define ADV7533_V1P2_ENABLE BIT(7) >> + >> enum adv7511_input_clock { >> ADV7511_INPUT_CLOCK_1X, >> ADV7511_INPUT_CLOCK_2X, >> @@ -320,6 +324,7 @@ struct adv7511_video_config { >> enum adv7511_type { >> ADV7511, >> ADV7533, >> + ADV7535, >> }; >> >> #define ADV7511_MAX_ADDRS 3 >> @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) >> } >> #endif >> >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> void adv7533_dsi_power_on(struct adv7511 *adv); >> void adv7533_dsi_power_off(struct adv7511 *adv); >> void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); >> @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); >> int adv7533_attach_dsi(struct adv7511 *adv); >> void adv7533_detach_dsi(struct adv7511 *adv); >> int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); >> -#else >> -static inline void adv7533_dsi_power_on(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline void adv7533_dsi_power_off(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline void adv7533_mode_set(struct adv7511 *adv, >> - const struct drm_display_mode *mode) >> -{ >> -} >> - >> -static inline int adv7533_patch_registers(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline int adv7533_attach_dsi(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline void adv7533_detach_dsi(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> -#endif >> >> #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >> int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> index 9e13e466e72c..35595472e771 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) >> */ >> regcache_sync(adv7511->regmap); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_dsi_power_on(adv7511); >> adv7511->powered = true; >> } >> @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) >> static void adv7511_power_off(struct adv7511 *adv7511) >> { >> __adv7511_power_off(adv7511); >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_dsi_power_off(adv7511); >> adv7511->powered = false; >> } >> @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, >> regmap_update_bits(adv7511->regmap, 0x17, >> 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_mode_set(adv7511, adj_mode); >> >> drm_mode_copy(&adv7511->curr_mode, adj_mode); >> @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) >> &adv7511_connector_helper_funcs); >> drm_connector_attach_encoder(&adv->connector, bridge->encoder); >> >> - if (adv->type == ADV7533) >> + if (adv->type == ADV7533 || adv->type == ADV7535) >> ret = adv7533_attach_dsi(adv); >> >> if (adv->i2c_main->irq) >> @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { >> "dvdd-3v", >> }; >> >> +/* The order of entries is important. If changed update hardcoded indices */ >> static const char * const adv7533_supply_names[] = { >> "avdd", >> "dvdd", >> @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) >> struct i2c_client *i2c = to_i2c_client(dev); >> struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> reg -= ADV7533_REG_CEC_OFFSET; >> >> switch (reg) { >> @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) >> goto err; >> } >> >> - if (adv->type == ADV7533) { >> + if (adv->type == ADV7533 || adv->type == ADV7535) { >> ret = adv7533_patch_cec_registers(adv); >> if (ret) >> goto err; >> @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) >> struct adv7511_link_config link_config; >> struct adv7511 *adv7511; >> struct device *dev = &i2c->dev; >> + struct regulator *reg_v1p2; >> unsigned int val; >> - int ret; >> + int ret, reg_v1p2_uV; >> >> if (!dev->of_node) >> return -EINVAL; >> @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) >> if (ret) >> goto uninit_regulators; >> >> + if (adv7511->type == ADV7533) { >> + reg_v1p2 = adv7511->supplies[5].consumer; >> + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); >> + >> + if (reg_v1p2_uV == 1200000) { >> + regmap_update_bits(adv7511->regmap, >> + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, >> + ADV7533_V1P2_ENABLE); >> + } >> + } >> + > > In patch adding support for adv7535 you modifies adv7533 path. It looks > suspicious, maybe it would be better to extract it to separate patch. > Anyway this change requires tests from adv7533 users. One more thing, maybe it would be better to add some tolerance to 1.2V test, quick look at public datasheet suggest +/- 0.06V. Regards Andrzej > > Beside this the patch looks OK. > > > Regards > > Andrzej > > >> adv7511_packet_disable(adv7511, 0xffff); >> >> adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", >> @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) >> { >> struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_detach_dsi(adv7511); >> i2c_unregister_device(adv7511->i2c_cec); >> if (adv7511->cec_clk) >> @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { >> { "adv7511", ADV7511 }, >> { "adv7511w", ADV7511 }, >> { "adv7513", ADV7511 }, >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> { "adv7533", ADV7533 }, >> -#endif >> + { "adv7535", ADV7535 }, >> { } >> }; >> MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); >> @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { >> { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, >> { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, >> { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, >> -#endif >> + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, >> { } >> }; >> MODULE_DEVICE_TABLE(of, adv7511_of_ids); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 @ 2020-01-17 10:55 ` Andrzej Hajda 0 siblings, 0 replies; 12+ messages in thread From: Andrzej Hajda @ 2020-01-17 10:55 UTC (permalink / raw) To: Bogdan Togorean, dri-devel Cc: mark.rutland, robdclark, jernej.skrabec, narmstrong, airlied, gregkh, jonas, linux-kernel, wsa+renesas, robh+dt, Laurent.pinchart, alexander.deucher, tglx, sam, matt.redfearn On 17.01.2020 11:27, Andrzej Hajda wrote: > On 07.01.2020 14:34, Bogdan Togorean wrote: >> ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows >> 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can be 1.2V >> or 1.8V and is configurable in a register. >> >> Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> >> --- >> drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- >> drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- >> drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------------- >> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++------ >> 4 files changed, 32 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig >> index 8a56ff81f4fb..47d4eb9e845d 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig >> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig >> @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 >> depends on OF >> select DRM_KMS_HELPER >> select REGMAP_I2C >> + select DRM_MIPI_DSI >> help >> - Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders. >> + Support for the Analog Device ADV7511(W)/13/33/35 HDMI encoders. >> >> config DRM_I2C_ADV7511_AUDIO >> bool "ADV7511 HDMI Audio driver" >> @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO >> Support the ADV7511 HDMI Audio interface. This is used in >> conjunction with the AV7511 HDMI driver. >> >> -config DRM_I2C_ADV7533 >> - bool "ADV7533 encoder" >> - depends on DRM_I2C_ADV7511 >> - select DRM_MIPI_DSI >> - default y >> - help >> - Support for the Analog Devices ADV7533 DSI to HDMI encoder. >> - >> config DRM_I2C_ADV7511_CEC >> - bool "ADV7511/33 HDMI CEC driver" >> + bool "ADV7511/33/35 HDMI CEC driver" >> depends on DRM_I2C_ADV7511 >> select CEC_CORE >> default y >> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile >> index b46ebeb35fd4..d8ceb534b51f 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/Makefile >> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile >> @@ -1,6 +1,5 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> -adv7511-y := adv7511_drv.o >> +adv7511-y := adv7511_drv.o adv7533.o >> adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o >> adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o >> -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o >> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> index 52b2adfdc877..ed9cfd944098 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> @@ -220,6 +220,10 @@ >> >> #define ADV7533_REG_CEC_OFFSET 0x70 >> >> +#define ADV7533_REG_SUPPLY_SELECT 0xe4 >> + >> +#define ADV7533_V1P2_ENABLE BIT(7) >> + >> enum adv7511_input_clock { >> ADV7511_INPUT_CLOCK_1X, >> ADV7511_INPUT_CLOCK_2X, >> @@ -320,6 +324,7 @@ struct adv7511_video_config { >> enum adv7511_type { >> ADV7511, >> ADV7533, >> + ADV7535, >> }; >> >> #define ADV7511_MAX_ADDRS 3 >> @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) >> } >> #endif >> >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> void adv7533_dsi_power_on(struct adv7511 *adv); >> void adv7533_dsi_power_off(struct adv7511 *adv); >> void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); >> @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct adv7511 *adv); >> int adv7533_attach_dsi(struct adv7511 *adv); >> void adv7533_detach_dsi(struct adv7511 *adv); >> int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); >> -#else >> -static inline void adv7533_dsi_power_on(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline void adv7533_dsi_power_off(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline void adv7533_mode_set(struct adv7511 *adv, >> - const struct drm_display_mode *mode) >> -{ >> -} >> - >> -static inline int adv7533_patch_registers(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline int adv7533_patch_cec_registers(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline int adv7533_attach_dsi(struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> - >> -static inline void adv7533_detach_dsi(struct adv7511 *adv) >> -{ >> -} >> - >> -static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv) >> -{ >> - return -ENODEV; >> -} >> -#endif >> >> #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO >> int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511); >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> index 9e13e466e72c..35595472e771 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) >> */ >> regcache_sync(adv7511->regmap); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_dsi_power_on(adv7511); >> adv7511->powered = true; >> } >> @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511) >> static void adv7511_power_off(struct adv7511 *adv7511) >> { >> __adv7511_power_off(adv7511); >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_dsi_power_off(adv7511); >> adv7511->powered = false; >> } >> @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511, >> regmap_update_bits(adv7511->regmap, 0x17, >> 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_mode_set(adv7511, adj_mode); >> >> drm_mode_copy(&adv7511->curr_mode, adj_mode); >> @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) >> &adv7511_connector_helper_funcs); >> drm_connector_attach_encoder(&adv->connector, bridge->encoder); >> >> - if (adv->type == ADV7533) >> + if (adv->type == ADV7533 || adv->type == ADV7535) >> ret = adv7533_attach_dsi(adv); >> >> if (adv->i2c_main->irq) >> @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = { >> "dvdd-3v", >> }; >> >> +/* The order of entries is important. If changed update hardcoded indices */ >> static const char * const adv7533_supply_names[] = { >> "avdd", >> "dvdd", >> @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device *dev, unsigned int reg) >> struct i2c_client *i2c = to_i2c_client(dev); >> struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> reg -= ADV7533_REG_CEC_OFFSET; >> >> switch (reg) { >> @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) >> goto err; >> } >> >> - if (adv->type == ADV7533) { >> + if (adv->type == ADV7533 || adv->type == ADV7535) { >> ret = adv7533_patch_cec_registers(adv); >> if (ret) >> goto err; >> @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) >> struct adv7511_link_config link_config; >> struct adv7511 *adv7511; >> struct device *dev = &i2c->dev; >> + struct regulator *reg_v1p2; >> unsigned int val; >> - int ret; >> + int ret, reg_v1p2_uV; >> >> if (!dev->of_node) >> return -EINVAL; >> @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) >> if (ret) >> goto uninit_regulators; >> >> + if (adv7511->type == ADV7533) { >> + reg_v1p2 = adv7511->supplies[5].consumer; >> + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); >> + >> + if (reg_v1p2_uV == 1200000) { >> + regmap_update_bits(adv7511->regmap, >> + ADV7533_REG_SUPPLY_SELECT, ADV7533_V1P2_ENABLE, >> + ADV7533_V1P2_ENABLE); >> + } >> + } >> + > > In patch adding support for adv7535 you modifies adv7533 path. It looks > suspicious, maybe it would be better to extract it to separate patch. > Anyway this change requires tests from adv7533 users. One more thing, maybe it would be better to add some tolerance to 1.2V test, quick look at public datasheet suggest +/- 0.06V. Regards Andrzej > > Beside this the patch looks OK. > > > Regards > > Andrzej > > >> adv7511_packet_disable(adv7511, 0xffff); >> >> adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", >> @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client *i2c) >> { >> struct adv7511 *adv7511 = i2c_get_clientdata(i2c); >> >> - if (adv7511->type == ADV7533) >> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) >> adv7533_detach_dsi(adv7511); >> i2c_unregister_device(adv7511->i2c_cec); >> if (adv7511->cec_clk) >> @@ -1266,9 +1279,8 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { >> { "adv7511", ADV7511 }, >> { "adv7511w", ADV7511 }, >> { "adv7513", ADV7511 }, >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> { "adv7533", ADV7533 }, >> -#endif >> + { "adv7535", ADV7535 }, >> { } >> }; >> MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); >> @@ -1277,9 +1289,8 @@ static const struct of_device_id adv7511_of_ids[] = { >> { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, >> { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, >> { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, >> -#ifdef CONFIG_DRM_I2C_ADV7533 >> { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, >> -#endif >> + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, >> { } >> }; >> MODULE_DEVICE_TABLE(of, adv7511_of_ids); > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 2020-01-17 10:55 ` Andrzej Hajda @ 2020-01-17 12:51 ` Togorean, Bogdan -1 siblings, 0 replies; 12+ messages in thread From: Togorean, Bogdan @ 2020-01-17 12:51 UTC (permalink / raw) To: dri-devel, a.hajda Cc: narmstrong, Laurent.pinchart, robdclark, matt.redfearn, linux-kernel, daniel, tglx, wsa+renesas, alexander.deucher, mark.rutland, robh+dt, jonas, gregkh, airlied, sam, jernej.skrabec On Fri, 2020-01-17 at 11:55 +0100, Andrzej Hajda wrote: > [External] > > On 17.01.2020 11:27, Andrzej Hajda wrote: > > On 07.01.2020 14:34, Bogdan Togorean wrote: > > > ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows > > > 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can > > > be 1.2V > > > or 1.8V and is configurable in a register. > > > > > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> > > > --- > > > drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- > > > drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- > > > drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------- > > > ------ > > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++-- > > > ---- > > > 4 files changed, 32 insertions(+), 63 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig > > > b/drivers/gpu/drm/bridge/adv7511/Kconfig > > > index 8a56ff81f4fb..47d4eb9e845d 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/Kconfig > > > +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig > > > @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 > > > depends on OF > > > select DRM_KMS_HELPER > > > select REGMAP_I2C > > > + select DRM_MIPI_DSI > > > help > > > - Support for the Analog Device ADV7511(W) and ADV7513 HDMI > > > encoders. > > > + Support for the Analog Device ADV7511(W)/13/33/35 HDMI > > > encoders. > > > > > > config DRM_I2C_ADV7511_AUDIO > > > bool "ADV7511 HDMI Audio driver" > > > @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO > > > Support the ADV7511 HDMI Audio interface. This is used in > > > conjunction with the AV7511 HDMI driver. > > > > > > -config DRM_I2C_ADV7533 > > > - bool "ADV7533 encoder" > > > - depends on DRM_I2C_ADV7511 > > > - select DRM_MIPI_DSI > > > - default y > > > - help > > > - Support for the Analog Devices ADV7533 DSI to HDMI encoder. > > > - > > > config DRM_I2C_ADV7511_CEC > > > - bool "ADV7511/33 HDMI CEC driver" > > > + bool "ADV7511/33/35 HDMI CEC driver" > > > depends on DRM_I2C_ADV7511 > > > select CEC_CORE > > > default y > > > diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile > > > b/drivers/gpu/drm/bridge/adv7511/Makefile > > > index b46ebeb35fd4..d8ceb534b51f 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > > > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > > > @@ -1,6 +1,5 @@ > > > # SPDX-License-Identifier: GPL-2.0-only > > > -adv7511-y := adv7511_drv.o > > > +adv7511-y := adv7511_drv.o adv7533.o > > > adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o > > > adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o > > > -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o > > > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > b/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > index 52b2adfdc877..ed9cfd944098 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > @@ -220,6 +220,10 @@ > > > > > > #define ADV7533_REG_CEC_OFFSET 0x70 > > > > > > +#define ADV7533_REG_SUPPLY_SELECT 0xe4 > > > + > > > +#define ADV7533_V1P2_ENABLE BIT(7) > > > + > > > enum adv7511_input_clock { > > > ADV7511_INPUT_CLOCK_1X, > > > ADV7511_INPUT_CLOCK_2X, > > > @@ -320,6 +324,7 @@ struct adv7511_video_config { > > > enum adv7511_type { > > > ADV7511, > > > ADV7533, > > > + ADV7535, > > > }; > > > > > > #define ADV7511_MAX_ADDRS 3 > > > @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct > > > device *dev, struct adv7511 *adv7511) > > > } > > > #endif > > > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > void adv7533_dsi_power_on(struct adv7511 *adv); > > > void adv7533_dsi_power_off(struct adv7511 *adv); > > > void adv7533_mode_set(struct adv7511 *adv, const struct > > > drm_display_mode *mode); > > > @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct > > > adv7511 *adv); > > > int adv7533_attach_dsi(struct adv7511 *adv); > > > void adv7533_detach_dsi(struct adv7511 *adv); > > > int adv7533_parse_dt(struct device_node *np, struct adv7511 > > > *adv); > > > -#else > > > -static inline void adv7533_dsi_power_on(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline void adv7533_dsi_power_off(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline void adv7533_mode_set(struct adv7511 *adv, > > > - const struct drm_display_mode > > > *mode) > > > -{ > > > -} > > > - > > > -static inline int adv7533_patch_registers(struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline int adv7533_patch_cec_registers(struct adv7511 > > > *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline int adv7533_attach_dsi(struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline void adv7533_detach_dsi(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline int adv7533_parse_dt(struct device_node *np, > > > struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > -#endif > > > > > > #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > > > int adv7511_audio_init(struct device *dev, struct adv7511 > > > *adv7511); > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > index 9e13e466e72c..35595472e771 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 > > > *adv7511) > > > */ > > > regcache_sync(adv7511->regmap); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_dsi_power_on(adv7511); > > > adv7511->powered = true; > > > } > > > @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct > > > adv7511 *adv7511) > > > static void adv7511_power_off(struct adv7511 *adv7511) > > > { > > > __adv7511_power_off(adv7511); > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_dsi_power_off(adv7511); > > > adv7511->powered = false; > > > } > > > @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 > > > *adv7511, > > > regmap_update_bits(adv7511->regmap, 0x17, > > > 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_mode_set(adv7511, adj_mode); > > > > > > drm_mode_copy(&adv7511->curr_mode, adj_mode); > > > @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct > > > drm_bridge *bridge) > > > &adv7511_connector_helper_funcs); > > > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > > > > > - if (adv->type == ADV7533) > > > + if (adv->type == ADV7533 || adv->type == ADV7535) > > > ret = adv7533_attach_dsi(adv); > > > > > > if (adv->i2c_main->irq) > > > @@ -903,6 +903,7 @@ static const char * const > > > adv7511_supply_names[] = { > > > "dvdd-3v", > > > }; > > > > > > +/* The order of entries is important. If changed update > > > hardcoded indices */ > > > static const char * const adv7533_supply_names[] = { > > > "avdd", > > > "dvdd", > > > @@ -952,7 +953,7 @@ static bool > > > adv7511_cec_register_volatile(struct device *dev, unsigned int > > > reg) > > > struct i2c_client *i2c = to_i2c_client(dev); > > > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > reg -= ADV7533_REG_CEC_OFFSET; > > > > > > switch (reg) { > > > @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct > > > adv7511 *adv) > > > goto err; > > > } > > > > > > - if (adv->type == ADV7533) { > > > + if (adv->type == ADV7533 || adv->type == ADV7535) { > > > ret = adv7533_patch_cec_registers(adv); > > > if (ret) > > > goto err; > > > @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client > > > *i2c, const struct i2c_device_id *id) > > > struct adv7511_link_config link_config; > > > struct adv7511 *adv7511; > > > struct device *dev = &i2c->dev; > > > + struct regulator *reg_v1p2; > > > unsigned int val; > > > - int ret; > > > + int ret, reg_v1p2_uV; > > > > > > if (!dev->of_node) > > > return -EINVAL; > > > @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client > > > *i2c, const struct i2c_device_id *id) > > > if (ret) > > > goto uninit_regulators; > > > > > > + if (adv7511->type == ADV7533) { > > > + reg_v1p2 = adv7511->supplies[5].consumer; > > > + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); > > > + > > > + if (reg_v1p2_uV == 1200000) { > > > + regmap_update_bits(adv7511->regmap, > > > + ADV7533_REG_SUPPLY_SELECT, > > > ADV7533_V1P2_ENABLE, > > > + ADV7533_V1P2_ENABLE); > > > + } > > > + } > > > + > > > > In patch adding support for adv7535 you modifies adv7533 path. It > > looks > > suspicious, maybe it would be better to extract it to separate > > patch. > > Anyway this change requires tests from adv7533 users. > > One more thing, maybe it would be better to add some tolerance to > 1.2V > test, quick look at public datasheet suggest +/- 0.06V. > > > Regards > > Andrzej Thank you Andrzej for review. I'll create separate patch for ADV7533 voltage selection and send V4. Regards, Bogdan > > > > Beside this the patch looks OK. > > > > > > Regards > > > > Andrzej > > > > > > > adv7511_packet_disable(adv7511, 0xffff); > > > > > > adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", > > > @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client > > > *i2c) > > > { > > > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_detach_dsi(adv7511); > > > i2c_unregister_device(adv7511->i2c_cec); > > > if (adv7511->cec_clk) > > > @@ -1266,9 +1279,8 @@ static const struct i2c_device_id > > > adv7511_i2c_ids[] = { > > > { "adv7511", ADV7511 }, > > > { "adv7511w", ADV7511 }, > > > { "adv7513", ADV7511 }, > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > { "adv7533", ADV7533 }, > > > -#endif > > > + { "adv7535", ADV7535 }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); > > > @@ -1277,9 +1289,8 @@ static const struct of_device_id > > > adv7511_of_ids[] = { > > > { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, > > > { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, > > > { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > > > -#endif > > > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(of, adv7511_of_ids); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 @ 2020-01-17 12:51 ` Togorean, Bogdan 0 siblings, 0 replies; 12+ messages in thread From: Togorean, Bogdan @ 2020-01-17 12:51 UTC (permalink / raw) To: dri-devel, a.hajda Cc: robdclark, mark.rutland, jernej.skrabec, narmstrong, airlied, gregkh, jonas, linux-kernel, wsa+renesas, robh+dt, Laurent.pinchart, alexander.deucher, tglx, sam, matt.redfearn On Fri, 2020-01-17 at 11:55 +0100, Andrzej Hajda wrote: > [External] > > On 17.01.2020 11:27, Andrzej Hajda wrote: > > On 07.01.2020 14:34, Bogdan Togorean wrote: > > > ADV7535 is a DSI to HDMI bridge chip like ADV7533 but it allows > > > 1080p@60Hz. v1p2 is fixed to 1.8V on ADV7535 but on ADV7533 can > > > be 1.2V > > > or 1.8V and is configurable in a register. > > > > > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com> > > > --- > > > drivers/gpu/drm/bridge/adv7511/Kconfig | 13 ++---- > > > drivers/gpu/drm/bridge/adv7511/Makefile | 3 +- > > > drivers/gpu/drm/bridge/adv7511/adv7511.h | 44 +++----------- > > > ------ > > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 35 ++++++++++-- > > > ---- > > > 4 files changed, 32 insertions(+), 63 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig > > > b/drivers/gpu/drm/bridge/adv7511/Kconfig > > > index 8a56ff81f4fb..47d4eb9e845d 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/Kconfig > > > +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig > > > @@ -4,8 +4,9 @@ config DRM_I2C_ADV7511 > > > depends on OF > > > select DRM_KMS_HELPER > > > select REGMAP_I2C > > > + select DRM_MIPI_DSI > > > help > > > - Support for the Analog Device ADV7511(W) and ADV7513 HDMI > > > encoders. > > > + Support for the Analog Device ADV7511(W)/13/33/35 HDMI > > > encoders. > > > > > > config DRM_I2C_ADV7511_AUDIO > > > bool "ADV7511 HDMI Audio driver" > > > @@ -15,16 +16,8 @@ config DRM_I2C_ADV7511_AUDIO > > > Support the ADV7511 HDMI Audio interface. This is used in > > > conjunction with the AV7511 HDMI driver. > > > > > > -config DRM_I2C_ADV7533 > > > - bool "ADV7533 encoder" > > > - depends on DRM_I2C_ADV7511 > > > - select DRM_MIPI_DSI > > > - default y > > > - help > > > - Support for the Analog Devices ADV7533 DSI to HDMI encoder. > > > - > > > config DRM_I2C_ADV7511_CEC > > > - bool "ADV7511/33 HDMI CEC driver" > > > + bool "ADV7511/33/35 HDMI CEC driver" > > > depends on DRM_I2C_ADV7511 > > > select CEC_CORE > > > default y > > > diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile > > > b/drivers/gpu/drm/bridge/adv7511/Makefile > > > index b46ebeb35fd4..d8ceb534b51f 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > > > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > > > @@ -1,6 +1,5 @@ > > > # SPDX-License-Identifier: GPL-2.0-only > > > -adv7511-y := adv7511_drv.o > > > +adv7511-y := adv7511_drv.o adv7533.o > > > adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o > > > adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o > > > -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o > > > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > b/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > index 52b2adfdc877..ed9cfd944098 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > > > @@ -220,6 +220,10 @@ > > > > > > #define ADV7533_REG_CEC_OFFSET 0x70 > > > > > > +#define ADV7533_REG_SUPPLY_SELECT 0xe4 > > > + > > > +#define ADV7533_V1P2_ENABLE BIT(7) > > > + > > > enum adv7511_input_clock { > > > ADV7511_INPUT_CLOCK_1X, > > > ADV7511_INPUT_CLOCK_2X, > > > @@ -320,6 +324,7 @@ struct adv7511_video_config { > > > enum adv7511_type { > > > ADV7511, > > > ADV7533, > > > + ADV7535, > > > }; > > > > > > #define ADV7511_MAX_ADDRS 3 > > > @@ -393,7 +398,6 @@ static inline int adv7511_cec_init(struct > > > device *dev, struct adv7511 *adv7511) > > > } > > > #endif > > > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > void adv7533_dsi_power_on(struct adv7511 *adv); > > > void adv7533_dsi_power_off(struct adv7511 *adv); > > > void adv7533_mode_set(struct adv7511 *adv, const struct > > > drm_display_mode *mode); > > > @@ -402,44 +406,6 @@ int adv7533_patch_cec_registers(struct > > > adv7511 *adv); > > > int adv7533_attach_dsi(struct adv7511 *adv); > > > void adv7533_detach_dsi(struct adv7511 *adv); > > > int adv7533_parse_dt(struct device_node *np, struct adv7511 > > > *adv); > > > -#else > > > -static inline void adv7533_dsi_power_on(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline void adv7533_dsi_power_off(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline void adv7533_mode_set(struct adv7511 *adv, > > > - const struct drm_display_mode > > > *mode) > > > -{ > > > -} > > > - > > > -static inline int adv7533_patch_registers(struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline int adv7533_patch_cec_registers(struct adv7511 > > > *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline int adv7533_attach_dsi(struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > - > > > -static inline void adv7533_detach_dsi(struct adv7511 *adv) > > > -{ > > > -} > > > - > > > -static inline int adv7533_parse_dt(struct device_node *np, > > > struct adv7511 *adv) > > > -{ > > > - return -ENODEV; > > > -} > > > -#endif > > > > > > #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > > > int adv7511_audio_init(struct device *dev, struct adv7511 > > > *adv7511); > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > index 9e13e466e72c..35595472e771 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 > > > *adv7511) > > > */ > > > regcache_sync(adv7511->regmap); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_dsi_power_on(adv7511); > > > adv7511->powered = true; > > > } > > > @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct > > > adv7511 *adv7511) > > > static void adv7511_power_off(struct adv7511 *adv7511) > > > { > > > __adv7511_power_off(adv7511); > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_dsi_power_off(adv7511); > > > adv7511->powered = false; > > > } > > > @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 > > > *adv7511, > > > regmap_update_bits(adv7511->regmap, 0x17, > > > 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_mode_set(adv7511, adj_mode); > > > > > > drm_mode_copy(&adv7511->curr_mode, adj_mode); > > > @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct > > > drm_bridge *bridge) > > > &adv7511_connector_helper_funcs); > > > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > > > > > - if (adv->type == ADV7533) > > > + if (adv->type == ADV7533 || adv->type == ADV7535) > > > ret = adv7533_attach_dsi(adv); > > > > > > if (adv->i2c_main->irq) > > > @@ -903,6 +903,7 @@ static const char * const > > > adv7511_supply_names[] = { > > > "dvdd-3v", > > > }; > > > > > > +/* The order of entries is important. If changed update > > > hardcoded indices */ > > > static const char * const adv7533_supply_names[] = { > > > "avdd", > > > "dvdd", > > > @@ -952,7 +953,7 @@ static bool > > > adv7511_cec_register_volatile(struct device *dev, unsigned int > > > reg) > > > struct i2c_client *i2c = to_i2c_client(dev); > > > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > reg -= ADV7533_REG_CEC_OFFSET; > > > > > > switch (reg) { > > > @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct > > > adv7511 *adv) > > > goto err; > > > } > > > > > > - if (adv->type == ADV7533) { > > > + if (adv->type == ADV7533 || adv->type == ADV7535) { > > > ret = adv7533_patch_cec_registers(adv); > > > if (ret) > > > goto err; > > > @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client > > > *i2c, const struct i2c_device_id *id) > > > struct adv7511_link_config link_config; > > > struct adv7511 *adv7511; > > > struct device *dev = &i2c->dev; > > > + struct regulator *reg_v1p2; > > > unsigned int val; > > > - int ret; > > > + int ret, reg_v1p2_uV; > > > > > > if (!dev->of_node) > > > return -EINVAL; > > > @@ -1163,6 +1165,17 @@ static int adv7511_probe(struct i2c_client > > > *i2c, const struct i2c_device_id *id) > > > if (ret) > > > goto uninit_regulators; > > > > > > + if (adv7511->type == ADV7533) { > > > + reg_v1p2 = adv7511->supplies[5].consumer; > > > + reg_v1p2_uV = regulator_get_voltage(reg_v1p2); > > > + > > > + if (reg_v1p2_uV == 1200000) { > > > + regmap_update_bits(adv7511->regmap, > > > + ADV7533_REG_SUPPLY_SELECT, > > > ADV7533_V1P2_ENABLE, > > > + ADV7533_V1P2_ENABLE); > > > + } > > > + } > > > + > > > > In patch adding support for adv7535 you modifies adv7533 path. It > > looks > > suspicious, maybe it would be better to extract it to separate > > patch. > > Anyway this change requires tests from adv7533 users. > > One more thing, maybe it would be better to add some tolerance to > 1.2V > test, quick look at public datasheet suggest +/- 0.06V. > > > Regards > > Andrzej Thank you Andrzej for review. I'll create separate patch for ADV7533 voltage selection and send V4. Regards, Bogdan > > > > Beside this the patch looks OK. > > > > > > Regards > > > > Andrzej > > > > > > > adv7511_packet_disable(adv7511, 0xffff); > > > > > > adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid", > > > @@ -1242,7 +1255,7 @@ static int adv7511_remove(struct i2c_client > > > *i2c) > > > { > > > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > > > > > - if (adv7511->type == ADV7533) > > > + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) > > > adv7533_detach_dsi(adv7511); > > > i2c_unregister_device(adv7511->i2c_cec); > > > if (adv7511->cec_clk) > > > @@ -1266,9 +1279,8 @@ static const struct i2c_device_id > > > adv7511_i2c_ids[] = { > > > { "adv7511", ADV7511 }, > > > { "adv7511w", ADV7511 }, > > > { "adv7513", ADV7511 }, > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > { "adv7533", ADV7533 }, > > > -#endif > > > + { "adv7535", ADV7535 }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(i2c, adv7511_i2c_ids); > > > @@ -1277,9 +1289,8 @@ static const struct of_device_id > > > adv7511_of_ids[] = { > > > { .compatible = "adi,adv7511", .data = (void *)ADV7511 }, > > > { .compatible = "adi,adv7511w", .data = (void *)ADV7511 }, > > > { .compatible = "adi,adv7513", .data = (void *)ADV7511 }, > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > > > -#endif > > > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(of, adv7511_of_ids); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-18 9:55 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-07 13:34 [RESEND v3 0/2] drm: bridge: adv7511: Add support For ADV7535 Bogdan Togorean 2020-01-07 13:34 ` Bogdan Togorean 2020-01-07 13:34 ` [RESEND v3 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support Bogdan Togorean 2020-01-07 13:34 ` Bogdan Togorean 2020-01-07 13:34 ` [RESEND v3 2/2] drm: bridge: adv7511: Add support for ADV7535 Bogdan Togorean 2020-01-07 13:34 ` Bogdan Togorean 2020-01-17 10:27 ` Andrzej Hajda 2020-01-17 10:27 ` Andrzej Hajda 2020-01-17 10:55 ` Andrzej Hajda 2020-01-17 10:55 ` Andrzej Hajda 2020-01-17 12:51 ` Togorean, Bogdan 2020-01-17 12:51 ` Togorean, Bogdan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.