| Message ID | 20191127190556.1574-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Cleanups around pre/post plane update | expand |
On Wed, 2019-11-27 at 21:05 +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Change the calling convention to just pass the state+crtc and > switch to intel_ types throughout. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++--------- > -- > drivers/gpu/drm/i915/display/intel_fbc.c | 14 ++-- > drivers/gpu/drm/i915/display/intel_fbc.h | 8 +- > 3 files changed, 51 insertions(+), 59 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index e341b97b7dec..72655b5b1365 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5920,13 +5920,10 @@ static void > intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) > * completely hide the primary plane. > */ > static void > -intel_post_enable_primary(struct drm_crtc *crtc, > - const struct intel_crtc_state > *new_crtc_state) > +intel_post_enable_primary(struct intel_crtc *crtc) > { > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + enum pipe pipe = crtc->pipe; > > /* > * Gen2 reports pipe underruns whenever all planes are > disabled. > @@ -6062,20 +6059,21 @@ static bool needs_scalerclk_wa(const struct > intel_crtc_state *crtc_state) > return false; > } > > -static void intel_post_plane_update(struct intel_crtc_state > *old_crtc_state) > +static void intel_post_plane_update(struct intel_atomic_state > *state, > + struct intel_crtc *crtc) > { > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > >uapi.crtc); > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > - struct intel_crtc_state *new_crtc_state = > - intel_atomic_get_new_crtc_state(to_intel_atomic_state(s > tate), > - crtc); > - struct drm_plane *primary = crtc->base.primary; > - struct drm_plane_state *old_primary_state = > - drm_atomic_get_old_plane_state(state, primary); > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + struct intel_plane *primary = to_intel_plane(crtc- > >base.primary); > + const struct intel_crtc_state *old_crtc_state = > + intel_atomic_get_old_crtc_state(state, crtc); > + const struct intel_crtc_state *new_crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + const struct intel_plane_state *old_primary_state = > + intel_atomic_get_old_plane_state(state, primary); > + const struct intel_plane_state *new_primary_state = > + intel_atomic_get_new_plane_state(state, primary); > > - intel_frontbuffer_flip(to_i915(crtc->base.dev), new_crtc_state- > >fb_bits); > + intel_frontbuffer_flip(dev_priv, new_crtc_state->fb_bits); > > if (new_crtc_state->update_wm_post && new_crtc_state- > >hw.active) > intel_update_watermarks(crtc); > @@ -6083,16 +6081,13 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > if (hsw_post_update_enable_ips(old_crtc_state, new_crtc_state)) > hsw_enable_ips(new_crtc_state); > > - if (old_primary_state) { > - struct drm_plane_state *new_primary_state = > - drm_atomic_get_new_plane_state(state, primary); > - > + if (new_primary_state) { This change from old_primary_state to new_primary_state is way more than the commit message says, the change looks right to me but maybe it deserves a separated patch? Same for the same change in intel_pre_plane_update() > intel_fbc_post_update(crtc); > > - if (new_primary_state->visible && > + if (new_primary_state->uapi.visible && > (needs_modeset(new_crtc_state) || > - !old_primary_state->visible)) > - intel_post_enable_primary(&crtc->base, > new_crtc_state); > + !old_primary_state->uapi.visible)) > + intel_post_enable_primary(crtc); > } > > if (needs_nv12_wa(old_crtc_state) && > @@ -6104,34 +6099,31 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > icl_wa_scalerclkgating(dev_priv, crtc->pipe, false); > } > > -static void intel_pre_plane_update(struct intel_crtc_state > *old_crtc_state, > - struct intel_crtc_state > *new_crtc_state) > +static void intel_pre_plane_update(struct intel_atomic_state *state, > + struct intel_crtc *crtc) > { > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > >uapi.crtc); > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > - struct drm_plane *primary = crtc->base.primary; > - struct drm_plane_state *old_primary_state = > - drm_atomic_get_old_plane_state(state, primary); > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + struct intel_plane *primary = to_intel_plane(crtc- > >base.primary); > + const struct intel_crtc_state *old_crtc_state = > + intel_atomic_get_old_crtc_state(state, crtc); > + const struct intel_crtc_state *new_crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + const struct intel_plane_state *old_primary_state = > + intel_atomic_get_old_plane_state(state, primary); > + const struct intel_plane_state *new_primary_state = > + intel_atomic_get_new_plane_state(state, primary); > bool modeset = needs_modeset(new_crtc_state); > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(state); > > if (hsw_pre_update_disable_ips(old_crtc_state, new_crtc_state)) > hsw_disable_ips(old_crtc_state); > > - if (old_primary_state) { > - struct intel_plane_state *new_primary_state = > - intel_atomic_get_new_plane_state(intel_state, > - to_intel_plane > (primary)); > - > + if (new_primary_state) { > intel_fbc_pre_update(crtc, new_crtc_state, > new_primary_state); > /* > * Gen2 reports pipe underruns whenever all planes are > disabled. > * So disable underrun reporting before all the planes > get disabled. > */ > - if (IS_GEN(dev_priv, 2) && old_primary_state->visible > && > + if (IS_GEN(dev_priv, 2) && old_primary_state- > >uapi.visible && > (modeset || !new_primary_state->uapi.visible)) > intel_set_cpu_fifo_underrun_reporting(dev_priv, > crtc->pipe, false); > } > @@ -6192,7 +6184,7 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > * us to. > */ > if (dev_priv->display.initial_watermarks) > - dev_priv->display.initial_watermarks(intel_state, > crtc); > + dev_priv->display.initial_watermarks(state, crtc); > else if (new_crtc_state->update_wm_pre) > intel_update_watermarks(crtc); > } > @@ -14365,7 +14357,7 @@ static void intel_update_crtc(struct > intel_crtc *crtc, > new_crtc_state->update_pipe)) > intel_color_load_luts(new_crtc_state); > > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > + intel_pre_plane_update(state, crtc); > > if (new_crtc_state->update_pipe) > intel_encoders_update_pipe(state, crtc); > @@ -14460,7 +14452,7 @@ static void > intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st > !old_slave_crtc_state); > > /* Disable Slave first */ > - intel_pre_plane_update(old_slave_crtc_state, > new_slave_crtc_state); > + intel_pre_plane_update(state, slave_crtc); > if (old_slave_crtc_state->hw.active) > intel_old_crtc_state_disables(state, > old_slave_crtc_state, > @@ -14468,7 +14460,7 @@ static void > intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st > slave_crtc); > > /* Disable Master */ > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > + intel_pre_plane_update(state, crtc); > if (old_crtc_state->hw.active) > intel_old_crtc_state_disables(state, > old_crtc_state, > @@ -14508,7 +14500,7 @@ static void > intel_commit_modeset_disables(struct intel_atomic_state *state) > else > continue; > } else { > - intel_pre_plane_update(old_crtc_state, > new_crtc_state); > + intel_pre_plane_update(state, crtc); > > if (old_crtc_state->hw.active) > intel_old_crtc_state_disables(state, > @@ -14896,7 +14888,7 @@ static void intel_atomic_commit_tail(struct > intel_atomic_state *state) > } > > for_each_oldnew_intel_crtc_in_state(state, crtc, > old_crtc_state, new_crtc_state, i) { > - intel_post_plane_update(old_crtc_state); > + intel_post_plane_update(state, crtc); > > if (put_domains[i]) > modeset_put_power_domains(dev_priv, > put_domains[i]); > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > b/drivers/gpu/drm/i915/display/intel_fbc.c > index 92c7eb243559..70c56abe1a83 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -420,7 +420,7 @@ static void intel_fbc_deactivate(struct > drm_i915_private *dev_priv, > } > > static bool multiple_pipes_ok(struct intel_crtc *crtc, > - struct intel_plane_state *plane_state) > + const struct intel_plane_state > *plane_state) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_fbc *fbc = &dev_priv->fbc; > @@ -656,8 +656,8 @@ static bool > intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) > } > > static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > - struct intel_crtc_state > *crtc_state, > - struct intel_plane_state > *plane_state) > + const struct intel_crtc_state > *crtc_state, > + const struct intel_plane_state > *plane_state) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_fbc *fbc = &dev_priv->fbc; > @@ -855,8 +855,8 @@ static void intel_fbc_get_reg_params(struct > intel_crtc *crtc, > } > > void intel_fbc_pre_update(struct intel_crtc *crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_plane_state *plane_state) > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) Nice Other than the first comment, LGTM. > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_fbc *fbc = &dev_priv->fbc; > @@ -1081,8 +1081,8 @@ void intel_fbc_choose_crtc(struct > drm_i915_private *dev_priv, > * intel_fbc_disable in the middle, as long as it is deactivated. > */ > void intel_fbc_enable(struct intel_crtc *crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_plane_state *plane_state) > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > struct intel_fbc *fbc = &dev_priv->fbc; > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h > b/drivers/gpu/drm/i915/display/intel_fbc.h > index 50272eda8d43..f58b0b1e3c5c 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.h > +++ b/drivers/gpu/drm/i915/display/intel_fbc.h > @@ -20,14 +20,14 @@ void intel_fbc_choose_crtc(struct > drm_i915_private *dev_priv, > struct intel_atomic_state *state); > bool intel_fbc_is_active(struct drm_i915_private *dev_priv); > void intel_fbc_pre_update(struct intel_crtc *crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_plane_state *plane_state); > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > void intel_fbc_post_update(struct intel_crtc *crtc); > void intel_fbc_init(struct drm_i915_private *dev_priv); > void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv); > void intel_fbc_enable(struct intel_crtc *crtc, > - struct intel_crtc_state *crtc_state, > - struct intel_plane_state *plane_state); > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > void intel_fbc_disable(struct intel_crtc *crtc); > void intel_fbc_global_disable(struct drm_i915_private *dev_priv); > void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
On Wed, Nov 27, 2019 at 11:25:07PM +0000, Souza, Jose wrote: > On Wed, 2019-11-27 at 21:05 +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Change the calling convention to just pass the state+crtc and > > switch to intel_ types throughout. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++--------- > > -- > > drivers/gpu/drm/i915/display/intel_fbc.c | 14 ++-- > > drivers/gpu/drm/i915/display/intel_fbc.h | 8 +- > > 3 files changed, 51 insertions(+), 59 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index e341b97b7dec..72655b5b1365 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5920,13 +5920,10 @@ static void > > intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) > > * completely hide the primary plane. > > */ > > static void > > -intel_post_enable_primary(struct drm_crtc *crtc, > > - const struct intel_crtc_state > > *new_crtc_state) > > +intel_post_enable_primary(struct intel_crtc *crtc) > > { > > - struct drm_device *dev = crtc->dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - enum pipe pipe = intel_crtc->pipe; > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + enum pipe pipe = crtc->pipe; > > > > /* > > * Gen2 reports pipe underruns whenever all planes are > > disabled. > > @@ -6062,20 +6059,21 @@ static bool needs_scalerclk_wa(const struct > > intel_crtc_state *crtc_state) > > return false; > > } > > > > -static void intel_post_plane_update(struct intel_crtc_state > > *old_crtc_state) > > +static void intel_post_plane_update(struct intel_atomic_state > > *state, > > + struct intel_crtc *crtc) > > { > > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > >uapi.crtc); > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > > - struct intel_crtc_state *new_crtc_state = > > - intel_atomic_get_new_crtc_state(to_intel_atomic_state(s > > tate), > > - crtc); > > - struct drm_plane *primary = crtc->base.primary; > > - struct drm_plane_state *old_primary_state = > > - drm_atomic_get_old_plane_state(state, primary); > > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > > + struct intel_plane *primary = to_intel_plane(crtc- > > >base.primary); > > + const struct intel_crtc_state *old_crtc_state = > > + intel_atomic_get_old_crtc_state(state, crtc); > > + const struct intel_crtc_state *new_crtc_state = > > + intel_atomic_get_new_crtc_state(state, crtc); > > + const struct intel_plane_state *old_primary_state = > > + intel_atomic_get_old_plane_state(state, primary); > > + const struct intel_plane_state *new_primary_state = > > + intel_atomic_get_new_plane_state(state, primary); > > > > - intel_frontbuffer_flip(to_i915(crtc->base.dev), new_crtc_state- > > >fb_bits); > > + intel_frontbuffer_flip(dev_priv, new_crtc_state->fb_bits); > > > > if (new_crtc_state->update_wm_post && new_crtc_state- > > >hw.active) > > intel_update_watermarks(crtc); > > @@ -6083,16 +6081,13 @@ static void intel_post_plane_update(struct > > intel_crtc_state *old_crtc_state) > > if (hsw_post_update_enable_ips(old_crtc_state, new_crtc_state)) > > hsw_enable_ips(new_crtc_state); > > > > - if (old_primary_state) { > > - struct drm_plane_state *new_primary_state = > > - drm_atomic_get_new_plane_state(state, primary); > > - > > + if (new_primary_state) { > > > This change from old_primary_state to new_primary_state is way more > than the commit message says, the change looks right to me but maybe it > deserves a separated patch? Same for the same change in > intel_pre_plane_update() I wanted to change it so I can eliminate old_primary_state in a subsequent patch. For whatever reason that change slipped into this patch. It's a nop change though since !new_state == !old_state always. > > > intel_fbc_post_update(crtc); > > > > - if (new_primary_state->visible && > > + if (new_primary_state->uapi.visible && > > (needs_modeset(new_crtc_state) || > > - !old_primary_state->visible)) > > - intel_post_enable_primary(&crtc->base, > > new_crtc_state); > > + !old_primary_state->uapi.visible)) > > + intel_post_enable_primary(crtc); > > } > > > > if (needs_nv12_wa(old_crtc_state) && > > @@ -6104,34 +6099,31 @@ static void intel_post_plane_update(struct > > intel_crtc_state *old_crtc_state) > > icl_wa_scalerclkgating(dev_priv, crtc->pipe, false); > > } > > > > -static void intel_pre_plane_update(struct intel_crtc_state > > *old_crtc_state, > > - struct intel_crtc_state > > *new_crtc_state) > > +static void intel_pre_plane_update(struct intel_atomic_state *state, > > + struct intel_crtc *crtc) > > { > > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > >uapi.crtc); > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > > - struct drm_plane *primary = crtc->base.primary; > > - struct drm_plane_state *old_primary_state = > > - drm_atomic_get_old_plane_state(state, primary); > > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > > + struct intel_plane *primary = to_intel_plane(crtc- > > >base.primary); > > + const struct intel_crtc_state *old_crtc_state = > > + intel_atomic_get_old_crtc_state(state, crtc); > > + const struct intel_crtc_state *new_crtc_state = > > + intel_atomic_get_new_crtc_state(state, crtc); > > + const struct intel_plane_state *old_primary_state = > > + intel_atomic_get_old_plane_state(state, primary); > > + const struct intel_plane_state *new_primary_state = > > + intel_atomic_get_new_plane_state(state, primary); > > bool modeset = needs_modeset(new_crtc_state); > > - struct intel_atomic_state *intel_state = > > - to_intel_atomic_state(state); > > > > if (hsw_pre_update_disable_ips(old_crtc_state, new_crtc_state)) > > hsw_disable_ips(old_crtc_state); > > > > - if (old_primary_state) { > > - struct intel_plane_state *new_primary_state = > > - intel_atomic_get_new_plane_state(intel_state, > > - to_intel_plane > > (primary)); > > - > > + if (new_primary_state) { > > intel_fbc_pre_update(crtc, new_crtc_state, > > new_primary_state); > > /* > > * Gen2 reports pipe underruns whenever all planes are > > disabled. > > * So disable underrun reporting before all the planes > > get disabled. > > */ > > - if (IS_GEN(dev_priv, 2) && old_primary_state->visible > > && > > + if (IS_GEN(dev_priv, 2) && old_primary_state- > > >uapi.visible && > > (modeset || !new_primary_state->uapi.visible)) > > intel_set_cpu_fifo_underrun_reporting(dev_priv, > > crtc->pipe, false); > > } > > @@ -6192,7 +6184,7 @@ static void intel_pre_plane_update(struct > > intel_crtc_state *old_crtc_state, > > * us to. > > */ > > if (dev_priv->display.initial_watermarks) > > - dev_priv->display.initial_watermarks(intel_state, > > crtc); > > + dev_priv->display.initial_watermarks(state, crtc); > > else if (new_crtc_state->update_wm_pre) > > intel_update_watermarks(crtc); > > } > > @@ -14365,7 +14357,7 @@ static void intel_update_crtc(struct > > intel_crtc *crtc, > > new_crtc_state->update_pipe)) > > intel_color_load_luts(new_crtc_state); > > > > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > > + intel_pre_plane_update(state, crtc); > > > > if (new_crtc_state->update_pipe) > > intel_encoders_update_pipe(state, crtc); > > @@ -14460,7 +14452,7 @@ static void > > intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st > > !old_slave_crtc_state); > > > > /* Disable Slave first */ > > - intel_pre_plane_update(old_slave_crtc_state, > > new_slave_crtc_state); > > + intel_pre_plane_update(state, slave_crtc); > > if (old_slave_crtc_state->hw.active) > > intel_old_crtc_state_disables(state, > > old_slave_crtc_state, > > @@ -14468,7 +14460,7 @@ static void > > intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st > > slave_crtc); > > > > /* Disable Master */ > > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > > + intel_pre_plane_update(state, crtc); > > if (old_crtc_state->hw.active) > > intel_old_crtc_state_disables(state, > > old_crtc_state, > > @@ -14508,7 +14500,7 @@ static void > > intel_commit_modeset_disables(struct intel_atomic_state *state) > > else > > continue; > > } else { > > - intel_pre_plane_update(old_crtc_state, > > new_crtc_state); > > + intel_pre_plane_update(state, crtc); > > > > if (old_crtc_state->hw.active) > > intel_old_crtc_state_disables(state, > > @@ -14896,7 +14888,7 @@ static void intel_atomic_commit_tail(struct > > intel_atomic_state *state) > > } > > > > for_each_oldnew_intel_crtc_in_state(state, crtc, > > old_crtc_state, new_crtc_state, i) { > > - intel_post_plane_update(old_crtc_state); > > + intel_post_plane_update(state, crtc); > > > > if (put_domains[i]) > > modeset_put_power_domains(dev_priv, > > put_domains[i]); > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index 92c7eb243559..70c56abe1a83 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -420,7 +420,7 @@ static void intel_fbc_deactivate(struct > > drm_i915_private *dev_priv, > > } > > > > static bool multiple_pipes_ok(struct intel_crtc *crtc, > > - struct intel_plane_state *plane_state) > > + const struct intel_plane_state > > *plane_state) > > { > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > struct intel_fbc *fbc = &dev_priv->fbc; > > @@ -656,8 +656,8 @@ static bool > > intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) > > } > > > > static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > > - struct intel_crtc_state > > *crtc_state, > > - struct intel_plane_state > > *plane_state) > > + const struct intel_crtc_state > > *crtc_state, > > + const struct intel_plane_state > > *plane_state) > > { > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > struct intel_fbc *fbc = &dev_priv->fbc; > > @@ -855,8 +855,8 @@ static void intel_fbc_get_reg_params(struct > > intel_crtc *crtc, > > } > > > > void intel_fbc_pre_update(struct intel_crtc *crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_plane_state *plane_state) > > + const struct intel_crtc_state *crtc_state, > > + const struct intel_plane_state *plane_state) > > Nice > > Other than the first comment, LGTM. > > > { > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > struct intel_fbc *fbc = &dev_priv->fbc; > > @@ -1081,8 +1081,8 @@ void intel_fbc_choose_crtc(struct > > drm_i915_private *dev_priv, > > * intel_fbc_disable in the middle, as long as it is deactivated. > > */ > > void intel_fbc_enable(struct intel_crtc *crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_plane_state *plane_state) > > + const struct intel_crtc_state *crtc_state, > > + const struct intel_plane_state *plane_state) > > { > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > struct intel_fbc *fbc = &dev_priv->fbc; > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h > > b/drivers/gpu/drm/i915/display/intel_fbc.h > > index 50272eda8d43..f58b0b1e3c5c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.h > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.h > > @@ -20,14 +20,14 @@ void intel_fbc_choose_crtc(struct > > drm_i915_private *dev_priv, > > struct intel_atomic_state *state); > > bool intel_fbc_is_active(struct drm_i915_private *dev_priv); > > void intel_fbc_pre_update(struct intel_crtc *crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_plane_state *plane_state); > > + const struct intel_crtc_state *crtc_state, > > + const struct intel_plane_state *plane_state); > > void intel_fbc_post_update(struct intel_crtc *crtc); > > void intel_fbc_init(struct drm_i915_private *dev_priv); > > void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv); > > void intel_fbc_enable(struct intel_crtc *crtc, > > - struct intel_crtc_state *crtc_state, > > - struct intel_plane_state *plane_state); > > + const struct intel_crtc_state *crtc_state, > > + const struct intel_plane_state *plane_state); > > void intel_fbc_disable(struct intel_crtc *crtc); > > void intel_fbc_global_disable(struct drm_i915_private *dev_priv); > > void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
On Thu, 2019-11-28 at 14:02 +0200, Ville Syrjälä wrote: > On Wed, Nov 27, 2019 at 11:25:07PM +0000, Souza, Jose wrote: > > On Wed, 2019-11-27 at 21:05 +0200, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Change the calling convention to just pass the state+crtc and > > > switch to intel_ types throughout. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++----- > > > ---- > > > -- > > > drivers/gpu/drm/i915/display/intel_fbc.c | 14 ++-- > > > drivers/gpu/drm/i915/display/intel_fbc.h | 8 +- > > > 3 files changed, 51 insertions(+), 59 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index e341b97b7dec..72655b5b1365 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -5920,13 +5920,10 @@ static void > > > intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) > > > * completely hide the primary plane. > > > */ > > > static void > > > -intel_post_enable_primary(struct drm_crtc *crtc, > > > - const struct intel_crtc_state > > > *new_crtc_state) > > > +intel_post_enable_primary(struct intel_crtc *crtc) > > > { > > > - struct drm_device *dev = crtc->dev; > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > - enum pipe pipe = intel_crtc->pipe; > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > + enum pipe pipe = crtc->pipe; > > > > > > /* > > > * Gen2 reports pipe underruns whenever all planes are > > > disabled. > > > @@ -6062,20 +6059,21 @@ static bool needs_scalerclk_wa(const > > > struct > > > intel_crtc_state *crtc_state) > > > return false; > > > } > > > > > > -static void intel_post_plane_update(struct intel_crtc_state > > > *old_crtc_state) > > > +static void intel_post_plane_update(struct intel_atomic_state > > > *state, > > > + struct intel_crtc *crtc) > > > { > > > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > > > uapi.crtc); > > > - struct drm_device *dev = crtc->base.dev; > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > > > - struct intel_crtc_state *new_crtc_state = > > > - intel_atomic_get_new_crtc_state(to_intel_atomic_state(s > > > tate), > > > - crtc); > > > - struct drm_plane *primary = crtc->base.primary; > > > - struct drm_plane_state *old_primary_state = > > > - drm_atomic_get_old_plane_state(state, primary); > > > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > > > + struct intel_plane *primary = to_intel_plane(crtc- > > > > base.primary); > > > + const struct intel_crtc_state *old_crtc_state = > > > + intel_atomic_get_old_crtc_state(state, crtc); > > > + const struct intel_crtc_state *new_crtc_state = > > > + intel_atomic_get_new_crtc_state(state, crtc); > > > + const struct intel_plane_state *old_primary_state = > > > + intel_atomic_get_old_plane_state(state, primary); > > > + const struct intel_plane_state *new_primary_state = > > > + intel_atomic_get_new_plane_state(state, primary); > > > > > > - intel_frontbuffer_flip(to_i915(crtc->base.dev), new_crtc_state- > > > > fb_bits); > > > + intel_frontbuffer_flip(dev_priv, new_crtc_state->fb_bits); > > > > > > if (new_crtc_state->update_wm_post && new_crtc_state- > > > > hw.active) > > > intel_update_watermarks(crtc); > > > @@ -6083,16 +6081,13 @@ static void > > > intel_post_plane_update(struct > > > intel_crtc_state *old_crtc_state) > > > if (hsw_post_update_enable_ips(old_crtc_state, new_crtc_state)) > > > hsw_enable_ips(new_crtc_state); > > > > > > - if (old_primary_state) { > > > - struct drm_plane_state *new_primary_state = > > > - drm_atomic_get_new_plane_state(state, primary); > > > - > > > + if (new_primary_state) { > > > > This change from old_primary_state to new_primary_state is way more > > than the commit message says, the change looks right to me but > > maybe it > > deserves a separated patch? Same for the same change in > > intel_pre_plane_update() > > I wanted to change it so I can eliminate old_primary_state in > a subsequent patch. For whatever reason that change slipped > into this patch. It's a nop change though since > !new_state == !old_state always. Maybe worthy to mention it in the description?! Will leave that up to you. Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > > intel_fbc_post_update(crtc); > > > > > > - if (new_primary_state->visible && > > > + if (new_primary_state->uapi.visible && > > > (needs_modeset(new_crtc_state) || > > > - !old_primary_state->visible)) > > > - intel_post_enable_primary(&crtc->base, > > > new_crtc_state); > > > + !old_primary_state->uapi.visible)) > > > + intel_post_enable_primary(crtc); > > > } > > > > > > if (needs_nv12_wa(old_crtc_state) && > > > @@ -6104,34 +6099,31 @@ static void > > > intel_post_plane_update(struct > > > intel_crtc_state *old_crtc_state) > > > icl_wa_scalerclkgating(dev_priv, crtc->pipe, false); > > > } > > > > > > -static void intel_pre_plane_update(struct intel_crtc_state > > > *old_crtc_state, > > > - struct intel_crtc_state > > > *new_crtc_state) > > > +static void intel_pre_plane_update(struct intel_atomic_state > > > *state, > > > + struct intel_crtc *crtc) > > > { > > > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > > > uapi.crtc); > > > - struct drm_device *dev = crtc->base.dev; > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > > > - struct drm_plane *primary = crtc->base.primary; > > > - struct drm_plane_state *old_primary_state = > > > - drm_atomic_get_old_plane_state(state, primary); > > > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > > > + struct intel_plane *primary = to_intel_plane(crtc- > > > > base.primary); > > > + const struct intel_crtc_state *old_crtc_state = > > > + intel_atomic_get_old_crtc_state(state, crtc); > > > + const struct intel_crtc_state *new_crtc_state = > > > + intel_atomic_get_new_crtc_state(state, crtc); > > > + const struct intel_plane_state *old_primary_state = > > > + intel_atomic_get_old_plane_state(state, primary); > > > + const struct intel_plane_state *new_primary_state = > > > + intel_atomic_get_new_plane_state(state, primary); > > > bool modeset = needs_modeset(new_crtc_state); > > > - struct intel_atomic_state *intel_state = > > > - to_intel_atomic_state(state); > > > > > > if (hsw_pre_update_disable_ips(old_crtc_state, new_crtc_state)) > > > hsw_disable_ips(old_crtc_state); > > > > > > - if (old_primary_state) { > > > - struct intel_plane_state *new_primary_state = > > > - intel_atomic_get_new_plane_state(intel_state, > > > - to_intel_plane > > > (primary)); > > > - > > > + if (new_primary_state) { > > > intel_fbc_pre_update(crtc, new_crtc_state, > > > new_primary_state); > > > /* > > > * Gen2 reports pipe underruns whenever all planes are > > > disabled. > > > * So disable underrun reporting before all the planes > > > get disabled. > > > */ > > > - if (IS_GEN(dev_priv, 2) && old_primary_state->visible > > > && > > > + if (IS_GEN(dev_priv, 2) && old_primary_state- > > > > uapi.visible && > > > (modeset || !new_primary_state->uapi.visible)) > > > intel_set_cpu_fifo_underrun_reporting(dev_priv, > > > crtc->pipe, false); > > > } > > > @@ -6192,7 +6184,7 @@ static void intel_pre_plane_update(struct > > > intel_crtc_state *old_crtc_state, > > > * us to. > > > */ > > > if (dev_priv->display.initial_watermarks) > > > - dev_priv->display.initial_watermarks(intel_state, > > > crtc); > > > + dev_priv->display.initial_watermarks(state, crtc); > > > else if (new_crtc_state->update_wm_pre) > > > intel_update_watermarks(crtc); > > > } > > > @@ -14365,7 +14357,7 @@ static void intel_update_crtc(struct > > > intel_crtc *crtc, > > > new_crtc_state->update_pipe)) > > > intel_color_load_luts(new_crtc_state); > > > > > > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > > > + intel_pre_plane_update(state, crtc); > > > > > > if (new_crtc_state->update_pipe) > > > intel_encoders_update_pipe(state, crtc); > > > @@ -14460,7 +14452,7 @@ static void > > > intel_trans_port_sync_modeset_disables(struct intel_atomic_state > > > *st > > > !old_slave_crtc_state); > > > > > > /* Disable Slave first */ > > > - intel_pre_plane_update(old_slave_crtc_state, > > > new_slave_crtc_state); > > > + intel_pre_plane_update(state, slave_crtc); > > > if (old_slave_crtc_state->hw.active) > > > intel_old_crtc_state_disables(state, > > > old_slave_crtc_state, > > > @@ -14468,7 +14460,7 @@ static void > > > intel_trans_port_sync_modeset_disables(struct intel_atomic_state > > > *st > > > slave_crtc); > > > > > > /* Disable Master */ > > > - intel_pre_plane_update(old_crtc_state, new_crtc_state); > > > + intel_pre_plane_update(state, crtc); > > > if (old_crtc_state->hw.active) > > > intel_old_crtc_state_disables(state, > > > old_crtc_state, > > > @@ -14508,7 +14500,7 @@ static void > > > intel_commit_modeset_disables(struct intel_atomic_state *state) > > > else > > > continue; > > > } else { > > > - intel_pre_plane_update(old_crtc_state, > > > new_crtc_state); > > > + intel_pre_plane_update(state, crtc); > > > > > > if (old_crtc_state->hw.active) > > > intel_old_crtc_state_disables(state, > > > @@ -14896,7 +14888,7 @@ static void > > > intel_atomic_commit_tail(struct > > > intel_atomic_state *state) > > > } > > > > > > for_each_oldnew_intel_crtc_in_state(state, crtc, > > > old_crtc_state, new_crtc_state, i) { > > > - intel_post_plane_update(old_crtc_state); > > > + intel_post_plane_update(state, crtc); > > > > > > if (put_domains[i]) > > > modeset_put_power_domains(dev_priv, > > > put_domains[i]); > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > > index 92c7eb243559..70c56abe1a83 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > > @@ -420,7 +420,7 @@ static void intel_fbc_deactivate(struct > > > drm_i915_private *dev_priv, > > > } > > > > > > static bool multiple_pipes_ok(struct intel_crtc *crtc, > > > - struct intel_plane_state *plane_state) > > > + const struct intel_plane_state > > > *plane_state) > > > { > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > @@ -656,8 +656,8 @@ static bool > > > intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) > > > } > > > > > > static void intel_fbc_update_state_cache(struct intel_crtc > > > *crtc, > > > - struct intel_crtc_state > > > *crtc_state, > > > - struct intel_plane_state > > > *plane_state) > > > + const struct intel_crtc_state > > > *crtc_state, > > > + const struct intel_plane_state > > > *plane_state) > > > { > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > @@ -855,8 +855,8 @@ static void intel_fbc_get_reg_params(struct > > > intel_crtc *crtc, > > > } > > > > > > void intel_fbc_pre_update(struct intel_crtc *crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_plane_state *plane_state) > > > + const struct intel_crtc_state *crtc_state, > > > + const struct intel_plane_state *plane_state) > > > > Nice > > > > Other than the first comment, LGTM. > > > > > { > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > @@ -1081,8 +1081,8 @@ void intel_fbc_choose_crtc(struct > > > drm_i915_private *dev_priv, > > > * intel_fbc_disable in the middle, as long as it is > > > deactivated. > > > */ > > > void intel_fbc_enable(struct intel_crtc *crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_plane_state *plane_state) > > > + const struct intel_crtc_state *crtc_state, > > > + const struct intel_plane_state *plane_state) > > > { > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > struct intel_fbc *fbc = &dev_priv->fbc; > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h > > > b/drivers/gpu/drm/i915/display/intel_fbc.h > > > index 50272eda8d43..f58b0b1e3c5c 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.h > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.h > > > @@ -20,14 +20,14 @@ void intel_fbc_choose_crtc(struct > > > drm_i915_private *dev_priv, > > > struct intel_atomic_state *state); > > > bool intel_fbc_is_active(struct drm_i915_private *dev_priv); > > > void intel_fbc_pre_update(struct intel_crtc *crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_plane_state *plane_state); > > > + const struct intel_crtc_state *crtc_state, > > > + const struct intel_plane_state *plane_state); > > > void intel_fbc_post_update(struct intel_crtc *crtc); > > > void intel_fbc_init(struct drm_i915_private *dev_priv); > > > void intel_fbc_init_pipe_state(struct drm_i915_private > > > *dev_priv); > > > void intel_fbc_enable(struct intel_crtc *crtc, > > > - struct intel_crtc_state *crtc_state, > > > - struct intel_plane_state *plane_state); > > > + const struct intel_crtc_state *crtc_state, > > > + const struct intel_plane_state *plane_state); > > > void intel_fbc_disable(struct intel_crtc *crtc); > > > void intel_fbc_global_disable(struct drm_i915_private > > > *dev_priv); > > > void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
On Tue, Dec 03, 2019 at 09:44:42PM +0000, Souza, Jose wrote: > On Thu, 2019-11-28 at 14:02 +0200, Ville Syrjälä wrote: > > On Wed, Nov 27, 2019 at 11:25:07PM +0000, Souza, Jose wrote: > > > On Wed, 2019-11-27 at 21:05 +0200, Ville Syrjala wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Change the calling convention to just pass the state+crtc and > > > > switch to intel_ types throughout. > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++----- > > > > ---- > > > > -- > > > > drivers/gpu/drm/i915/display/intel_fbc.c | 14 ++-- > > > > drivers/gpu/drm/i915/display/intel_fbc.h | 8 +- > > > > 3 files changed, 51 insertions(+), 59 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index e341b97b7dec..72655b5b1365 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -5920,13 +5920,10 @@ static void > > > > intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) > > > > * completely hide the primary plane. > > > > */ > > > > static void > > > > -intel_post_enable_primary(struct drm_crtc *crtc, > > > > - const struct intel_crtc_state > > > > *new_crtc_state) > > > > +intel_post_enable_primary(struct intel_crtc *crtc) > > > > { > > > > - struct drm_device *dev = crtc->dev; > > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > > - enum pipe pipe = intel_crtc->pipe; > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > + enum pipe pipe = crtc->pipe; > > > > > > > > /* > > > > * Gen2 reports pipe underruns whenever all planes are > > > > disabled. > > > > @@ -6062,20 +6059,21 @@ static bool needs_scalerclk_wa(const > > > > struct > > > > intel_crtc_state *crtc_state) > > > > return false; > > > > } > > > > > > > > -static void intel_post_plane_update(struct intel_crtc_state > > > > *old_crtc_state) > > > > +static void intel_post_plane_update(struct intel_atomic_state > > > > *state, > > > > + struct intel_crtc *crtc) > > > > { > > > > - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > > > > uapi.crtc); > > > > - struct drm_device *dev = crtc->base.dev; > > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > > - struct drm_atomic_state *state = old_crtc_state->uapi.state; > > > > - struct intel_crtc_state *new_crtc_state = > > > > - intel_atomic_get_new_crtc_state(to_intel_atomic_state(s > > > > tate), > > > > - crtc); > > > > - struct drm_plane *primary = crtc->base.primary; > > > > - struct drm_plane_state *old_primary_state = > > > > - drm_atomic_get_old_plane_state(state, primary); > > > > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > > > > + struct intel_plane *primary = to_intel_plane(crtc- > > > > > base.primary); > > > > + const struct intel_crtc_state *old_crtc_state = > > > > + intel_atomic_get_old_crtc_state(state, crtc); > > > > + const struct intel_crtc_state *new_crtc_state = > > > > + intel_atomic_get_new_crtc_state(state, crtc); > > > > + const struct intel_plane_state *old_primary_state = > > > > + intel_atomic_get_old_plane_state(state, primary); > > > > + const struct intel_plane_state *new_primary_state = > > > > + intel_atomic_get_new_plane_state(state, primary); > > > > > > > > - intel_frontbuffer_flip(to_i915(crtc->base.dev), new_crtc_state- > > > > > fb_bits); > > > > + intel_frontbuffer_flip(dev_priv, new_crtc_state->fb_bits); > > > > > > > > if (new_crtc_state->update_wm_post && new_crtc_state- > > > > > hw.active) > > > > intel_update_watermarks(crtc); > > > > @@ -6083,16 +6081,13 @@ static void > > > > intel_post_plane_update(struct > > > > intel_crtc_state *old_crtc_state) > > > > if (hsw_post_update_enable_ips(old_crtc_state, new_crtc_state)) > > > > hsw_enable_ips(new_crtc_state); > > > > > > > > - if (old_primary_state) { > > > > - struct drm_plane_state *new_primary_state = > > > > - drm_atomic_get_new_plane_state(state, primary); > > > > - > > > > + if (new_primary_state) { > > > > > > This change from old_primary_state to new_primary_state is way more > > > than the commit message says, the change looks right to me but > > > maybe it > > > deserves a separated patch? Same for the same change in > > > intel_pre_plane_update() > > > > I wanted to change it so I can eliminate old_primary_state in > > a subsequent patch. For whatever reason that change slipped > > into this patch. It's a nop change though since > > !new_state == !old_state always. > > Maybe worthy to mention it in the description?! Will leave that up to > you. Sure. Amended a bit while pushing. Thanks for the review.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e341b97b7dec..72655b5b1365 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5920,13 +5920,10 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) * completely hide the primary plane. */ static void -intel_post_enable_primary(struct drm_crtc *crtc, - const struct intel_crtc_state *new_crtc_state) +intel_post_enable_primary(struct intel_crtc *crtc) { - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum pipe pipe = crtc->pipe; /* * Gen2 reports pipe underruns whenever all planes are disabled. @@ -6062,20 +6059,21 @@ static bool needs_scalerclk_wa(const struct intel_crtc_state *crtc_state) return false; } -static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) +static void intel_post_plane_update(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct drm_atomic_state *state = old_crtc_state->uapi.state; - struct intel_crtc_state *new_crtc_state = - intel_atomic_get_new_crtc_state(to_intel_atomic_state(state), - crtc); - struct drm_plane *primary = crtc->base.primary; - struct drm_plane_state *old_primary_state = - drm_atomic_get_old_plane_state(state, primary); + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_plane *primary = to_intel_plane(crtc->base.primary); + const struct intel_crtc_state *old_crtc_state = + intel_atomic_get_old_crtc_state(state, crtc); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + const struct intel_plane_state *old_primary_state = + intel_atomic_get_old_plane_state(state, primary); + const struct intel_plane_state *new_primary_state = + intel_atomic_get_new_plane_state(state, primary); - intel_frontbuffer_flip(to_i915(crtc->base.dev), new_crtc_state->fb_bits); + intel_frontbuffer_flip(dev_priv, new_crtc_state->fb_bits); if (new_crtc_state->update_wm_post && new_crtc_state->hw.active) intel_update_watermarks(crtc); @@ -6083,16 +6081,13 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (hsw_post_update_enable_ips(old_crtc_state, new_crtc_state)) hsw_enable_ips(new_crtc_state); - if (old_primary_state) { - struct drm_plane_state *new_primary_state = - drm_atomic_get_new_plane_state(state, primary); - + if (new_primary_state) { intel_fbc_post_update(crtc); - if (new_primary_state->visible && + if (new_primary_state->uapi.visible && (needs_modeset(new_crtc_state) || - !old_primary_state->visible)) - intel_post_enable_primary(&crtc->base, new_crtc_state); + !old_primary_state->uapi.visible)) + intel_post_enable_primary(crtc); } if (needs_nv12_wa(old_crtc_state) && @@ -6104,34 +6099,31 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) icl_wa_scalerclkgating(dev_priv, crtc->pipe, false); } -static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, - struct intel_crtc_state *new_crtc_state) +static void intel_pre_plane_update(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct drm_atomic_state *state = old_crtc_state->uapi.state; - struct drm_plane *primary = crtc->base.primary; - struct drm_plane_state *old_primary_state = - drm_atomic_get_old_plane_state(state, primary); + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_plane *primary = to_intel_plane(crtc->base.primary); + const struct intel_crtc_state *old_crtc_state = + intel_atomic_get_old_crtc_state(state, crtc); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + const struct intel_plane_state *old_primary_state = + intel_atomic_get_old_plane_state(state, primary); + const struct intel_plane_state *new_primary_state = + intel_atomic_get_new_plane_state(state, primary); bool modeset = needs_modeset(new_crtc_state); - struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); if (hsw_pre_update_disable_ips(old_crtc_state, new_crtc_state)) hsw_disable_ips(old_crtc_state); - if (old_primary_state) { - struct intel_plane_state *new_primary_state = - intel_atomic_get_new_plane_state(intel_state, - to_intel_plane(primary)); - + if (new_primary_state) { intel_fbc_pre_update(crtc, new_crtc_state, new_primary_state); /* * Gen2 reports pipe underruns whenever all planes are disabled. * So disable underrun reporting before all the planes get disabled. */ - if (IS_GEN(dev_priv, 2) && old_primary_state->visible && + if (IS_GEN(dev_priv, 2) && old_primary_state->uapi.visible && (modeset || !new_primary_state->uapi.visible)) intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } @@ -6192,7 +6184,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, * us to. */ if (dev_priv->display.initial_watermarks) - dev_priv->display.initial_watermarks(intel_state, crtc); + dev_priv->display.initial_watermarks(state, crtc); else if (new_crtc_state->update_wm_pre) intel_update_watermarks(crtc); } @@ -14365,7 +14357,7 @@ static void intel_update_crtc(struct intel_crtc *crtc, new_crtc_state->update_pipe)) intel_color_load_luts(new_crtc_state); - intel_pre_plane_update(old_crtc_state, new_crtc_state); + intel_pre_plane_update(state, crtc); if (new_crtc_state->update_pipe) intel_encoders_update_pipe(state, crtc); @@ -14460,7 +14452,7 @@ static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st !old_slave_crtc_state); /* Disable Slave first */ - intel_pre_plane_update(old_slave_crtc_state, new_slave_crtc_state); + intel_pre_plane_update(state, slave_crtc); if (old_slave_crtc_state->hw.active) intel_old_crtc_state_disables(state, old_slave_crtc_state, @@ -14468,7 +14460,7 @@ static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *st slave_crtc); /* Disable Master */ - intel_pre_plane_update(old_crtc_state, new_crtc_state); + intel_pre_plane_update(state, crtc); if (old_crtc_state->hw.active) intel_old_crtc_state_disables(state, old_crtc_state, @@ -14508,7 +14500,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) else continue; } else { - intel_pre_plane_update(old_crtc_state, new_crtc_state); + intel_pre_plane_update(state, crtc); if (old_crtc_state->hw.active) intel_old_crtc_state_disables(state, @@ -14896,7 +14888,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) } for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { - intel_post_plane_update(old_crtc_state); + intel_post_plane_update(state, crtc); if (put_domains[i]) modeset_put_power_domains(dev_priv, put_domains[i]); diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 92c7eb243559..70c56abe1a83 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -420,7 +420,7 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv, } static bool multiple_pipes_ok(struct intel_crtc *crtc, - struct intel_plane_state *plane_state) + const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_fbc *fbc = &dev_priv->fbc; @@ -656,8 +656,8 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) } static void intel_fbc_update_state_cache(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state, - struct intel_plane_state *plane_state) + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_fbc *fbc = &dev_priv->fbc; @@ -855,8 +855,8 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc, } void intel_fbc_pre_update(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state, - struct intel_plane_state *plane_state) + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_fbc *fbc = &dev_priv->fbc; @@ -1081,8 +1081,8 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, * intel_fbc_disable in the middle, as long as it is deactivated. */ void intel_fbc_enable(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state, - struct intel_plane_state *plane_state) + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_fbc *fbc = &dev_priv->fbc; diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h index 50272eda8d43..f58b0b1e3c5c 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.h +++ b/drivers/gpu/drm/i915/display/intel_fbc.h @@ -20,14 +20,14 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, struct intel_atomic_state *state); bool intel_fbc_is_active(struct drm_i915_private *dev_priv); void intel_fbc_pre_update(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state, - struct intel_plane_state *plane_state); + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state); void intel_fbc_post_update(struct intel_crtc *crtc); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv); void intel_fbc_enable(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state, - struct intel_plane_state *plane_state); + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state); void intel_fbc_disable(struct intel_crtc *crtc); void intel_fbc_global_disable(struct drm_i915_private *dev_priv); void intel_fbc_invalidate(struct drm_i915_private *dev_priv,