On Mon, Jun 13, 2022 at 5:02 PM Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > > On 13.06.2022 13:17, Jagan Teki wrote: > > On Wed, May 11, 2022 at 4:01 PM Andrzej Hajda <andrzej.hajda@xxxxxxxxx> wrote: > >> On 04.05.2022 13:40, Jagan Teki wrote: > >>> Fixing up the mode flags is required in order to correlate the > >>> correct sync flags of the surrounding components in the chain > >>> to make sure the whole pipeline can work properly. > >>> > >>> So, handle the mode flags via bridge, atomic_check. > >>> > >>> v2: > >>> * none > >>> > >>> v1: > >>> * fix mode flags in atomic_check instead of mode_fixup > >>> > >>> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> > >>> --- > >>> drivers/gpu/drm/bridge/samsung-dsim.c | 14 ++++++++++++++ > >>> 1 file changed, 14 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > >>> index b618e52d0ee3..bd78cef890e4 100644 > >>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c > >>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > >>> @@ -1340,6 +1340,19 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge, > >>> pm_runtime_put_sync(dsi->dev); > >>> } > >>> > >>> +static int samsung_dsim_atomic_check(struct drm_bridge *bridge, > >>> + struct drm_bridge_state *bridge_state, > >>> + struct drm_crtc_state *crtc_state, > >>> + struct drm_connector_state *conn_state) > >>> +{ > >>> + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; > >>> + > >>> + adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > >>> + adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > >> > >> 1. Shouldn't this be in mode_fixup callback? > > Possible to do it on mode_fixup, yes. if we want to do the same stuff > > on atomic API then atomic_check is the proper helper. > > > >> 2. Where this requirement comes from? As Marek says it breaks Samsung > >> platforms and is against DSIM specification[1]: > > At least the bridge chain starting from LCDIF+DSIM requires active high sync. > > Then please make this specific to the imx variant. The current version > breaks DSI operation on all Exynos based boards. But exynos trm also says the same? "45.2.2.1.2 RGB Interface Vsync, Hsync, and VDEN are active high signals" Jagan.