diff mbox series

[v3,8/8] drm/i915/display/icl: In port sync mode disable slaves first then masters

Message ID 20190624210850.17223-9-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable Transcoder Port Sync feature for tiled displays | expand

Commit Message

Navare, Manasi June 24, 2019, 9:08 p.m. UTC
In the transcoder port sync mode, the slave transcoders mask their vblanks
until master transcoder's vblank so while disabling them, make
sure slaves are disabled first and then the masters.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 117 ++++++++++++++++++-
 1 file changed, 111 insertions(+), 6 deletions(-)

Comments

Lucas De Marchi June 25, 2019, 6:34 a.m. UTC | #1
On Mon, Jun 24, 2019 at 2:07 PM Manasi Navare <manasi.d.navare@intel.com> wrote:
>
> In the transcoder port sync mode, the slave transcoders mask their vblanks
> until master transcoder's vblank so while disabling them, make
> sure slaves are disabled first and then the masters.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 117 ++++++++++++++++++-
>  1 file changed, 111 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 0a0d97ef03d6..85746a26d0e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13901,6 +13901,106 @@ static void intel_commit_modeset_disables(struct drm_atomic_state *state)
>         }
>  }
>
> +static void icl_commit_modeset_disables(struct drm_atomic_state *state)
> +{
> +       struct drm_device *dev = state->dev;
> +       struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +       struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
> +       struct drm_crtc *crtc;
> +       struct intel_crtc *intel_crtc;
> +       int i;
> +
> +       /*
> +        * Disable all the Port Sync Slaves first
> +        */
> +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> +               intel_crtc = to_intel_crtc(crtc);
> +
> +               if (!needs_modeset(new_crtc_state) ||
> +                   !is_trans_port_sync_slave(old_intel_crtc_state))
> +                       continue;
> +
> +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> +
> +               if (old_crtc_state->active) {
> +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> +
> +                       /*
> +                        * We need to disable pipe CRC before disabling the pipe,
> +                        * or we race against vblank off.
> +                        */
> +                       intel_crtc_disable_pipe_crc(intel_crtc);
> +
> +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> +                       intel_crtc->active = false;
> +                       intel_fbc_disable(intel_crtc);
> +                       intel_disable_shared_dpll(old_intel_crtc_state);
> +
> +                       /*
> +                        * Underruns don't always raise interrupts,
> +                        * so check manually.
> +                        */
> +                       intel_check_cpu_fifo_underruns(dev_priv);
> +                       intel_check_pch_fifo_underruns(dev_priv);
> +
> +                       /* FIXME unify this for all platforms */
> +                       if (!new_crtc_state->active &&
> +                           !HAS_GMCH(dev_priv) &&
> +                           dev_priv->display.initial_watermarks)
> +                               dev_priv->display.initial_watermarks(intel_state,
> +                                                                    new_intel_crtc_state);
> +               }
> +       }
> +
> +       /*
> +        * Disable rest of the CRTCs other than slaves
> +        */
> +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> +               intel_crtc = to_intel_crtc(crtc);
> +
> +               if (!needs_modeset(new_crtc_state) ||
> +                   is_trans_port_sync_slave(old_intel_crtc_state))
> +                       continue;
> +
> +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> +
> +               if (old_crtc_state->active) {
> +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> +
> +                       /*
> +                        * We need to disable pipe CRC before disabling the pipe,
> +                        * or we race against vblank off.
> +                        */
> +                       intel_crtc_disable_pipe_crc(intel_crtc);
> +
> +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> +                       intel_crtc->active = false;
> +                       intel_fbc_disable(intel_crtc);
> +                       intel_disable_shared_dpll(old_intel_crtc_state);
> +
> +                       /*
> +                        * Underruns don't always raise interrupts,
> +                        * so check manually.
> +                        */
> +                       intel_check_cpu_fifo_underruns(dev_priv);
> +                       intel_check_pch_fifo_underruns(dev_priv);
> +
> +                       /* FIXME unify this for all platforms */
> +                       if (!new_crtc_state->active &&
> +                           !HAS_GMCH(dev_priv) &&
> +                           dev_priv->display.initial_watermarks)
> +                               dev_priv->display.initial_watermarks(intel_state,
> +                                                                    new_intel_crtc_state);
> +               }
> +       }

Not a fan of this duplicate code here. At least a helper function
would be needed.

Jose, didn't you have a pending patch to disable in reverse order to
solve this same problem?

Lucas De Marchi

> +}
> +
>  static void intel_commit_modeset_enables(struct drm_atomic_state *state)
>  {
>         struct drm_crtc *crtc;
> @@ -14257,6 +14357,11 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>
>         intel_atomic_commit_fence_wait(intel_state);
>
> +       drm_atomic_helper_wait_for_dependencies(state);
> +
> +       if (intel_state->modeset)
> +               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> +
>         for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>                 new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
>                 intel_crtc = to_intel_crtc(crtc);
> @@ -14269,11 +14374,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>                 }
>         }
>
> -       drm_atomic_helper_wait_for_dependencies(state);
> -
> -       if (intel_state->modeset)
> -               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> -
>         dev_priv->display.commit_modeset_disables(state);
>
>         /* FIXME: Eventually get rid of our intel_crtc->config pointer */
> @@ -16170,7 +16270,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>         else
>                 dev_priv->display.commit_modeset_enables = intel_commit_modeset_enables;
>
> -       dev_priv->display.commit_modeset_disables = intel_commit_modeset_disables;
> +       if (INTEL_GEN(dev_priv) >= 11)
> +               dev_priv->display.commit_modeset_disables =
> +                       icl_commit_modeset_disables;
> +       else
> +               dev_priv->display.commit_modeset_disables =
> +                       intel_commit_modeset_disables;
>
>  }
>
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Navare, Manasi June 25, 2019, 7:02 p.m. UTC | #2
On Mon, Jun 24, 2019 at 11:34:42PM -0700, Lucas De Marchi wrote:
> On Mon, Jun 24, 2019 at 2:07 PM Manasi Navare <manasi.d.navare@intel.com> wrote:
> >
> > In the transcoder port sync mode, the slave transcoders mask their vblanks
> > until master transcoder's vblank so while disabling them, make
> > sure slaves are disabled first and then the masters.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 117 ++++++++++++++++++-
> >  1 file changed, 111 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 0a0d97ef03d6..85746a26d0e0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13901,6 +13901,106 @@ static void intel_commit_modeset_disables(struct drm_atomic_state *state)
> >         }
> >  }
> >
> > +static void icl_commit_modeset_disables(struct drm_atomic_state *state)
> > +{
> > +       struct drm_device *dev = state->dev;
> > +       struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> > +       struct drm_i915_private *dev_priv = to_i915(dev);
> > +       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > +       struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
> > +       struct drm_crtc *crtc;
> > +       struct intel_crtc *intel_crtc;
> > +       int i;
> > +
> > +       /*
> > +        * Disable all the Port Sync Slaves first
> > +        */
> > +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> > +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> > +               intel_crtc = to_intel_crtc(crtc);
> > +
> > +               if (!needs_modeset(new_crtc_state) ||
> > +                   !is_trans_port_sync_slave(old_intel_crtc_state))
> > +                       continue;
> > +
> > +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> > +
> > +               if (old_crtc_state->active) {
> > +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> > +
> > +                       /*
> > +                        * We need to disable pipe CRC before disabling the pipe,
> > +                        * or we race against vblank off.
> > +                        */
> > +                       intel_crtc_disable_pipe_crc(intel_crtc);
> > +
> > +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> > +                       intel_crtc->active = false;
> > +                       intel_fbc_disable(intel_crtc);
> > +                       intel_disable_shared_dpll(old_intel_crtc_state);
> > +
> > +                       /*
> > +                        * Underruns don't always raise interrupts,
> > +                        * so check manually.
> > +                        */
> > +                       intel_check_cpu_fifo_underruns(dev_priv);
> > +                       intel_check_pch_fifo_underruns(dev_priv);
> > +
> > +                       /* FIXME unify this for all platforms */
> > +                       if (!new_crtc_state->active &&
> > +                           !HAS_GMCH(dev_priv) &&
> > +                           dev_priv->display.initial_watermarks)
> > +                               dev_priv->display.initial_watermarks(intel_state,
> > +                                                                    new_intel_crtc_state);
> > +               }
> > +       }
> > +
> > +       /*
> > +        * Disable rest of the CRTCs other than slaves
> > +        */
> > +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> > +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> > +               intel_crtc = to_intel_crtc(crtc);
> > +
> > +               if (!needs_modeset(new_crtc_state) ||
> > +                   is_trans_port_sync_slave(old_intel_crtc_state))
> > +                       continue;
> > +
> > +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> > +
> > +               if (old_crtc_state->active) {
> > +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> > +
> > +                       /*
> > +                        * We need to disable pipe CRC before disabling the pipe,
> > +                        * or we race against vblank off.
> > +                        */
> > +                       intel_crtc_disable_pipe_crc(intel_crtc);
> > +
> > +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> > +                       intel_crtc->active = false;
> > +                       intel_fbc_disable(intel_crtc);
> > +                       intel_disable_shared_dpll(old_intel_crtc_state);
> > +
> > +                       /*
> > +                        * Underruns don't always raise interrupts,
> > +                        * so check manually.
> > +                        */
> > +                       intel_check_cpu_fifo_underruns(dev_priv);
> > +                       intel_check_pch_fifo_underruns(dev_priv);
> > +
> > +                       /* FIXME unify this for all platforms */
> > +                       if (!new_crtc_state->active &&
> > +                           !HAS_GMCH(dev_priv) &&
> > +                           dev_priv->display.initial_watermarks)
> > +                               dev_priv->display.initial_watermarks(intel_state,
> > +                                                                    new_intel_crtc_state);
> > +               }
> > +       }
> 
> Not a fan of this duplicate code here. At least a helper function
> would be needed.

Hmm yea creating a helper is a good idea, will do that

> 
> Jose, didn't you have a pending patch to disable in reverse order to
> solve this same problem?

This is not necessarily in the reverse order, it just does it for all slaves
first and then all masters

Manasi

> 
> Lucas De Marchi
> 
> > +}
> > +
> >  static void intel_commit_modeset_enables(struct drm_atomic_state *state)
> >  {
> >         struct drm_crtc *crtc;
> > @@ -14257,6 +14357,11 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >
> >         intel_atomic_commit_fence_wait(intel_state);
> >
> > +       drm_atomic_helper_wait_for_dependencies(state);
> > +
> > +       if (intel_state->modeset)
> > +               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> > +
> >         for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> >                 new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> >                 intel_crtc = to_intel_crtc(crtc);
> > @@ -14269,11 +14374,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >                 }
> >         }
> >
> > -       drm_atomic_helper_wait_for_dependencies(state);
> > -
> > -       if (intel_state->modeset)
> > -               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> > -
> >         dev_priv->display.commit_modeset_disables(state);
> >
> >         /* FIXME: Eventually get rid of our intel_crtc->config pointer */
> > @@ -16170,7 +16270,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> >         else
> >                 dev_priv->display.commit_modeset_enables = intel_commit_modeset_enables;
> >
> > -       dev_priv->display.commit_modeset_disables = intel_commit_modeset_disables;
> > +       if (INTEL_GEN(dev_priv) >= 11)
> > +               dev_priv->display.commit_modeset_disables =
> > +                       icl_commit_modeset_disables;
> > +       else
> > +               dev_priv->display.commit_modeset_disables =
> > +                       intel_commit_modeset_disables;
> >
> >  }
> >
> > --
> > 2.19.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi
Navare, Manasi June 27, 2019, 8:01 p.m. UTC | #3
On Mon, Jun 24, 2019 at 11:34:42PM -0700, Lucas De Marchi wrote:
> On Mon, Jun 24, 2019 at 2:07 PM Manasi Navare <manasi.d.navare@intel.com> wrote:
> >
> > In the transcoder port sync mode, the slave transcoders mask their vblanks
> > until master transcoder's vblank so while disabling them, make
> > sure slaves are disabled first and then the masters.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 117 ++++++++++++++++++-
> >  1 file changed, 111 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 0a0d97ef03d6..85746a26d0e0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13901,6 +13901,106 @@ static void intel_commit_modeset_disables(struct drm_atomic_state *state)
> >         }
> >  }
> >
> > +static void icl_commit_modeset_disables(struct drm_atomic_state *state)
> > +{
> > +       struct drm_device *dev = state->dev;
> > +       struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> > +       struct drm_i915_private *dev_priv = to_i915(dev);
> > +       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > +       struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
> > +       struct drm_crtc *crtc;
> > +       struct intel_crtc *intel_crtc;
> > +       int i;
> > +
> > +       /*
> > +        * Disable all the Port Sync Slaves first
> > +        */
> > +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> > +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> > +               intel_crtc = to_intel_crtc(crtc);
> > +
> > +               if (!needs_modeset(new_crtc_state) ||
> > +                   !is_trans_port_sync_slave(old_intel_crtc_state))
> > +                       continue;
> > +
> > +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> > +
> > +               if (old_crtc_state->active) {
> > +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> > +
> > +                       /*
> > +                        * We need to disable pipe CRC before disabling the pipe,
> > +                        * or we race against vblank off.
> > +                        */
> > +                       intel_crtc_disable_pipe_crc(intel_crtc);
> > +
> > +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> > +                       intel_crtc->active = false;
> > +                       intel_fbc_disable(intel_crtc);
> > +                       intel_disable_shared_dpll(old_intel_crtc_state);
> > +
> > +                       /*
> > +                        * Underruns don't always raise interrupts,
> > +                        * so check manually.
> > +                        */
> > +                       intel_check_cpu_fifo_underruns(dev_priv);
> > +                       intel_check_pch_fifo_underruns(dev_priv);
> > +
> > +                       /* FIXME unify this for all platforms */
> > +                       if (!new_crtc_state->active &&
> > +                           !HAS_GMCH(dev_priv) &&
> > +                           dev_priv->display.initial_watermarks)
> > +                               dev_priv->display.initial_watermarks(intel_state,
> > +                                                                    new_intel_crtc_state);
> > +               }

I will move this part inside if (old_crtc_state->active) to a helper function.
Any suggestions on the name of this helper?

Manasi

> > +       }
> > +
> > +       /*
> > +        * Disable rest of the CRTCs other than slaves
> > +        */
> > +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > +               old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
> > +               new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> > +               intel_crtc = to_intel_crtc(crtc);
> > +
> > +               if (!needs_modeset(new_crtc_state) ||
> > +                   is_trans_port_sync_slave(old_intel_crtc_state))
> > +                       continue;
> > +
> > +               intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
> > +
> > +               if (old_crtc_state->active) {
> > +                       intel_crtc_disable_planes(intel_state, intel_crtc);
> > +
> > +                       /*
> > +                        * We need to disable pipe CRC before disabling the pipe,
> > +                        * or we race against vblank off.
> > +                        */
> > +                       intel_crtc_disable_pipe_crc(intel_crtc);
> > +
> > +                       dev_priv->display.crtc_disable(old_intel_crtc_state, state);
> > +                       intel_crtc->active = false;
> > +                       intel_fbc_disable(intel_crtc);
> > +                       intel_disable_shared_dpll(old_intel_crtc_state);
> > +
> > +                       /*
> > +                        * Underruns don't always raise interrupts,
> > +                        * so check manually.
> > +                        */
> > +                       intel_check_cpu_fifo_underruns(dev_priv);
> > +                       intel_check_pch_fifo_underruns(dev_priv);
> > +
> > +                       /* FIXME unify this for all platforms */
> > +                       if (!new_crtc_state->active &&
> > +                           !HAS_GMCH(dev_priv) &&
> > +                           dev_priv->display.initial_watermarks)
> > +                               dev_priv->display.initial_watermarks(intel_state,
> > +                                                                    new_intel_crtc_state);
> > +               }
> > +       }
> 
> Not a fan of this duplicate code here. At least a helper function
> would be needed.
> 
> Jose, didn't you have a pending patch to disable in reverse order to
> solve this same problem?
> 
> Lucas De Marchi
> 
> > +}
> > +
> >  static void intel_commit_modeset_enables(struct drm_atomic_state *state)
> >  {
> >         struct drm_crtc *crtc;
> > @@ -14257,6 +14357,11 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >
> >         intel_atomic_commit_fence_wait(intel_state);
> >
> > +       drm_atomic_helper_wait_for_dependencies(state);
> > +
> > +       if (intel_state->modeset)
> > +               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> > +
> >         for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> >                 new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
> >                 intel_crtc = to_intel_crtc(crtc);
> > @@ -14269,11 +14374,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >                 }
> >         }
> >
> > -       drm_atomic_helper_wait_for_dependencies(state);
> > -
> > -       if (intel_state->modeset)
> > -               wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> > -
> >         dev_priv->display.commit_modeset_disables(state);
> >
> >         /* FIXME: Eventually get rid of our intel_crtc->config pointer */
> > @@ -16170,7 +16270,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> >         else
> >                 dev_priv->display.commit_modeset_enables = intel_commit_modeset_enables;
> >
> > -       dev_priv->display.commit_modeset_disables = intel_commit_modeset_disables;
> > +       if (INTEL_GEN(dev_priv) >= 11)
> > +               dev_priv->display.commit_modeset_disables =
> > +                       icl_commit_modeset_disables;
> > +       else
> > +               dev_priv->display.commit_modeset_disables =
> > +                       intel_commit_modeset_disables;
> >
> >  }
> >
> > --
> > 2.19.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 0a0d97ef03d6..85746a26d0e0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13901,6 +13901,106 @@  static void intel_commit_modeset_disables(struct drm_atomic_state *state)
 	}
 }
 
+static void icl_commit_modeset_disables(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
+	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
+	int i;
+
+	/*
+	 * Disable all the Port Sync Slaves first
+	 */
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
+		new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
+		intel_crtc = to_intel_crtc(crtc);
+
+		if (!needs_modeset(new_crtc_state) ||
+		    !is_trans_port_sync_slave(old_intel_crtc_state))
+			continue;
+
+		intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
+
+		if (old_crtc_state->active) {
+			intel_crtc_disable_planes(intel_state, intel_crtc);
+
+			/*
+			 * We need to disable pipe CRC before disabling the pipe,
+			 * or we race against vblank off.
+			 */
+			intel_crtc_disable_pipe_crc(intel_crtc);
+
+			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
+			intel_crtc->active = false;
+			intel_fbc_disable(intel_crtc);
+			intel_disable_shared_dpll(old_intel_crtc_state);
+
+			/*
+			 * Underruns don't always raise interrupts,
+			 * so check manually.
+			 */
+			intel_check_cpu_fifo_underruns(dev_priv);
+			intel_check_pch_fifo_underruns(dev_priv);
+
+			/* FIXME unify this for all platforms */
+			if (!new_crtc_state->active &&
+			    !HAS_GMCH(dev_priv) &&
+			    dev_priv->display.initial_watermarks)
+				dev_priv->display.initial_watermarks(intel_state,
+								     new_intel_crtc_state);
+		}
+	}
+
+	/*
+	 * Disable rest of the CRTCs other than slaves
+	 */
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		old_intel_crtc_state = to_intel_crtc_state(old_crtc_state);
+		new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
+		intel_crtc = to_intel_crtc(crtc);
+
+		if (!needs_modeset(new_crtc_state) ||
+		    is_trans_port_sync_slave(old_intel_crtc_state))
+			continue;
+
+		intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
+
+		if (old_crtc_state->active) {
+			intel_crtc_disable_planes(intel_state, intel_crtc);
+
+			/*
+			 * We need to disable pipe CRC before disabling the pipe,
+			 * or we race against vblank off.
+			 */
+			intel_crtc_disable_pipe_crc(intel_crtc);
+
+			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
+			intel_crtc->active = false;
+			intel_fbc_disable(intel_crtc);
+			intel_disable_shared_dpll(old_intel_crtc_state);
+
+			/*
+			 * Underruns don't always raise interrupts,
+			 * so check manually.
+			 */
+			intel_check_cpu_fifo_underruns(dev_priv);
+			intel_check_pch_fifo_underruns(dev_priv);
+
+			/* FIXME unify this for all platforms */
+			if (!new_crtc_state->active &&
+			    !HAS_GMCH(dev_priv) &&
+			    dev_priv->display.initial_watermarks)
+				dev_priv->display.initial_watermarks(intel_state,
+								     new_intel_crtc_state);
+		}
+	}
+}
+
 static void intel_commit_modeset_enables(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
@@ -14257,6 +14357,11 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 
 	intel_atomic_commit_fence_wait(intel_state);
 
+	drm_atomic_helper_wait_for_dependencies(state);
+
+	if (intel_state->modeset)
+		wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
+
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
 		intel_crtc = to_intel_crtc(crtc);
@@ -14269,11 +14374,6 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		}
 	}
 
-	drm_atomic_helper_wait_for_dependencies(state);
-
-	if (intel_state->modeset)
-		wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
-
 	dev_priv->display.commit_modeset_disables(state);
 
 	/* FIXME: Eventually get rid of our intel_crtc->config pointer */
@@ -16170,7 +16270,12 @@  void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	else
 		dev_priv->display.commit_modeset_enables = intel_commit_modeset_enables;
 
-	dev_priv->display.commit_modeset_disables = intel_commit_modeset_disables;
+	if (INTEL_GEN(dev_priv) >= 11)
+		dev_priv->display.commit_modeset_disables =
+			icl_commit_modeset_disables;
+	else
+		dev_priv->display.commit_modeset_disables =
+			intel_commit_modeset_disables;
 
 }