Re: [PATCH v3 2/2] drm/bridge: Add tc358768 driver

From: Peter Ujfalusi
Date: Mon Jan 27 2020 - 06:59:06 EST




On 27/01/2020 12.56, Peter Ujfalusi wrote:
> Add basic support for the Toshiba TC358768 RGB to DSI bridge.
> Not all the features of the TC358768 is implemented by the initial driver:
> MIPI_DSI_MODE_VIDEO and MIPI_DSI_FMT_RGB888 is only supported and tested.
>
> Only write is implemented for mipi_dsi_host_ops.transfer.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
> ---
> drivers/gpu/drm/bridge/Kconfig | 10 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/tc358768.c | 1040 +++++++++++++++++++++++++++++
> 3 files changed, 1051 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/tc358768.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 0b9ca5862455..3fef3513bdd0 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -122,6 +122,16 @@ config DRM_TOSHIBA_TC358767
> ---help---
> Toshiba TC358767 eDP bridge chip driver.
>
> +config DRM_TOSHIBA_TC358768
> + tristate "Toshiba TC358768 MIPI DSI bridge"
> + depends on OF
> + select DRM_KMS_HELPER
> + select REGMAP_I2C
> + select DRM_PANEL
> + select DRM_MIPI_DSI
> + help
> + Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
> +
> config DRM_TI_TFP410
> tristate "TI TFP410 DVI/HDMI bridge"
> depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index cd16ce830270..06fc265de0ef 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
> obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> new file mode 100644
> index 000000000000..244309c1112e
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -0,0 +1,1040 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <video/mipi_display.h>
> +#include <video/videomode.h>
> +
> +/* Global (16-bit addressable) */
> +#define TC358768_CHIPID 0x0000
> +#define TC358768_SYSCTL 0x0002
> +#define TC358768_CONFCTL 0x0004
> +#define TC358768_VSDLY 0x0006
> +#define TC358768_DATAFMT 0x0008
> +#define TC358768_GPIOEN 0x000E
> +#define TC358768_GPIODIR 0x0010
> +#define TC358768_GPIOIN 0x0012
> +#define TC358768_GPIOOUT 0x0014
> +#define TC358768_PLLCTL0 0x0016
> +#define TC358768_PLLCTL1 0x0018
> +#define TC358768_CMDBYTE 0x0022
> +#define TC358768_PP_MISC 0x0032
> +#define TC358768_DSITX_DT 0x0050
> +#define TC358768_FIFOSTATUS 0x00F8
> +
> +/* Debug (16-bit addressable) */
> +#define TC358768_VBUFCTRL 0x00E0
> +#define TC358768_DBG_WIDTH 0x00E2
> +#define TC358768_DBG_VBLANK 0x00E4
> +#define TC358768_DBG_DATA 0x00E8
> +
> +/* TX PHY (32-bit addressable) */
> +#define TC358768_CLW_DPHYCONTTX 0x0100
> +#define TC358768_D0W_DPHYCONTTX 0x0104
> +#define TC358768_D1W_DPHYCONTTX 0x0108
> +#define TC358768_D2W_DPHYCONTTX 0x010C
> +#define TC358768_D3W_DPHYCONTTX 0x0110
> +#define TC358768_CLW_CNTRL 0x0140
> +#define TC358768_D0W_CNTRL 0x0144
> +#define TC358768_D1W_CNTRL 0x0148
> +#define TC358768_D2W_CNTRL 0x014C
> +#define TC358768_D3W_CNTRL 0x0150
> +
> +/* TX PPI (32-bit addressable) */
> +#define TC358768_STARTCNTRL 0x0204
> +#define TC358768_DSITXSTATUS 0x0208
> +#define TC358768_LINEINITCNT 0x0210
> +#define TC358768_LPTXTIMECNT 0x0214
> +#define TC358768_TCLK_HEADERCNT 0x0218
> +#define TC358768_TCLK_TRAILCNT 0x021C
> +#define TC358768_THS_HEADERCNT 0x0220
> +#define TC358768_TWAKEUP 0x0224
> +#define TC358768_TCLK_POSTCNT 0x0228
> +#define TC358768_THS_TRAILCNT 0x022C
> +#define TC358768_HSTXVREGCNT 0x0230
> +#define TC358768_HSTXVREGEN 0x0234
> +#define TC358768_TXOPTIONCNTRL 0x0238
> +#define TC358768_BTACNTRL1 0x023C
> +
> +/* TX CTRL (32-bit addressable) */
> +#define TC358768_DSI_CONTROL 0x040C
> +#define TC358768_DSI_STATUS 0x0410
> +#define TC358768_DSI_INT 0x0414
> +#define TC358768_DSI_INT_ENA 0x0418
> +#define TC358768_DSICMD_RDFIFO 0x0430
> +#define TC358768_DSI_ACKERR 0x0434
> +#define TC358768_DSI_ACKERR_INTENA 0x0438
> +#define TC358768_DSI_ACKERR_HALT 0x043c
> +#define TC358768_DSI_RXERR 0x0440
> +#define TC358768_DSI_RXERR_INTENA 0x0444
> +#define TC358768_DSI_RXERR_HALT 0x0448
> +#define TC358768_DSI_ERR 0x044C
> +#define TC358768_DSI_ERR_INTENA 0x0450
> +#define TC358768_DSI_ERR_HALT 0x0454
> +#define TC358768_DSI_CONFW 0x0500
> +#define TC358768_DSI_LPCMD 0x0500
> +#define TC358768_DSI_RESET 0x0504
> +#define TC358768_DSI_INT_CLR 0x050C
> +#define TC358768_DSI_START 0x0518
> +
> +/* DSITX CTRL (16-bit addressable) */
> +#define TC358768_DSICMD_TX 0x0600
> +#define TC358768_DSICMD_TYPE 0x0602
> +#define TC358768_DSICMD_WC 0x0604
> +#define TC358768_DSICMD_WD0 0x0610
> +#define TC358768_DSICMD_WD1 0x0612
> +#define TC358768_DSICMD_WD2 0x0614
> +#define TC358768_DSICMD_WD3 0x0616
> +#define TC358768_DSI_EVENT 0x0620
> +#define TC358768_DSI_VSW 0x0622
> +#define TC358768_DSI_VBPR 0x0624
> +#define TC358768_DSI_VACT 0x0626
> +#define TC358768_DSI_HSW 0x0628
> +#define TC358768_DSI_HBPR 0x062A
> +#define TC358768_DSI_HACT 0x062C
> +
> +/* TC358768_DSI_CONTROL (0x040C) register */
> +#define TC358768_DSI_CONTROL_DIS_MODE BIT(15)
> +#define TC358768_DSI_CONTROL_TXMD BIT(7)
> +#define TC358768_DSI_CONTROL_HSCKMD BIT(5)
> +#define TC358768_DSI_CONTROL_EOTDIS BIT(0)
> +
> +/* TC358768_DSI_CONFW (0x0500) register */
> +#define TC358768_DSI_CONFW_MODE_SET (5 << 29)
> +#define TC358768_DSI_CONFW_MODE_CLR (6 << 29)
> +#define TC358768_DSI_CONFW_ADDR_DSI_CONTROL (0x3 << 24)
> +
> +static const char * const tc358768_supplies[] = {
> + "vddc", "vddmipi", "vddio"
> +};
> +
> +struct tc358768_dsi_output {
> + struct mipi_dsi_device *dev;
> + struct drm_panel *panel;
> + struct drm_bridge *bridge;
> +};
> +
> +struct tc358768_priv {
> + struct device *dev;
> + struct regmap *regmap;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(tc358768_supplies)];
> + struct clk *refclk;
> + int enabled;
> + int error;
> +
> + struct mipi_dsi_host dsi_host;
> + struct drm_bridge bridge;
> + struct tc358768_dsi_output output;
> +
> + u32 pd_lines; /* number of Parallel Port Input Data Lines */
> + u32 dsi_lanes; /* number of DSI Lanes */
> +
> + /* Parameters for PLL programming */
> + u32 fbd; /* PLL feedback divider */
> + u32 prd; /* PLL input divider */
> + u32 frs; /* PLL Freqency range for HSCK (post divider) */
> +
> + u32 dsiclk; /* pll_clk / 2 */
> +};
> +
> +static inline struct tc358768_priv *dsi_host_to_tc358768(struct mipi_dsi_host
> + *host)
> +{
> + return container_of(host, struct tc358768_priv, dsi_host);
> +}
> +
> +static inline struct tc358768_priv *bridge_to_tc358768(struct drm_bridge
> + *bridge)
> +{
> + return container_of(bridge, struct tc358768_priv, bridge);
> +}
> +
> +static int tc358768_clear_error(struct tc358768_priv *priv)
> +{
> + int ret = priv->error;
> +
> + priv->error = 0;
> + return ret;
> +}
> +
> +static void tc358768_write(struct tc358768_priv *priv, u32 reg, u32 val)
> +{
> + size_t count = 2;
> +
> + if (priv->error)
> + return;
> +
> + /* 16-bit register? */
> + if (reg < 0x100 || reg >= 0x600)
> + count = 1;
> +
> + priv->error = regmap_bulk_write(priv->regmap, reg, &val, count);
> +}
> +
> +static void tc358768_read(struct tc358768_priv *priv, u32 reg, u32 *val)
> +{
> + size_t count = 2;
> +
> + if (priv->error)
> + return;
> +
> + /* 16-bit register? */
> + if (reg < 0x100 || reg >= 0x600) {
> + *val = 0;
> + count = 1;
> + }
> +
> + priv->error = regmap_bulk_read(priv->regmap, reg, val, count);
> +}
> +
> +static void tc358768_update_bits(struct tc358768_priv *priv, u32 reg, u32 mask,
> + u32 val)
> +{
> + u32 tmp, orig;
> +
> + tc358768_read(priv, reg, &orig);
> + tmp = orig & ~mask;
> + tmp |= val & mask;
> + if (tmp != orig)
> + tc358768_write(priv, reg, tmp);
> +}
> +
> +static int tc358768_sw_reset(struct tc358768_priv *priv)
> +{
> + /* Assert Reset */
> + tc358768_write(priv, TC358768_SYSCTL, 1);
> + /* Release Reset, Exit Sleep */
> + tc358768_write(priv, TC358768_SYSCTL, 0);
> +
> + return tc358768_clear_error(priv);
> +}
> +
> +static void tc358768_hw_enable(struct tc358768_priv *priv)
> +{
> + int ret;
> +
> + if (priv->enabled++ > 0)
> + return;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
> + if (ret < 0)
> + dev_err(priv->dev, "error enabling regulators (%d)\n", ret);
> +
> + if (priv->reset_gpio)
> + usleep_range(200, 300);
> +
> + /*
> + * The RESX is active low (GPIO_ACTIVE_LOW).
> + * DEASSERT (value = 0) the reset_gpio to enable the chip
> + */
> + gpiod_set_value_cansleep(priv->reset_gpio, 0);
> +
> + /* wait for encoder clocks to stabilize */
> + usleep_range(1000, 2000);
> +}
> +
> +static void tc358768_hw_disable(struct tc358768_priv *priv)
> +{
> + int ret;
> +
> + if (--priv->enabled != 0)

I should be doing:
if (!priv->enabled || --priv->enabled != 0)

to avoid going negative on the enabled, which could happen I think if an
enable path fails and later the bridge is disabled.

- PÃter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki