Thanks for the review pushed to dinq Manasi
On Wed, Aug 28, 2019 at 03:47:01PM -0700, Manasi Navare wrote: > Create a new function intel_commit_modeset_disables() consistent with the > naming > in drm atomic helpers and similar to the enable function. > This helps better organize the disable sequence in atomic_commit_tail() > > No functional change > > v4: > * Do not create a function pointer, just a function (Maarten) > v3: > * Rebase (Manasi) > v2: > * Create a helper for old_crtc_state disables (Lucas) > > Suggested-by: Daniel Vetter <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Cc: Maarten Lankhorst <[email protected]> > Cc: Matt Roper <[email protected]> > Cc: Jani Nikula <[email protected]> > Signed-off-by: Manasi Navare <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_display.c | 92 ++++++++++++-------- > drivers/gpu/drm/i915/i915_drv.h | 1 + > 2 files changed, 59 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 77dfb0f6a6e0..5c53b45dc06e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -13758,6 +13758,61 @@ static void intel_update_crtc(struct intel_crtc > *crtc, > intel_finish_crtc_commit(state, crtc); > } > > +static void intel_old_crtc_state_disables(struct intel_atomic_state *state, > + struct intel_crtc_state > *old_crtc_state, > + struct intel_crtc_state > *new_crtc_state, > + struct intel_crtc *crtc) > +{ > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + > + intel_crtc_disable_planes(state, crtc); > + > + /* > + * We need to disable pipe CRC before disabling the pipe, > + * or we race against vblank off. > + */ > + intel_crtc_disable_pipe_crc(crtc); > + > + dev_priv->display.crtc_disable(old_crtc_state, state); > + crtc->active = false; > + intel_fbc_disable(crtc); > + intel_disable_shared_dpll(old_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->base.active && > + !HAS_GMCH(dev_priv) && > + dev_priv->display.initial_watermarks) > + dev_priv->display.initial_watermarks(state, > + new_crtc_state); > +} > + > +static void intel_commit_modeset_disables(struct intel_atomic_state *state) > +{ > + struct intel_crtc_state *new_crtc_state, *old_crtc_state; > + struct intel_crtc *crtc; > + int i; > + > + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > + if (!needs_modeset(new_crtc_state)) > + continue; > + > + intel_pre_plane_update(old_crtc_state, new_crtc_state); > + > + if (old_crtc_state->base.active) > + intel_old_crtc_state_disables(state, > + old_crtc_state, > + new_crtc_state, > + crtc); > + } > +} > + > static void intel_commit_modeset_enables(struct intel_atomic_state *state) > { > struct intel_crtc *crtc; > @@ -13938,42 +13993,10 @@ static void intel_atomic_commit_tail(struct > intel_atomic_state *state) > put_domains[crtc->pipe] = > modeset_get_crtc_power_domains(new_crtc_state); > } > - > - if (!needs_modeset(new_crtc_state)) > - continue; > - > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > - > - if (old_crtc_state->base.active) { > - intel_crtc_disable_planes(state, crtc); > - > - /* > - * We need to disable pipe CRC before disabling the > pipe, > - * or we race against vblank off. > - */ > - intel_crtc_disable_pipe_crc(crtc); > - > - dev_priv->display.crtc_disable(old_crtc_state, state); > - crtc->active = false; > - intel_fbc_disable(crtc); > - intel_disable_shared_dpll(old_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->base.active && > - !HAS_GMCH(dev_priv) && > - dev_priv->display.initial_watermarks) > - dev_priv->display.initial_watermarks(state, > - > new_crtc_state); > - } > } > > + intel_commit_modeset_disables(state); > + > /* FIXME: Eventually get rid of our crtc->config pointer */ > for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) > crtc->config = new_crtc_state; > @@ -15929,6 +15952,7 @@ void intel_init_display_hooks(struct drm_i915_private > *dev_priv) > dev_priv->display.commit_modeset_enables = > skl_commit_modeset_enables; > else > dev_priv->display.commit_modeset_enables = > intel_commit_modeset_enables; > + > } > > static i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv) > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 75a42e8df67e..db7480831e52 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -289,6 +289,7 @@ struct drm_i915_display_funcs { > void (*crtc_disable)(struct intel_crtc_state *old_crtc_state, > struct intel_atomic_state *old_state); > void (*commit_modeset_enables)(struct intel_atomic_state *state); > + void (*commit_modeset_disables)(struct intel_atomic_state *state); > void (*audio_codec_enable)(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > const struct drm_connector_state > *conn_state); > -- > 2.19.1 > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
