* [PATCH v2 0/2] drm: bridge: adv7511: Add support For ADV7535 @ 2019-08-09 14:16 Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 Bogdan Togorean 0 siblings, 2 replies; 11+ messages in thread From: Bogdan Togorean @ 2019-08-09 14:16 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, a.hajda, Laurent.pinchart, sam, gregkh, allison, tglx, matt.redfearn, 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 v2: - rename CONFIG_DRM_I2C_ADV7533 to CONFIG_DRM_I2C_ADV753X and update decription - removed "v1p2" index search and hardcoded it 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 | 8 ++--- drivers/gpu/drm/bridge/adv7511/Makefile | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++------ 5 files changed, 44 insertions(+), 27 deletions(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support 2019-08-09 14:16 [PATCH v2 0/2] drm: bridge: adv7511: Add support For ADV7535 Bogdan Togorean @ 2019-08-09 14:16 ` Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 Bogdan Togorean 1 sibling, 0 replies; 11+ messages in thread From: Bogdan Togorean @ 2019-08-09 14:16 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, a.hajda, Laurent.pinchart, sam, gregkh, allison, tglx, matt.redfearn, linux-kernel, 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> --- .../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.22.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-09 14:16 [PATCH v2 0/2] drm: bridge: adv7511: Add support For ADV7535 Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support Bogdan Togorean @ 2019-08-09 14:16 ` Bogdan Togorean 2019-08-09 15:25 ` Sam Ravnborg 1 sibling, 1 reply; 11+ messages in thread From: Bogdan Togorean @ 2019-08-09 14:16 UTC (permalink / raw) To: dri-devel Cc: airlied, daniel, robh+dt, a.hajda, Laurent.pinchart, sam, gregkh, allison, tglx, matt.redfearn, 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 | 8 ++--- drivers/gpu/drm/bridge/adv7511/Makefile | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 ++++++++++++++------ 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig index 8a56ff81f4fb..fa43acd46ab7 100644 --- a/drivers/gpu/drm/bridge/adv7511/Kconfig +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig @@ -15,16 +15,16 @@ 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" +config DRM_I2C_ADV753x + bool "ADV753x encoder" depends on DRM_I2C_ADV7511 select DRM_MIPI_DSI default y help - Support for the Analog Devices ADV7533 DSI to HDMI encoder. + Support for the Analog Devices ADV7533/5 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..319efddb268e 100644 --- a/drivers/gpu/drm/bridge/adv7511/Makefile +++ b/drivers/gpu/drm/bridge/adv7511/Makefile @@ -2,5 +2,5 @@ adv7511-y := adv7511_drv.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 +adv7511-$(CONFIG_DRM_I2C_ADV753x) += 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..38288c3c3c53 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -91,6 +91,7 @@ #define ADV7511_REG_ARC_CTRL 0xdf #define ADV7511_REG_CEC_I2C_ADDR 0xe1 #define ADV7511_REG_CEC_CTRL 0xe2 +#define ADV7511_REG_SUPPLY_SELECT 0xe4 #define ADV7511_REG_CHIP_ID_HIGH 0xf5 #define ADV7511_REG_CHIP_ID_LOW 0xf6 @@ -320,6 +321,7 @@ struct adv7511_video_config { enum adv7511_type { ADV7511, ADV7533, + ADV7535, }; #define ADV7511_MAX_ADDRS 3 @@ -393,7 +395,7 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) } #endif -#ifdef CONFIG_DRM_I2C_ADV7533 +#ifdef CONFIG_DRM_I2C_ADV753x 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); diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index f6d2681f6927..b1501344df3e 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,16 @@ 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, + ADV7511_REG_SUPPLY_SELECT, 0x80, 0x80); + } + } + adv7511_packet_disable(adv7511, 0xffff); adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid", @@ -1242,7 +1254,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,8 +1278,9 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { { "adv7511", ADV7511 }, { "adv7511w", ADV7511 }, { "adv7513", ADV7511 }, -#ifdef CONFIG_DRM_I2C_ADV7533 +#ifdef CONFIG_DRM_I2C_ADV753x { "adv7533", ADV7533 }, + { "adv7535", ADV7535 }, #endif { } }; @@ -1277,8 +1290,9 @@ 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 +#ifdef CONFIG_DRM_I2C_ADV753x { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, #endif { } }; -- 2.22.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-09 14:16 ` [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 Bogdan Togorean @ 2019-08-09 15:25 ` Sam Ravnborg 2019-08-19 8:59 ` Togorean, Bogdan 0 siblings, 1 reply; 11+ messages in thread From: Sam Ravnborg @ 2019-08-09 15:25 UTC (permalink / raw) To: Bogdan Togorean Cc: dri-devel, airlied, daniel, robh+dt, a.hajda, Laurent.pinchart, gregkh, allison, tglx, matt.redfearn, linux-kernel Hi Bogdan. This patch triggered a few general comments. > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > @@ -2,5 +2,5 @@ > adv7511-y := adv7511_drv.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 > +adv7511-$(CONFIG_DRM_I2C_ADV753x) += 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..38288c3c3c53 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > @@ -91,6 +91,7 @@ > #define ADV7511_REG_ARC_CTRL 0xdf > #define ADV7511_REG_CEC_I2C_ADDR 0xe1 > #define ADV7511_REG_CEC_CTRL 0xe2 > +#define ADV7511_REG_SUPPLY_SELECT 0xe4 > #define ADV7511_REG_CHIP_ID_HIGH 0xf5 > #define ADV7511_REG_CHIP_ID_LOW 0xf6 > > @@ -320,6 +321,7 @@ struct adv7511_video_config { > enum adv7511_type { > ADV7511, > ADV7533, > + ADV7535, > }; > > #define ADV7511_MAX_ADDRS 3 > @@ -393,7 +395,7 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) > } > #endif > > -#ifdef CONFIG_DRM_I2C_ADV7533 > +#ifdef CONFIG_DRM_I2C_ADV753x > 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); The else part here define dummy functions. > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index f6d2681f6927..b1501344df3e 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); In the driver we check for adv7511->type - and call adv7533_dsi_power_on() only for the two types where we have this function defined. A simpler approach would be to always call adv7533_dsi_power_on(), and let the existing logic pick up the right version. The dummy version should then return 0 to say OK. Same goes for several places below. > 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,16 @@ 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, > + ADV7511_REG_SUPPLY_SELECT, 0x80, 0x80); > + } > + } > + > adv7511_packet_disable(adv7511, 0xffff); > > adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid", > @@ -1242,7 +1254,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,8 +1278,9 @@ static const struct i2c_device_id adv7511_i2c_ids[] = { > { "adv7511", ADV7511 }, > { "adv7511w", ADV7511 }, > { "adv7513", ADV7511 }, > -#ifdef CONFIG_DRM_I2C_ADV7533 > +#ifdef CONFIG_DRM_I2C_ADV753x > { "adv7533", ADV7533 }, > + { "adv7535", ADV7535 }, > #endif This ifdef may not be needed?? If we did not get this type we will not look it up. > { } > }; > @@ -1277,8 +1290,9 @@ 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 > +#ifdef CONFIG_DRM_I2C_ADV753x > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > #endif > { } > }; Sam ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-09 15:25 ` Sam Ravnborg @ 2019-08-19 8:59 ` Togorean, Bogdan 2019-08-19 10:46 ` Sam Ravnborg 0 siblings, 1 reply; 11+ messages in thread From: Togorean, Bogdan @ 2019-08-19 8:59 UTC (permalink / raw) To: sam Cc: Laurent.pinchart, a.hajda, airlied, gregkh, dri-devel, linux-kernel, allison, tglx, matt.redfearn, daniel, robh+dt Thank you Sam for review, On Fri, 2019-08-09 at 17:25 +0200, Sam Ravnborg wrote: > [External] > > Hi Bogdan. > > This patch triggered a few general comments. > > > --- a/drivers/gpu/drm/bridge/adv7511/Makefile > > +++ b/drivers/gpu/drm/bridge/adv7511/Makefile > > @@ -2,5 +2,5 @@ > > adv7511-y := adv7511_drv.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 > > +adv7511-$(CONFIG_DRM_I2C_ADV753x) += 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..38288c3c3c53 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > > @@ -91,6 +91,7 @@ > > #define ADV7511_REG_ARC_CTRL 0xdf > > #define ADV7511_REG_CEC_I2C_ADDR 0xe1 > > #define ADV7511_REG_CEC_CTRL 0xe2 > > +#define ADV7511_REG_SUPPLY_SELECT 0xe4 > > #define ADV7511_REG_CHIP_ID_HIGH 0xf5 > > #define ADV7511_REG_CHIP_ID_LOW 0xf6 > > > > @@ -320,6 +321,7 @@ struct adv7511_video_config { > > enum adv7511_type { > > ADV7511, > > ADV7533, > > + ADV7535, > > }; > > > > #define ADV7511_MAX_ADDRS 3 > > @@ -393,7 +395,7 @@ static inline int adv7511_cec_init(struct > > device *dev, struct adv7511 *adv7511) > > } > > #endif > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > +#ifdef CONFIG_DRM_I2C_ADV753x > > 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); > > The else part here define dummy functions. > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index f6d2681f6927..b1501344df3e 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); > > In the driver we check for adv7511->type - and call > adv7533_dsi_power_on() only for the two types where we have this > function defined. > A simpler approach would be to always call adv7533_dsi_power_on(), > and > let the existing logic pick up the right version. > The dummy version should then return 0 to say OK. > > Same goes for several places below. > I agree with your remarks. Will implement them in v3 > > > 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,16 @@ 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, > > + ADV7511_REG_SUPPLY_SELECT, 0x80, 0x80); > > + } > > + } > > + > > adv7511_packet_disable(adv7511, 0xffff); > > > > adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid", > > @@ -1242,7 +1254,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,8 +1278,9 @@ static const struct i2c_device_id > > adv7511_i2c_ids[] = { > > { "adv7511", ADV7511 }, > > { "adv7511w", ADV7511 }, > > { "adv7513", ADV7511 }, > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > +#ifdef CONFIG_DRM_I2C_ADV753x > > { "adv7533", ADV7533 }, > > + { "adv7535", ADV7535 }, > > #endif > > This ifdef may not be needed?? > If we did not get this type we will not look it up. But if we have defined in DT adv7533/5 device but CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. That would be ok? > > > { } > > }; > > @@ -1277,8 +1290,9 @@ 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 > > +#ifdef CONFIG_DRM_I2C_ADV753x > > { .compatible = "adi,adv7533", .data = (void *)ADV7533 }, > > + { .compatible = "adi,adv7535", .data = (void *)ADV7535 }, > > #endif > > { } > > }; > > Sam ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-19 8:59 ` Togorean, Bogdan @ 2019-08-19 10:46 ` Sam Ravnborg 2019-08-20 8:53 ` Daniel Vetter 0 siblings, 1 reply; 11+ messages in thread From: Sam Ravnborg @ 2019-08-19 10:46 UTC (permalink / raw) To: Togorean, Bogdan Cc: Laurent.pinchart, a.hajda, airlied, gregkh, dri-devel, linux-kernel, allison, tglx, matt.redfearn, daniel, robh+dt Hi Bogdan. > > > adv7533_detach_dsi(adv7511); > > > i2c_unregister_device(adv7511->i2c_cec); > > > if (adv7511->cec_clk) > > > @@ -1266,8 +1278,9 @@ static const struct i2c_device_id > > > adv7511_i2c_ids[] = { > > > { "adv7511", ADV7511 }, > > > { "adv7511w", ADV7511 }, > > > { "adv7513", ADV7511 }, > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > +#ifdef CONFIG_DRM_I2C_ADV753x > > > { "adv7533", ADV7533 }, > > > + { "adv7535", ADV7535 }, > > > #endif > > > > This ifdef may not be needed?? > > If we did not get this type we will not look it up. > But if we have defined in DT adv7533/5 device but > CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. That > would be ok? What do we gain from this complexity in the end. Why not let the driver always support all variants. If this result in a simpler driver, and less choices in Kconfig then it is a win-win. Sam ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-19 10:46 ` Sam Ravnborg @ 2019-08-20 8:53 ` Daniel Vetter 2019-08-21 5:34 ` Togorean, Bogdan 0 siblings, 1 reply; 11+ messages in thread From: Daniel Vetter @ 2019-08-20 8:53 UTC (permalink / raw) To: Sam Ravnborg Cc: Togorean, Bogdan, Laurent.pinchart, a.hajda, airlied, gregkh, dri-devel, linux-kernel, allison, tglx, matt.redfearn, daniel, robh+dt On Mon, Aug 19, 2019 at 12:46:16PM +0200, Sam Ravnborg wrote: > Hi Bogdan. > > > > > adv7533_detach_dsi(adv7511); > > > > i2c_unregister_device(adv7511->i2c_cec); > > > > if (adv7511->cec_clk) > > > > @@ -1266,8 +1278,9 @@ static const struct i2c_device_id > > > > adv7511_i2c_ids[] = { > > > > { "adv7511", ADV7511 }, > > > > { "adv7511w", ADV7511 }, > > > > { "adv7513", ADV7511 }, > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > > +#ifdef CONFIG_DRM_I2C_ADV753x > > > > { "adv7533", ADV7533 }, > > > > + { "adv7535", ADV7535 }, > > > > #endif > > > > > > This ifdef may not be needed?? > > > If we did not get this type we will not look it up. > > But if we have defined in DT adv7533/5 device but > > CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. That > > would be ok? > > What do we gain from this complexity in the end. > Why not let the driver always support all variants. > > If this result in a simpler driver, and less choices in Kconfig > then it is a win-win. Yeah in general we don't Kconfig within drivers in drm to disable specific code-paths. It's not worth the pain. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-20 8:53 ` Daniel Vetter @ 2019-08-21 5:34 ` Togorean, Bogdan 2019-11-27 11:52 ` Schrempf Frieder 0 siblings, 1 reply; 11+ messages in thread From: Togorean, Bogdan @ 2019-08-21 5:34 UTC (permalink / raw) To: daniel, sam Cc: Laurent.pinchart, a.hajda, airlied, gregkh, dri-devel, linux-kernel, allison, tglx, matt.redfearn, robh+dt On Tue, 2019-08-20 at 10:53 +0200, Daniel Vetter wrote: > [External] > > On Mon, Aug 19, 2019 at 12:46:16PM +0200, Sam Ravnborg wrote: > > Hi Bogdan. > > > > > > > adv7533_detach_dsi(adv7511); > > > > > i2c_unregister_device(adv7511->i2c_cec); > > > > > if (adv7511->cec_clk) > > > > > @@ -1266,8 +1278,9 @@ static const struct i2c_device_id > > > > > adv7511_i2c_ids[] = { > > > > > { "adv7511", ADV7511 }, > > > > > { "adv7511w", ADV7511 }, > > > > > { "adv7513", ADV7511 }, > > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > > > +#ifdef CONFIG_DRM_I2C_ADV753x > > > > > { "adv7533", ADV7533 }, > > > > > + { "adv7535", ADV7535 }, > > > > > #endif > > > > > > > > This ifdef may not be needed?? > > > > If we did not get this type we will not look it up. > > > But if we have defined in DT adv7533/5 device but > > > CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. > > > That > > > would be ok? > > > > What do we gain from this complexity in the end. > > Why not let the driver always support all variants. > > > > If this result in a simpler driver, and less choices in Kconfig > > then it is a win-win. > > Yeah in general we don't Kconfig within drivers in drm to disable > specific > code-paths. It's not worth the pain. Ack, Thank you for clarification. Will remove in V3. > -Daniel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-08-21 5:34 ` Togorean, Bogdan @ 2019-11-27 11:52 ` Schrempf Frieder 2019-11-27 14:22 ` Togorean, Bogdan 0 siblings, 1 reply; 11+ messages in thread From: Schrempf Frieder @ 2019-11-27 11:52 UTC (permalink / raw) To: Togorean, Bogdan, daniel, sam Cc: Laurent.pinchart, a.hajda, airlied, gregkh, dri-devel, linux-kernel, allison, tglx, matt.redfearn, robh+dt Hi Bogdan, On 21.08.19 07:34, Togorean, Bogdan wrote: > On Tue, 2019-08-20 at 10:53 +0200, Daniel Vetter wrote: >> [External] >> >> On Mon, Aug 19, 2019 at 12:46:16PM +0200, Sam Ravnborg wrote: >>> Hi Bogdan. >>> >>>>>> adv7533_detach_dsi(adv7511); >>>>>> i2c_unregister_device(adv7511->i2c_cec); >>>>>> if (adv7511->cec_clk) >>>>>> @@ -1266,8 +1278,9 @@ static const struct i2c_device_id >>>>>> adv7511_i2c_ids[] = { >>>>>> { "adv7511", ADV7511 }, >>>>>> { "adv7511w", ADV7511 }, >>>>>> { "adv7513", ADV7511 }, >>>>>> -#ifdef CONFIG_DRM_I2C_ADV7533 >>>>>> +#ifdef CONFIG_DRM_I2C_ADV753x >>>>>> { "adv7533", ADV7533 }, >>>>>> + { "adv7535", ADV7535 }, >>>>>> #endif >>>>> >>>>> This ifdef may not be needed?? >>>>> If we did not get this type we will not look it up. >>>> But if we have defined in DT adv7533/5 device but >>>> CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. >>>> That >>>> would be ok? >>> >>> What do we gain from this complexity in the end. >>> Why not let the driver always support all variants. >>> >>> If this result in a simpler driver, and less choices in Kconfig >>> then it is a win-win. >> >> Yeah in general we don't Kconfig within drivers in drm to disable >> specific >> code-paths. It's not worth the pain. > > Ack, > Thank you for clarification. Will remove in V3. Are you still working on this? Do you plan to send a v3? I will soon lay my hands on a board with the ADV7535 and would like to see this merged. Also for patch 1/2, it seems you already have a R-b for v1 from Laurent, but you didn't carry the tag to v2. Thanks, Frieder ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-11-27 11:52 ` Schrempf Frieder @ 2019-11-27 14:22 ` Togorean, Bogdan 2019-11-27 14:46 ` Schrempf Frieder 0 siblings, 1 reply; 11+ messages in thread From: Togorean, Bogdan @ 2019-11-27 14:22 UTC (permalink / raw) To: frieder.schrempf Cc: Laurent.pinchart, a.hajda, airlied, gregkh, sam, dri-devel, linux-kernel, allison, tglx, matt.redfearn, robh+dt, daniel Hi Frieder, I'm glad to find there are other persons interested in this driver and especially support for ADV7535. Unfortunately I had to put on hold the development due to other activities but I'll send V3 tomorrow. I also started work on HDCP support for this driver and hope to send soon a patch for that. Best regards, Bogdan On Wed, 2019-11-27 at 11:52 +0000, Schrempf Frieder wrote: > [External] > > Hi Bogdan, > > On 21.08.19 07:34, Togorean, Bogdan wrote: > > On Tue, 2019-08-20 at 10:53 +0200, Daniel Vetter wrote: > > > [External] > > > > > > On Mon, Aug 19, 2019 at 12:46:16PM +0200, Sam Ravnborg wrote: > > > > Hi Bogdan. > > > > > > > > > > > adv7533_detach_dsi(adv7511); > > > > > > > i2c_unregister_device(adv7511->i2c_cec); > > > > > > > if (adv7511->cec_clk) > > > > > > > @@ -1266,8 +1278,9 @@ static const struct i2c_device_id > > > > > > > adv7511_i2c_ids[] = { > > > > > > > { "adv7511", ADV7511 }, > > > > > > > { "adv7511w", ADV7511 }, > > > > > > > { "adv7513", ADV7511 }, > > > > > > > -#ifdef CONFIG_DRM_I2C_ADV7533 > > > > > > > +#ifdef CONFIG_DRM_I2C_ADV753x > > > > > > > { "adv7533", ADV7533 }, > > > > > > > + { "adv7535", ADV7535 }, > > > > > > > #endif > > > > > > > > > > > > This ifdef may not be needed?? > > > > > > If we did not get this type we will not look it up. > > > > > But if we have defined in DT adv7533/5 device but > > > > > CONFIG_DRM_I2C_ADV753x not selected probe will fail with > > > > > ENODEV. > > > > > That > > > > > would be ok? > > > > > > > > What do we gain from this complexity in the end. > > > > Why not let the driver always support all variants. > > > > > > > > If this result in a simpler driver, and less choices in Kconfig > > > > then it is a win-win. > > > > > > Yeah in general we don't Kconfig within drivers in drm to disable > > > specific > > > code-paths. It's not worth the pain. > > > > Ack, > > Thank you for clarification. Will remove in V3. > > Are you still working on this? Do you plan to send a v3? > I will soon lay my hands on a board with the ADV7535 and would like > to > see this merged. > Also for patch 1/2, it seems you already have a R-b for v1 from > Laurent, > but you didn't carry the tag to v2. > > Thanks, > Frieder ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 2019-11-27 14:22 ` Togorean, Bogdan @ 2019-11-27 14:46 ` Schrempf Frieder 0 siblings, 0 replies; 11+ messages in thread From: Schrempf Frieder @ 2019-11-27 14:46 UTC (permalink / raw) To: Togorean, Bogdan Cc: Laurent.pinchart, a.hajda, airlied, gregkh, sam, dri-devel, linux-kernel, allison, tglx, matt.redfearn, robh+dt, daniel On 27.11.19 15:22, Togorean, Bogdan wrote: > Hi Frieder, > > I'm glad to find there are other persons interested in this driver and > especially support for ADV7535. Unfortunately I had to put on hold the > development due to other activities but I'll send V3 tomorrow. Great to hear that. Thanks for your effort. I will try to test your v3 when I have received the new hardware and got it up and running. > > I also started work on HDCP support for this driver and hope to send > soon a patch for that. > > Best regards, > Bogdan > > On Wed, 2019-11-27 at 11:52 +0000, Schrempf Frieder wrote: >> [External] >> >> Hi Bogdan, >> >> On 21.08.19 07:34, Togorean, Bogdan wrote: >>> On Tue, 2019-08-20 at 10:53 +0200, Daniel Vetter wrote: >>>> [External] >>>> >>>> On Mon, Aug 19, 2019 at 12:46:16PM +0200, Sam Ravnborg wrote: >>>>> Hi Bogdan. >>>>> >>>>>>>> adv7533_detach_dsi(adv7511); >>>>>>>> i2c_unregister_device(adv7511->i2c_cec); >>>>>>>> if (adv7511->cec_clk) >>>>>>>> @@ -1266,8 +1278,9 @@ static const struct i2c_device_id >>>>>>>> adv7511_i2c_ids[] = { >>>>>>>> { "adv7511", ADV7511 }, >>>>>>>> { "adv7511w", ADV7511 }, >>>>>>>> { "adv7513", ADV7511 }, >>>>>>>> -#ifdef CONFIG_DRM_I2C_ADV7533 >>>>>>>> +#ifdef CONFIG_DRM_I2C_ADV753x >>>>>>>> { "adv7533", ADV7533 }, >>>>>>>> + { "adv7535", ADV7535 }, >>>>>>>> #endif >>>>>>> >>>>>>> This ifdef may not be needed?? >>>>>>> If we did not get this type we will not look it up. >>>>>> But if we have defined in DT adv7533/5 device but >>>>>> CONFIG_DRM_I2C_ADV753x not selected probe will fail with >>>>>> ENODEV. >>>>>> That >>>>>> would be ok? >>>>> >>>>> What do we gain from this complexity in the end. >>>>> Why not let the driver always support all variants. >>>>> >>>>> If this result in a simpler driver, and less choices in Kconfig >>>>> then it is a win-win. >>>> >>>> Yeah in general we don't Kconfig within drivers in drm to disable >>>> specific >>>> code-paths. It's not worth the pain. >> > >>> Ack, >>> Thank you for clarification. Will remove in V3. >> >> Are you still working on this? Do you plan to send a v3? >> I will soon lay my hands on a board with the ADV7535 and would like >> to >> see this merged. >> Also for patch 1/2, it seems you already have a R-b for v1 from >> Laurent, >> but you didn't carry the tag to v2. >> >> Thanks, >> Frieder ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-11-27 14:46 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-09 14:16 [PATCH v2 0/2] drm: bridge: adv7511: Add support For ADV7535 Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 1/2] dt-bindings: drm: bridge: adv7511: Add ADV7535 support Bogdan Togorean 2019-08-09 14:16 ` [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535 Bogdan Togorean 2019-08-09 15:25 ` Sam Ravnborg 2019-08-19 8:59 ` Togorean, Bogdan 2019-08-19 10:46 ` Sam Ravnborg 2019-08-20 8:53 ` Daniel Vetter 2019-08-21 5:34 ` Togorean, Bogdan 2019-11-27 11:52 ` Schrempf Frieder 2019-11-27 14:22 ` Togorean, Bogdan 2019-11-27 14:46 ` Schrempf Frieder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).