Re: [PATCH 12/14] drm: bridge: icn6211: Add I2C configuration support

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1: Type: text/plain, Size: 5185 bytes --]

On Fri, Jan 14, 2022 at 04:48:36AM +0100, Marek Vasut wrote:
> The ICN6211 chip starts in I2C configuration mode after cold boot.
> Implement support for configuring the chip via I2C in addition to
> the current DSI LP command mode configuration support. The later
> seems to be available only on chips which have additional MCU on
> the panel/bridge board which preconfigures the ICN6211, while the
> I2C configuration mode added by this patch does not require any
> such MCU.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> To: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/bridge/chipone-icn6211.c | 219 +++++++++++++++++++----
>  1 file changed, 188 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
> index 8226fefeedfc9..313c588297eca 100644
> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c
> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/regulator/consumer.h>
> @@ -133,14 +134,17 @@
>  
>  struct chipone {
>  	struct device *dev;
> +	struct i2c_client *client;
>  	struct drm_bridge bridge;
>  	struct drm_bridge *panel_bridge;
>  	struct device_node *host_node;
> +	struct mipi_dsi_device *dsi;
>  	struct gpio_desc *enable_gpio;
>  	struct regulator *vdd1;
>  	struct regulator *vdd2;
>  	struct regulator *vdd3;
>  	int dsi_lanes;
> +	bool interface_i2c;
>  };
>  
>  static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge)
> @@ -172,20 +176,14 @@ bridge_to_mode(struct drm_bridge *bridge, struct drm_atomic_state *state)
>  	return &crtc_state->adjusted_mode;
>  }
>  
> -static inline int chipone_dsi_write(struct chipone *icn,  const void *seq,
> -				    size_t len)
> +static void ICN6211_DSI(struct chipone *icn, u8 reg, u8 val)
>  {
> -	struct mipi_dsi_device *dsi = to_mipi_dsi_device(icn->dev);
> -
> -	return mipi_dsi_generic_write(dsi, seq, len);
> +	if (icn->interface_i2c)
> +		i2c_smbus_write_byte_data(icn->client, reg, val);
> +	else
> +		mipi_dsi_generic_write(icn->dsi, (u8[]){reg, val}, 2);
>  }
>  
> -#define ICN6211_DSI(icn, seq...)				\
> -	{							\
> -		const u8 d[] = { seq };				\
> -		chipone_dsi_write(icn, d, ARRAY_SIZE(d));	\
> -	}
> -
>  static void chipone_configure_pll(struct chipone *icn,
>  				  const struct drm_display_mode *mode)
>  {
> @@ -282,7 +280,10 @@ static void chipone_atomic_enable(struct drm_bridge *bridge,
>  	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
>  	bus_flags = bridge_state->output_bus_cfg.flags;
>  
> -	ICN6211_DSI(icn, MIPI_CFG_PW, MIPI_CFG_PW_CONFIG_DSI);
> +	if (icn->interface_i2c)
> +		ICN6211_DSI(icn, MIPI_CFG_PW, MIPI_CFG_PW_CONFIG_I2C);
> +	else
> +		ICN6211_DSI(icn, MIPI_CFG_PW, MIPI_CFG_PW_CONFIG_DSI);
>  
>  	ICN6211_DSI(icn, HACTIVE_LI, mode->hdisplay & 0xff);
>  
> @@ -396,11 +397,86 @@ static void chipone_atomic_post_disable(struct drm_bridge *bridge,
>  	gpiod_set_value(icn->enable_gpio, 0);
>  }
>  
> +static int chipone_dsi_attach(struct chipone *icn)
> +{
> +	struct mipi_dsi_device *dsi = icn->dsi;
> +	int ret;
> +
> +	dsi->lanes = icn->dsi_lanes;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0)
> +		dev_err(icn->dev, "failed to attach dsi\n");
> +
> +	return ret;
> +}
> +
> +static int chipone_dsi_setup(struct chipone *icn)
> +{
> +	struct device *dev = icn->dev;
> +	struct mipi_dsi_device *dsi;
> +	struct mipi_dsi_host *host;
> +	int ret = 0;
> +
> +	const struct mipi_dsi_device_info info = {
> +		.type = "chipone",
> +		.channel = 0,
> +		.node = NULL,
> +	};
> +
> +	host = of_find_mipi_dsi_host_by_node(icn->host_node);
> +	if (!host) {
> +		dev_err(dev, "failed to find dsi host\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	dsi = mipi_dsi_device_register_full(host, &info);
> +	if (IS_ERR(dsi)) {
> +		return dev_err_probe(dev, PTR_ERR(dsi),
> +				     "failed to create dsi device\n");
> +	}
> +
> +	icn->dsi = dsi;
> +
> +	ret = chipone_dsi_attach(icn);
> +	if (ret < 0)
> +		mipi_dsi_device_unregister(dsi);
> +
> +	return ret;
> +}
> +
>  static int chipone_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags)
>  {
>  	struct chipone *icn = bridge_to_chipone(bridge);
> +	struct drm_bridge *abridge = bridge;
> +	int ret;
> +
> +	if (icn->interface_i2c) {
> +		ret = chipone_dsi_setup(icn);
> +		if (ret)
> +			return ret;
> +
> +		abridge = &icn->bridge;

This needs to happen at probe/bind time. See:
https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#special-care-with-mipi-dsi-bridges

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]