<div dir="auto">Even better :)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Apr 30, 2021, 13:46 Liu Ying <<a href="mailto:victor.liu@nxp.com">victor.liu@nxp.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Robert,<br>
<br>
On Fri, 2021-04-30 at 11:56 +0200, Robert Foss wrote:<br>
> Hey Liu,<br>
> <br>
> This patch does not apply on upstream-drm-misc/drm-misc-next. When it<br>
> passes local testing & building, I'm ready to merge it.<br>
<br>
I see Neil has already pushed this entire patch series to<br>
drm-misc-next.<br>
<br>
<br>
<a href="https://cgit.freedesktop.org/drm/drm-misc/commit/?id=885811372fe101c4299c53eecc9fee72cf927a0c" rel="noreferrer noreferrer" target="_blank">https://cgit.freedesktop.org/drm/drm-misc/commit/?id=885811372fe101c4299c53eecc9fee72cf927a0c</a><br>
<br>
I also see Guido's R-b and T-b tags on this series, though they comes<br>
after Neil's push perhaps.<br>
<br>
Thanks,<br>
Liu Ying<br>
<br>
> <br>
> On Fri, 23 Apr 2021 at 11:42, Liu Ying <<a href="mailto:victor.liu@nxp.com" target="_blank" rel="noreferrer">victor.liu@nxp.com</a>> wrote:<br>
> > This patch replaces ->mode_fixup() with ->atomic_check() so that<br>
> > a full modeset can be requested from there when crtc_state->active<br>
> > is changed to be true(which implies only connector's DPMS is<br>
> > brought<br>
> > out of "Off" status, though not necessarily).  Bridge functions are<br>
> > added or changed to accommodate the ->atomic_check() callback. <br>
> > That<br>
> > full modeset is needed by the up-coming patch which gets MIPI DSI<br>
> > controller and PHY ready in ->mode_set(), because it makes sure<br>
> > ->mode_set() and ->atomic_disable() are called in pairs.<br>
> > <br>
> > Cc: Andrzej Hajda <<a href="mailto:a.hajda@samsung.com" target="_blank" rel="noreferrer">a.hajda@samsung.com</a>><br>
> > Cc: Neil Armstrong <<a href="mailto:narmstrong@baylibre.com" target="_blank" rel="noreferrer">narmstrong@baylibre.com</a>><br>
> > Cc: Robert Foss <<a href="mailto:robert.foss@linaro.org" target="_blank" rel="noreferrer">robert.foss@linaro.org</a>><br>
> > Cc: Laurent Pinchart <<a href="mailto:Laurent.pinchart@ideasonboard.com" target="_blank" rel="noreferrer">Laurent.pinchart@ideasonboard.com</a>><br>
> > Cc: Jonas Karlman <<a href="mailto:jonas@kwiboo.se" target="_blank" rel="noreferrer">jonas@kwiboo.se</a>><br>
> > Cc: Jernej Skrabec <<a href="mailto:jernej.skrabec@siol.net" target="_blank" rel="noreferrer">jernej.skrabec@siol.net</a>><br>
> > Cc: David Airlie <<a href="mailto:airlied@linux.ie" target="_blank" rel="noreferrer">airlied@linux.ie</a>><br>
> > Cc: Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank" rel="noreferrer">daniel@ffwll.ch</a>><br>
> > Cc: Guido Günther <<a href="mailto:agx@sigxcpu.org" target="_blank" rel="noreferrer">agx@sigxcpu.org</a>><br>
> > Cc: Robert Chiras <<a href="mailto:robert.chiras@nxp.com" target="_blank" rel="noreferrer">robert.chiras@nxp.com</a>><br>
> > Cc: NXP Linux Team <<a href="mailto:linux-imx@nxp.com" target="_blank" rel="noreferrer">linux-imx@nxp.com</a>><br>
> > Signed-off-by: Liu Ying <<a href="mailto:victor.liu@nxp.com" target="_blank" rel="noreferrer">victor.liu@nxp.com</a>><br>
> > ---<br>
> > v2->v3:<br>
> > * Split from the single patch in v2 to clarify changes. (Neil)<br>
> > <br>
> >  drivers/gpu/drm/bridge/nwl-dsi.c | 61 ++++++++++++++++++++------<br>
> > ------<br>
> >  1 file changed, 39 insertions(+), 22 deletions(-)<br>
> > <br>
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c<br>
> > b/drivers/gpu/drm/bridge/nwl-dsi.c<br>
> > index 66b67402f1acd..c65ca860712d2 100644<br>
> > --- a/drivers/gpu/drm/bridge/nwl-dsi.c<br>
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c<br>
> > @@ -21,6 +21,7 @@<br>
> >  #include <linux/sys_soc.h><br>
> >  #include <linux/time64.h><br>
> > <br>
> > +#include <drm/drm_atomic_state_helper.h><br>
> >  #include <drm/drm_bridge.h><br>
> >  #include <drm/drm_mipi_dsi.h><br>
> >  #include <drm/drm_of.h><br>
> > @@ -742,7 +743,9 @@ static int nwl_dsi_disable(struct nwl_dsi *dsi)<br>
> >         return 0;<br>
> >  }<br>
> > <br>
> > -static void nwl_dsi_bridge_disable(struct drm_bridge *bridge)<br>
> > +static void<br>
> > +nwl_dsi_bridge_atomic_disable(struct drm_bridge *bridge,<br>
> > +                             struct drm_bridge_state<br>
> > *old_bridge_state)<br>
> >  {<br>
> >         struct nwl_dsi *dsi = bridge_to_dsi(bridge);<br>
> >         int ret;<br>
> > @@ -803,17 +806,6 @@ static int nwl_dsi_get_dphy_params(struct<br>
> > nwl_dsi *dsi,<br>
> >         return 0;<br>
> >  }<br>
> > <br>
> > -static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge,<br>
> > -                                     const struct drm_display_mode<br>
> > *mode,<br>
> > -                                     struct drm_display_mode<br>
> > *adjusted_mode)<br>
> > -{<br>
> > -       /* At least LCDIF + NWL needs active high sync */<br>
> > -       adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |<br>
> > DRM_MODE_FLAG_PVSYNC);<br>
> > -       adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC |<br>
> > DRM_MODE_FLAG_NVSYNC);<br>
> > -<br>
> > -       return true;<br>
> > -}<br>
> > -<br>
> >  static enum drm_mode_status<br>
> >  nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge,<br>
> >                           const struct drm_display_info *info,<br>
> > @@ -831,6 +823,24 @@ nwl_dsi_bridge_mode_valid(struct drm_bridge<br>
> > *bridge,<br>
> >         return MODE_OK;<br>
> >  }<br>
> > <br>
> > +static int nwl_dsi_bridge_atomic_check(struct drm_bridge *bridge,<br>
> > +                                      struct drm_bridge_state<br>
> > *bridge_state,<br>
> > +                                      struct drm_crtc_state<br>
> > *crtc_state,<br>
> > +                                      struct drm_connector_state<br>
> > *conn_state)<br>
> > +{<br>
> > +       struct drm_display_mode *adjusted_mode = &crtc_state-<br>
> > >adjusted_mode;<br>
> > +<br>
> > +       /* At least LCDIF + NWL needs active high sync */<br>
> > +       adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |<br>
> > DRM_MODE_FLAG_PVSYNC);<br>
> > +       adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC |<br>
> > DRM_MODE_FLAG_NVSYNC);<br>
> > +<br>
> > +       /* Do a full modeset if crtc_state->active is changed to be<br>
> > true. */<br>
> > +       if (crtc_state->active_changed && crtc_state->active)<br>
> > +               crtc_state->mode_changed = true;<br>
> > +<br>
> > +       return 0;<br>
> > +}<br>
> > +<br>
> >  static void<br>
> >  nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,<br>
> >                         const struct drm_display_mode *mode,<br>
> > @@ -862,7 +872,9 @@ nwl_dsi_bridge_mode_set(struct drm_bridge<br>
> > *bridge,<br>
> >         drm_mode_debug_printmodeline(adjusted_mode);<br>
> >  }<br>
> > <br>
> > -static void nwl_dsi_bridge_pre_enable(struct drm_bridge *bridge)<br>
> > +static void<br>
> > +nwl_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,<br>
> > +                                struct drm_bridge_state<br>
> > *old_bridge_state)<br>
> >  {<br>
> >         struct nwl_dsi *dsi = bridge_to_dsi(bridge);<br>
> >         int ret;<br>
> > @@ -897,7 +909,9 @@ static void nwl_dsi_bridge_pre_enable(struct<br>
> > drm_bridge *bridge)<br>
> >         }<br>
> >  }<br>
> > <br>
> > -static void nwl_dsi_bridge_enable(struct drm_bridge *bridge)<br>
> > +static void<br>
> > +nwl_dsi_bridge_atomic_enable(struct drm_bridge *bridge,<br>
> > +                            struct drm_bridge_state<br>
> > *old_bridge_state)<br>
> >  {<br>
> >         struct nwl_dsi *dsi = bridge_to_dsi(bridge);<br>
> >         int ret;<br>
> > @@ -942,14 +956,17 @@ static void nwl_dsi_bridge_detach(struct<br>
> > drm_bridge *bridge)<br>
> >  }<br>
> > <br>
> >  static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {<br>
> > -       .pre_enable = nwl_dsi_bridge_pre_enable,<br>
> > -       .enable     = nwl_dsi_bridge_enable,<br>
> > -       .disable    = nwl_dsi_bridge_disable,<br>
> > -       .mode_fixup = nwl_dsi_bridge_mode_fixup,<br>
> > -       .mode_set   = nwl_dsi_bridge_mode_set,<br>
> > -       .mode_valid = nwl_dsi_bridge_mode_valid,<br>
> > -       .attach     = nwl_dsi_bridge_attach,<br>
> > -       .detach     = nwl_dsi_bridge_detach,<br>
> > +       .atomic_duplicate_state =<br>
> > drm_atomic_helper_bridge_duplicate_state,<br>
> > +       .atomic_destroy_state   =<br>
> > drm_atomic_helper_bridge_destroy_state,<br>
> > +       .atomic_reset           = drm_atomic_helper_bridge_reset,<br>
> > +       .atomic_check           = nwl_dsi_bridge_atomic_check,<br>
> > +       .atomic_pre_enable      = nwl_dsi_bridge_atomic_pre_enable,<br>
> > +       .atomic_enable          = nwl_dsi_bridge_atomic_enable,<br>
> > +       .atomic_disable         = nwl_dsi_bridge_atomic_disable,<br>
> > +       .mode_set               = nwl_dsi_bridge_mode_set,<br>
> > +       .mode_valid             = nwl_dsi_bridge_mode_valid,<br>
> > +       .attach                 = nwl_dsi_bridge_attach,<br>
> > +       .detach                 = nwl_dsi_bridge_detach,<br>
> >  };<br>
> > <br>
> >  static int nwl_dsi_parse_dt(struct nwl_dsi *dsi)<br>
<br>
</blockquote></div>