On Wed, 5 May 2021 at 11:03, Marek Vasut <marex@xxxxxxx> wrote:
>
> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> but easy to add.
>
> The driver operates the chip via I2C bus. Currently the LVDS clock are
> always derived from DSI clock lane, which is the usual mode of operation.
> Support for clock from external oscillator is not implemented, but it is
> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> LVDS666 is not supported, but could be added if needed.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Cc: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Loic Poulain <loic.poulain@xxxxxxxxxx>
> Cc: Philippe Schenker <philippe.schenker@xxxxxxxxxxx>
> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Cc: Valentin Raevsky <valentin@xxxxxxxxxxxxxx>
> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> ---
<snip>
> +
> +static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> +
> + /*
> + * Reset the chip, pull EN line low for t_reset=10ms,
> + * then high for t_en=1ms.
> + */
> + regcache_mark_dirty(ctx->regmap);
> + gpiod_set_value(ctx->enable_gpio, 0);
> + usleep_range(10000, 11000);
> + gpiod_set_value(ctx->enable_gpio, 1);
> + usleep_range(1000, 1100);
> +}
> +
<snip>
> +
> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> + unsigned int pval;
> + u16 val;
> + int ret;
> +
> + /* Clear reset, disable PLL */
> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
Sorry, a further thread of discussion coming from the investigations
I've been involved with.
You've powered up in pre_enable, and are sending the I2C writes in enable.
>From the docs for drm_bridge_funcs->enable[1]
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
So video is running when enable is called, and the DSI data lanes may
be HS. (Someone correct me if that is an incorrect reading of the
text).
The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
seq 8 as being "Change DSI data lanes to HS state and start DSI video
stream", AFTER all the I2C has been completed except reading back
registers and checking for errors.
With video running you don't fulfil the second part of init seq 2 "the
DSI data lanes MUST be driven to LP11 state"
My investigations have been over delaying starting the DSI video
stream until after enable, but reading the descriptive text for enable
I believe the Pi is correct to be sending video at that point.
I guess there is some ambiguity as to whether the clock lane is going
to be in HS mode during pre_enable. On the Pi the PHY and clocks will
be enabled prior to pre_enable to allow for sending DSI commands
during pre_enable, but it may not be true on other platforms.
Dave
[1] https://elixir.bootlin.com/linux/latest/source/include/drm/drm_bridge.h#L256
> +
> + /* Reference clock derived from DSI link clock. */
> + regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
> + REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
> + REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
> + regmap_write(ctx->regmap, REG_DSI_CLK,
> + REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx)));
> + regmap_write(ctx->regmap, REG_RC_DSI_CLK,
> + REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));
> +
> + /* Set number of DSI lanes and LVDS link config. */
> + regmap_write(ctx->regmap, REG_DSI_LANE,
> + REG_DSI_LANE_LVDS_LINK_CFG_DUAL |
> + REG_DSI_LANE_CHA_DSI_LANES(~(ctx->dsi_lanes - 1)) |
> + /* CHB is DSI85-only, set to default on DSI83/DSI84 */
> + REG_DSI_LANE_CHB_DSI_LANES(3));
> + /* No equalization. */
> + regmap_write(ctx->regmap, REG_DSI_EQ, 0x00);
> +
> + /* RGB888 is the only format supported so far. */
> + val = (ctx->mode.flags & DRM_MODE_FLAG_NHSYNC ?
> + REG_LVDS_FMT_HS_NEG_POLARITY : 0) |
> + (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC ?
> + REG_LVDS_FMT_VS_NEG_POLARITY : 0) |
> + REG_LVDS_FMT_CHA_24BPP_MODE;
> + if (ctx->lvds_dual_link)
> + val |= REG_LVDS_FMT_CHB_24BPP_MODE;
> + else
> + val |= REG_LVDS_FMT_LVDS_LINK_CFG;
> +
> + regmap_write(ctx->regmap, REG_LVDS_FMT, val);
> + regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
> + regmap_write(ctx->regmap, REG_LVDS_LANE,
> + (ctx->lvds_dual_link_even_odd_swap ?
> + REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
> + REG_LVDS_LANE_CHA_LVDS_TERM |
> + REG_LVDS_LANE_CHB_LVDS_TERM);
> + regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
> +
> + regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
> + &ctx->mode.hdisplay, 2);
> + regmap_bulk_write(ctx->regmap, REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
> + &ctx->mode.vdisplay, 2);
> + val = 32 + 1; /* 32 + 1 pixel clock to ensure proper operation */
> + regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &val, 2);
> + val = ctx->mode.hsync_end - ctx->mode.hsync_start;
> + regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
> + &val, 2);
> + val = ctx->mode.vsync_end - ctx->mode.vsync_start;
> + regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
> + &val, 2);
> + regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
> + ctx->mode.htotal - ctx->mode.hsync_end);
> + regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_BACK_PORCH,
> + ctx->mode.vtotal - ctx->mode.vsync_end);
> + regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
> + ctx->mode.hsync_start - ctx->mode.hdisplay);
> + regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
> + ctx->mode.vsync_start - ctx->mode.vdisplay);
> + regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
> +
> + /* Enable PLL */
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
> + usleep_range(3000, 4000);
> + ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
> + pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
> + 1000, 100000);
> + if (ret) {
> + dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> + /* On failure, disable PLL again and exit. */
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> + return;
> + }
> +
> + /* Trigger reset after CSR register update. */
> + regmap_write(ctx->regmap, REG_RC_RESET, REG_RC_RESET_SOFT_RESET);
> +
> + /* Clear all errors that got asserted during initialization. */
> + regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
> + regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
> +}
> +
