[PATCH v2 08/12] drm: bridge: samsung-dsim: Add atomic_check

[Lucas Stach]

Am Montag, dem 13.06.2022 um 13:36 +0200 schrieb Marek Szyprowski:
> On 13.06.2022 13:34, Jagan Teki wrote:
> > On Mon, Jun 13, 2022 at 5:02 PM Marek Szyprowski
> > <m.szyprowski at samsung.com> wrote:
> > > On 13.06.2022 13:17, Jagan Teki wrote:
> > > > On Wed, May 11, 2022 at 4:01 PM Andrzej Hajda <andrzej.hajda at intel.com> 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 at amarulasolutions.com>
> > > > > > ---
> > > > > >     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"
> 
> Right, but You are forcing the negative sync signals:
> 
> adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> 
And this is a quirk of the i.MX8MM (and possibly i.MX8MN) SoC
integration. The same Samsung DSI IP core on the i.MX8MP does require
active high sync signals, matching the documentation.

Regards,
Lucas