On Wed, Jan 08, 2025 at 11:19:02AM +0100, Herve Codina wrote: > In some cases observed during ESD tests, the TI SN65DSI83 cannot recover > from errors by itself. A full restart of the bridge is needed in those > cases to have the bridge output LVDS signals again. > > Also, during tests, cases were observed where reading the status of the > bridge was not even possible. Indeed, in those cases, the bridge stops > to acknowledge I2C transactions. Only a full reset of the bridge (power > off/on) brings back the bridge to a functional state. > > The TI SN65DSI83 has some error detection capabilities. Introduce an > error recovery mechanism based on this detection. > > The errors detected are signaled through an interrupt. On system where > this interrupt is not available, the driver uses a polling monitoring > fallback to check for errors. When an error is present or when reading > the bridge status leads to an I2C failure, the recovery process is > launched. > > Restarting the bridge needs to redo the initialization sequence. This > initialization sequence has to be done with the DSI data lanes driven in > LP11 state. In order to do that, the recovery process resets the whole > output path (i.e the path from the encoder to the connector) where the > bridge is located. > > Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx> > --- > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 147 ++++++++++++++++++++++++++ > 1 file changed, 147 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > index e6264514bb3f..74bc05647436 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -35,9 +35,12 @@ > #include <linux/of_graph.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > +#include <linux/timer.h> > +#include <linux/workqueue.h> > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > +#include <drm/drm_drv.h> /* DRM_MODESET_LOCK_ALL_BEGIN() needs drm_drv_uses_atomic_modeset() */ > #include <drm/drm_mipi_dsi.h> > #include <drm/drm_of.h> > #include <drm/drm_panel.h> > @@ -147,6 +150,9 @@ struct sn65dsi83 { > struct regulator *vcc; > bool lvds_dual_link; > bool lvds_dual_link_even_odd_swap; > + bool use_irq; > + struct delayed_work monitor_work; > + struct work_struct reset_work; > }; > > static const struct regmap_range sn65dsi83_readable_ranges[] = { > @@ -328,6 +334,111 @@ static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx) > return dsi_div - 1; > } > > +static int sn65dsi83_reset_pipe(struct sn65dsi83 *sn65dsi83) > +{ > + struct drm_atomic_state *state = ERR_PTR(-EINVAL); > + struct drm_device *dev = sn65dsi83->bridge.dev; > + struct drm_connector_state *connector_state; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_connector *connector; > + int err; > + > + /* > + * Reset active outputs of the related CRTC. > + * > + * This way, drm core will reconfigure each components in the CRTC > + * outputs path. In our case, this will force the previous component to > + * go back in LP11 mode and so allow the reconfiguration of SN64DSI83 > + * bridge. > + * > + * Keep the lock during the whole operation to be atomic. > + */ > + > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err); > + > + state = drm_atomic_helper_duplicate_state(dev, &ctx); > + if (IS_ERR(state)) { > + err = PTR_ERR(state); > + goto unlock; > + } No, you must not allocate a new state for this, you need to reuse the existing state. You'll find it in bridge->base.state->state. > + state->acquire_ctx = &ctx; > + > + connector = drm_atomic_get_old_connector_for_encoder(state, > + sn65dsi83->bridge.encoder); > + if (!connector) { > + err = -EINVAL; > + goto unlock; > + } > + > + connector_state = drm_atomic_get_connector_state(state, connector); > + if (IS_ERR(connector_state)) { > + err = PTR_ERR(connector_state); > + goto unlock; > + } > + > + err = drm_atomic_helper_reset_pipe(connector_state->crtc, &ctx); > + if (err < 0) > + goto unlock; And you'll find the crtc in bridge->encoder->crtc. Maxime
Attachment:
signature.asc
Description: PGP signature