On Mon, Jul 24, 2023 at 8:16 AM Frieder Schrempf <frie...@fris.de> wrote:
>
> From: Frieder Schrempf <frieder.schre...@kontron.de>
>
> In case the downstream bridge or panel uses DSI transfers before the
> DSI host was actually initialized through samsung_dsim_atomic_enable()
> which clears the stop state (LP11) mode, all transfers will fail.
>
> This happens with downstream bridges that are controlled by DSI
> commands such as the tc358762.
>
> As documented in [1] DSI hosts are expected to allow transfers
> outside the normal bridge enable/disable flow.
>
> To fix this make sure that stop state is cleared in
> samsung_dsim_host_transfer() which restores the previous
> behavior.
>
> We also factor out the common code to enable/disable stop state
> to samsung_dsim_set_stop_state().
>
> [1] https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
>
> Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow to 
> meet spec")
> Reported-by: Tim Harvey <thar...@gateworks.com>
> Signed-off-by: Frieder Schrempf <frieder.schre...@kontron.de>
> ---
> Changes for v2:
>   * Fix reversed stop state enable/disable in samsung_dsim_set_stop_state()
>
> Hi Tim, could you please give this patch a try and report back if
> it fixes your problem? Thanks!
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 043b8109e64a..73ec60757dbc 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1386,6 +1386,18 @@ static void samsung_dsim_disable_irq(struct 
> samsung_dsim *dsi)
>         disable_irq(dsi->irq);
>  }
>
> +static void samsung_dsim_set_stop_state(struct samsung_dsim *dsi, bool 
> enable)
> +{
> +       u32 reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> +
> +       if (enable)
> +               reg |= DSIM_FORCE_STOP_STATE;
> +       else
> +               reg &= ~DSIM_FORCE_STOP_STATE;
> +
> +       samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +}
> +
>  static int samsung_dsim_init(struct samsung_dsim *dsi)
>  {
>         const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> @@ -1445,15 +1457,12 @@ static void samsung_dsim_atomic_enable(struct 
> drm_bridge *bridge,
>                                        struct drm_bridge_state 
> *old_bridge_state)
>  {
>         struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> -       u32 reg;
>
>         if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>                 samsung_dsim_set_display_mode(dsi);
>                 samsung_dsim_set_display_enable(dsi, true);
>         } else {
> -               reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> -               reg &= ~DSIM_FORCE_STOP_STATE;
> -               samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> +               samsung_dsim_set_stop_state(dsi, false);
>         }
>
>         dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> @@ -1463,16 +1472,12 @@ static void samsung_dsim_atomic_disable(struct 
> drm_bridge *bridge,
>                                         struct drm_bridge_state 
> *old_bridge_state)
>  {
>         struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> -       u32 reg;
>
>         if (!(dsi->state & DSIM_STATE_ENABLED))
>                 return;
>
> -       if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
> -               reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
> -               reg |= DSIM_FORCE_STOP_STATE;
> -               samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
> -       }
> +       if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> +               samsung_dsim_set_stop_state(dsi, true);
>
>         dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>  }
> @@ -1775,6 +1780,8 @@ static ssize_t samsung_dsim_host_transfer(struct 
> mipi_dsi_host *host,
>         if (ret)
>                 return ret;
>
> +       samsung_dsim_set_stop_state(dsi, false);
> +
>         ret = mipi_dsi_create_packet(&xfer.packet, msg);
>         if (ret < 0)
>                 return ret;
> --
> 2.41.0
>

Frieder,

Sorry for the delay. Yes this resolves the regression I ran into. I
tested it on top of v6.5-rc6 on a gw72xx-0x with a DFROBOT DRF0678 7in
800x480 (Raspberry Pi) display which has the Toshiba TC358762
compatible DSI to DBI bridge.

Let's please get this into v6.5 as soon as possible.

Best regards,

Tim

Reply via email to