From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Souza, Jose" <jose.souza@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 4/7] drm/i915: Clean up intel_{pre, post}_plane_update()
Date: Thu, 28 Nov 2019 14:02:01 +0200 [thread overview]
Message-ID: <20191128120201.GR1208@intel.com> (raw)
In-Reply-To: <7c8dc4d946397ad797d418ae98313965194c8211.camel@intel.com>
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,
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Souza, Jose" <jose.souza@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 4/7] drm/i915: Clean up intel_{pre, post}_plane_update()
Date: Thu, 28 Nov 2019 14:02:01 +0200 [thread overview]
Message-ID: <20191128120201.GR1208@intel.com> (raw)
Message-ID: <20191128120201.KzBm6yqFg06CDY5W0nDm3cWyPyqRbmaBx68iF7nn2bg@z> (raw)
In-Reply-To: <7c8dc4d946397ad797d418ae98313965194c8211.camel@intel.com>
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,
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-11-28 12:02 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-27 19:05 [PATCH 0/7] drm/i915: Cleanups around pre/post plane update Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 19:05 ` [PATCH 1/7] drm/i915: Clean up arguments to nv12/scaler w/a funcs Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:06 ` Souza, Jose
2019-11-27 23:06 ` [Intel-gfx] " Souza, Jose
2019-11-27 19:05 ` [PATCH 2/7] drm/i915: Pass dev_priv to ilk_disable_lp_wm() Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:06 ` Souza, Jose
2019-11-27 23:06 ` [Intel-gfx] " Souza, Jose
2019-11-27 19:05 ` [PATCH 3/7] drm/i915: s/pipe_config/new_crtc_state/ intel_{pre, post}_plane_update() Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:09 ` Souza, Jose
2019-11-27 23:09 ` [Intel-gfx] " Souza, Jose
2019-11-27 23:11 ` Souza, Jose
2019-11-27 23:11 ` [Intel-gfx] " Souza, Jose
2019-11-28 12:00 ` Ville Syrjälä
2019-11-28 12:00 ` [Intel-gfx] " Ville Syrjälä
2019-11-27 19:05 ` [PATCH 4/7] drm/i915: Clean up " Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:25 ` Souza, Jose
2019-11-27 23:25 ` [Intel-gfx] " Souza, Jose
2019-11-28 12:02 ` Ville Syrjälä [this message]
2019-11-28 12:02 ` Ville Syrjälä
2019-12-03 21:44 ` Souza, Jose
2019-12-04 14:01 ` Ville Syrjälä
2019-11-27 19:05 ` [PATCH 5/7] drm/i915: Clean up the gen2 "no planes -> underrun" workaround Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:44 ` Souza, Jose
2019-11-27 23:44 ` [Intel-gfx] " Souza, Jose
2019-11-27 19:05 ` [PATCH 6/7] drm/i915: Nuke intel_pre_disable_primary_noatomic() Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:56 ` Souza, Jose
2019-11-27 23:56 ` [Intel-gfx] " Souza, Jose
2019-11-27 19:05 ` [PATCH 7/7] drm/i915: Make intel_crtc_arm_fifo_underrun() functional on gen2 Ville Syrjala
2019-11-27 19:05 ` [Intel-gfx] " Ville Syrjala
2019-11-27 23:57 ` Souza, Jose
2019-11-27 23:57 ` [Intel-gfx] " Souza, Jose
2019-11-27 22:17 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanups around pre/post plane update Patchwork
2019-11-27 22:17 ` [Intel-gfx] " Patchwork
2019-11-29 2:05 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-11-29 2:05 ` [Intel-gfx] " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191128120201.GR1208@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.