| Message ID | 20181101150605.18235-11-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Program SKL+ watermarks/ddb more carefully | expand |
On Thu, Nov 01, 2018 at 05:06:01PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We're going to need access to the new crtc state in ->disable_plane() > for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass > the crtc state down. > > We'll also try to make intel_crtc_disable_planes() drtr as much as > it's possible. The fact that we don't have a separate crtc state > for the disabled state when we're going to re-enable the crtc later > means we might end up poking at a few extra planes in there. But > that's harmless. I suppose one migth argue that we wouldn't have to > care about proper ddb/wm/csc/gamma if the pipe is going to permanently > disable anyway, but the state checker probably cares so we should try > our best to make sure everything is programmed correctly even in that > case. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++--------- > drivers/gpu/drm/i915/intel_display.h | 8 +++++ > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_sprite.c | 12 ++++--- > 5 files changed, 42 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 010269a12390..69fc7010190c 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > } else { > trace_intel_disable_plane(&plane->base, crtc); > > - plane->disable_plane(plane, crtc); > + plane->disable_plane(plane, new_crtc_state); > } > } > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 33d73915b73e..6088ae554e56 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2781,7 +2781,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, > intel_pre_disable_primary_noatomic(&crtc->base); > > trace_intel_disable_plane(&plane->base, crtc); > - plane->disable_plane(plane, crtc); > + plane->disable_plane(plane, crtc_state); > } > > static void > @@ -3386,7 +3386,7 @@ static void i9xx_update_plane(struct intel_plane *plane, > } > > static void i9xx_disable_plane(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > @@ -5421,23 +5421,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > intel_update_watermarks(crtc); > } > > -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) > +static void intel_crtc_disable_planes(struct intel_atomic_state *state, > + struct intel_crtc *crtc) > { > - struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + const struct intel_crtc_state *new_crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + unsigned int update_mask = new_crtc_state->update_planes; > + const struct intel_plane_state *old_plane_state; > struct intel_plane *plane; > unsigned fb_bits = 0; > + int i; > > intel_crtc_dpms_overlay_disable(crtc); > > - for_each_intel_plane_on_crtc(dev, crtc, plane) { > - if (plane_mask & BIT(plane->id)) { > - plane->disable_plane(plane, crtc); > + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { why getting "i" if it is unused later? do you have plans to reuse this for_each_old_intel_plane_in_state somewhere else? Besides the "i" I'm not sure I liked it returning multiple things. if we are not going to use in more places I'd prefer something more verbose here than macromagics. > + if (crtc->pipe != plane->pipe || > + !(update_mask & BIT(plane->id))) > + continue; > + > + plane->disable_plane(plane, new_crtc_state); > > + if (old_plane_state->base.visible) > fb_bits |= plane->frontbuffer_bit; > - } > } > > - intel_frontbuffer_flip(to_i915(dev), fb_bits); > + intel_frontbuffer_flip(dev_priv, fb_bits); > } > > static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, > @@ -9855,9 +9864,9 @@ static void i845_update_cursor(struct intel_plane *plane, > } > > static void i845_disable_cursor(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > - i845_update_cursor(plane, NULL, NULL); > + i845_update_cursor(plane, crtc_state, NULL); > } > > static bool i845_cursor_get_hw_state(struct intel_plane *plane, > @@ -10084,9 +10093,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, > } > > static void i9xx_disable_cursor(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > - i9xx_update_cursor(plane, NULL, NULL); > + i9xx_update_cursor(plane, crtc_state, NULL); > } > > static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, > @@ -12840,7 +12849,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); > > if (old_crtc_state->active) { > - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); > + intel_crtc_disable_planes(intel_state, intel_crtc); > > /* > * We need to disable pipe CRC before disabling the pipe, > @@ -13695,7 +13704,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, > to_intel_plane_state(plane->state)); > } else { > trace_intel_disable_plane(plane, to_intel_crtc(crtc)); > - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); > + intel_plane->disable_plane(intel_plane, crtc_state); > } > > intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > index 5d50decbcbb5..df9e6ebb27de 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -382,6 +382,14 @@ struct intel_link_m_n { > for_each_power_well_rev(__dev_priv, __power_well) \ > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ > + for ((__i) = 0; \ > + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ > + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ > + (__i)++) \ > + for_each_if(plane) > + > #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 7a55f5921d34..facd5cb0b540 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1015,7 +1015,7 @@ struct intel_plane { > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > void (*disable_plane)(struct intel_plane *plane, > - struct intel_crtc *crtc); > + const struct intel_crtc_state *crtc_state); > bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); > int (*check_plane)(struct intel_crtc_state *crtc_state, > struct intel_plane_state *plane_state); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 84c5f532fba5..2f97a298c24e 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -488,7 +488,8 @@ icl_update_slave(struct intel_plane *plane, > } > > static void > -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +skl_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum plane_id plane_id = plane->id; > @@ -751,7 +752,8 @@ vlv_update_plane(struct intel_plane *plane, > } > > static void > -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +vlv_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > @@ -918,7 +920,8 @@ ivb_update_plane(struct intel_plane *plane, > } > > static void > -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +ivb_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > @@ -1084,7 +1087,8 @@ g4x_update_plane(struct intel_plane *plane, > } > > static void > -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +g4x_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Nov 07, 2018 at 02:08:43PM -0800, Rodrigo Vivi wrote: > On Thu, Nov 01, 2018 at 05:06:01PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We're going to need access to the new crtc state in ->disable_plane() > > for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass > > the crtc state down. > > > > We'll also try to make intel_crtc_disable_planes() drtr as much as > > it's possible. The fact that we don't have a separate crtc state > > for the disabled state when we're going to re-enable the crtc later > > means we might end up poking at a few extra planes in there. But > > that's harmless. I suppose one migth argue that we wouldn't have to > > care about proper ddb/wm/csc/gamma if the pipe is going to permanently > > disable anyway, but the state checker probably cares so we should try > > our best to make sure everything is programmed correctly even in that > > case. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++--------- > > drivers/gpu/drm/i915/intel_display.h | 8 +++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_sprite.c | 12 ++++--- > > 5 files changed, 42 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > index 010269a12390..69fc7010190c 100644 > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > > } else { > > trace_intel_disable_plane(&plane->base, crtc); > > > > - plane->disable_plane(plane, crtc); > > + plane->disable_plane(plane, new_crtc_state); > > } > > } > > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 33d73915b73e..6088ae554e56 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2781,7 +2781,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, > > intel_pre_disable_primary_noatomic(&crtc->base); > > > > trace_intel_disable_plane(&plane->base, crtc); > > - plane->disable_plane(plane, crtc); > > + plane->disable_plane(plane, crtc_state); > > } > > > > static void > > @@ -3386,7 +3386,7 @@ static void i9xx_update_plane(struct intel_plane *plane, > > } > > > > static void i9xx_disable_plane(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > > @@ -5421,23 +5421,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > > intel_update_watermarks(crtc); > > } > > > > -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) > > +static void intel_crtc_disable_planes(struct intel_atomic_state *state, > > + struct intel_crtc *crtc) > > { > > - struct drm_device *dev = crtc->base.dev; > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + const struct intel_crtc_state *new_crtc_state = > > + intel_atomic_get_new_crtc_state(state, crtc); > > + unsigned int update_mask = new_crtc_state->update_planes; > > + const struct intel_plane_state *old_plane_state; > > struct intel_plane *plane; > > unsigned fb_bits = 0; > > + int i; > > > > intel_crtc_dpms_overlay_disable(crtc); > > > > - for_each_intel_plane_on_crtc(dev, crtc, plane) { > > - if (plane_mask & BIT(plane->id)) { > > - plane->disable_plane(plane, crtc); > > + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { > > why getting "i" if it is unused later? The macro requires it. I've been pondering using c99 and moving the declaration into the for loop itself, but I'm not sure whether that's still acceptable in the kernel. IIRC there is no other kernel code doing that but there is some in tools/. > > do you have plans to reuse this for_each_old_intel_plane_in_state somewhere else? > Besides the "i" I'm not sure I liked it returning multiple things. These things are used all over atomic code. There are drm_ versions that operate on the drm_ types, and for the purposes of not cluttering our code so much with aliasing pointers, casts and whatnot we have our intel_ variants as well. > > if we are not going to use in more places I'd prefer something more verbose > here than macromagics. > > > + if (crtc->pipe != plane->pipe || > > + !(update_mask & BIT(plane->id))) > > + continue; > > + > > + plane->disable_plane(plane, new_crtc_state); > > > > + if (old_plane_state->base.visible) > > fb_bits |= plane->frontbuffer_bit; > > - } > > } > > > > - intel_frontbuffer_flip(to_i915(dev), fb_bits); > > + intel_frontbuffer_flip(dev_priv, fb_bits); > > } > > > > static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, > > @@ -9855,9 +9864,9 @@ static void i845_update_cursor(struct intel_plane *plane, > > } > > > > static void i845_disable_cursor(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > - i845_update_cursor(plane, NULL, NULL); > > + i845_update_cursor(plane, crtc_state, NULL); > > } > > > > static bool i845_cursor_get_hw_state(struct intel_plane *plane, > > @@ -10084,9 +10093,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, > > } > > > > static void i9xx_disable_cursor(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > - i9xx_update_cursor(plane, NULL, NULL); > > + i9xx_update_cursor(plane, crtc_state, NULL); > > } > > > > static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, > > @@ -12840,7 +12849,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > > intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); > > > > if (old_crtc_state->active) { > > - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); > > + intel_crtc_disable_planes(intel_state, intel_crtc); > > > > /* > > * We need to disable pipe CRC before disabling the pipe, > > @@ -13695,7 +13704,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, > > to_intel_plane_state(plane->state)); > > } else { > > trace_intel_disable_plane(plane, to_intel_crtc(crtc)); > > - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); > > + intel_plane->disable_plane(intel_plane, crtc_state); > > } > > > > intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > index 5d50decbcbb5..df9e6ebb27de 100644 > > --- a/drivers/gpu/drm/i915/intel_display.h > > +++ b/drivers/gpu/drm/i915/intel_display.h > > @@ -382,6 +382,14 @@ struct intel_link_m_n { > > for_each_power_well_rev(__dev_priv, __power_well) \ > > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > > > +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ > > + for ((__i) = 0; \ > > + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > > + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ > > + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ > > + (__i)++) \ > > + for_each_if(plane) > > + > > #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 7a55f5921d34..facd5cb0b540 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1015,7 +1015,7 @@ struct intel_plane { > > const struct intel_crtc_state *crtc_state, > > const struct intel_plane_state *plane_state); > > void (*disable_plane)(struct intel_plane *plane, > > - struct intel_crtc *crtc); > > + const struct intel_crtc_state *crtc_state); > > bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); > > int (*check_plane)(struct intel_crtc_state *crtc_state, > > struct intel_plane_state *plane_state); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 84c5f532fba5..2f97a298c24e 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -488,7 +488,8 @@ icl_update_slave(struct intel_plane *plane, > > } > > > > static void > > -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +skl_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum plane_id plane_id = plane->id; > > @@ -751,7 +752,8 @@ vlv_update_plane(struct intel_plane *plane, > > } > > > > static void > > -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +vlv_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > @@ -918,7 +920,8 @@ ivb_update_plane(struct intel_plane *plane, > > } > > > > static void > > -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +ivb_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > @@ -1084,7 +1087,8 @@ g4x_update_plane(struct intel_plane *plane, > > } > > > > static void > > -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +g4x_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > -- > > 2.18.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Nov 01, 2018 at 05:06:01PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We're going to need access to the new crtc state in ->disable_plane() > for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass > the crtc state down. > > We'll also try to make intel_crtc_disable_planes() drtr as much as What does "drtr" stand for? The main behavioral change I see in that function is that we don't do frontbuffer tracking for nv12 slave planes anymore (and in the future won't do it for other planes that weren't visible, but you still want to reprogram them in some way). Matt > it's possible. The fact that we don't have a separate crtc state > for the disabled state when we're going to re-enable the crtc later > means we might end up poking at a few extra planes in there. But > that's harmless. I suppose one migth argue that we wouldn't have to > care about proper ddb/wm/csc/gamma if the pipe is going to permanently > disable anyway, but the state checker probably cares so we should try > our best to make sure everything is programmed correctly even in that > case. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++--------- > drivers/gpu/drm/i915/intel_display.h | 8 +++++ > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_sprite.c | 12 ++++--- > 5 files changed, 42 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 010269a12390..69fc7010190c 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > } else { > trace_intel_disable_plane(&plane->base, crtc); > > - plane->disable_plane(plane, crtc); > + plane->disable_plane(plane, new_crtc_state); > } > } > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 33d73915b73e..6088ae554e56 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2781,7 +2781,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, > intel_pre_disable_primary_noatomic(&crtc->base); > > trace_intel_disable_plane(&plane->base, crtc); > - plane->disable_plane(plane, crtc); > + plane->disable_plane(plane, crtc_state); > } > > static void > @@ -3386,7 +3386,7 @@ static void i9xx_update_plane(struct intel_plane *plane, > } > > static void i9xx_disable_plane(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > @@ -5421,23 +5421,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > intel_update_watermarks(crtc); > } > > -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) > +static void intel_crtc_disable_planes(struct intel_atomic_state *state, > + struct intel_crtc *crtc) > { > - struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + const struct intel_crtc_state *new_crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + unsigned int update_mask = new_crtc_state->update_planes; > + const struct intel_plane_state *old_plane_state; > struct intel_plane *plane; > unsigned fb_bits = 0; > + int i; > > intel_crtc_dpms_overlay_disable(crtc); > > - for_each_intel_plane_on_crtc(dev, crtc, plane) { > - if (plane_mask & BIT(plane->id)) { > - plane->disable_plane(plane, crtc); > + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { > + if (crtc->pipe != plane->pipe || > + !(update_mask & BIT(plane->id))) > + continue; > + > + plane->disable_plane(plane, new_crtc_state); > > + if (old_plane_state->base.visible) > fb_bits |= plane->frontbuffer_bit; > - } > } > > - intel_frontbuffer_flip(to_i915(dev), fb_bits); > + intel_frontbuffer_flip(dev_priv, fb_bits); > } > > static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, > @@ -9855,9 +9864,9 @@ static void i845_update_cursor(struct intel_plane *plane, > } > > static void i845_disable_cursor(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > - i845_update_cursor(plane, NULL, NULL); > + i845_update_cursor(plane, crtc_state, NULL); > } > > static bool i845_cursor_get_hw_state(struct intel_plane *plane, > @@ -10084,9 +10093,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, > } > > static void i9xx_disable_cursor(struct intel_plane *plane, > - struct intel_crtc *crtc) > + const struct intel_crtc_state *crtc_state) > { > - i9xx_update_cursor(plane, NULL, NULL); > + i9xx_update_cursor(plane, crtc_state, NULL); > } > > static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, > @@ -12840,7 +12849,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); > > if (old_crtc_state->active) { > - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); > + intel_crtc_disable_planes(intel_state, intel_crtc); > > /* > * We need to disable pipe CRC before disabling the pipe, > @@ -13695,7 +13704,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, > to_intel_plane_state(plane->state)); > } else { > trace_intel_disable_plane(plane, to_intel_crtc(crtc)); > - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); > + intel_plane->disable_plane(intel_plane, crtc_state); > } > > intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > index 5d50decbcbb5..df9e6ebb27de 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -382,6 +382,14 @@ struct intel_link_m_n { > for_each_power_well_rev(__dev_priv, __power_well) \ > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ > + for ((__i) = 0; \ > + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ > + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ > + (__i)++) \ > + for_each_if(plane) > + > #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 7a55f5921d34..facd5cb0b540 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1015,7 +1015,7 @@ struct intel_plane { > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > void (*disable_plane)(struct intel_plane *plane, > - struct intel_crtc *crtc); > + const struct intel_crtc_state *crtc_state); > bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); > int (*check_plane)(struct intel_crtc_state *crtc_state, > struct intel_plane_state *plane_state); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 84c5f532fba5..2f97a298c24e 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -488,7 +488,8 @@ icl_update_slave(struct intel_plane *plane, > } > > static void > -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +skl_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum plane_id plane_id = plane->id; > @@ -751,7 +752,8 @@ vlv_update_plane(struct intel_plane *plane, > } > > static void > -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +vlv_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > @@ -918,7 +920,8 @@ ivb_update_plane(struct intel_plane *plane, > } > > static void > -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +ivb_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > @@ -1084,7 +1087,8 @@ g4x_update_plane(struct intel_plane *plane, > } > > static void > -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > +g4x_disable_plane(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum pipe pipe = plane->pipe; > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Nov 08, 2018 at 03:52:02PM -0800, Matt Roper wrote: > On Thu, Nov 01, 2018 at 05:06:01PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We're going to need access to the new crtc state in ->disable_plane() > > for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass > > the crtc state down. > > > > We'll also try to make intel_crtc_disable_planes() drtr as much as > > What does "drtr" stand for? Should have been "dtrt" as in "do the right thing". > > The main behavioral change I see in that function is that we don't do > frontbuffer tracking for nv12 slave planes anymore (and in the future > won't do it for other planes that weren't visible, but you still want to > reprogram them in some way). So intel_post_plane_update() calls frontbuffer_flip(pipe_config->fb_bits), pipe_config->fb_bits is populated by intel_plane_atomic_calc_changes() based on the old+new plane visibility, hence new slave planes won't be set in that bitmask, unless they were previosuly used as non-slave planes. So I think that does agree with my new code pretty well. However, thinking a bit more what the frontbuffer tracking is trying to achieve, maybe we should always include every visible plane that is being reprogrammed. Eg. if we have to reprogram plane 1A on account of a ddb change the resulting flip does cause an fbc nuke, which could be used by the frontbuffer tracking to re-enable fbc if it had been previosuly invalidated. But this probably needs more thought so should be done later (if ever). > > > Matt > > > it's possible. The fact that we don't have a separate crtc state > > for the disabled state when we're going to re-enable the crtc later > > means we might end up poking at a few extra planes in there. But > > that's harmless. I suppose one migth argue that we wouldn't have to > > care about proper ddb/wm/csc/gamma if the pipe is going to permanently > > disable anyway, but the state checker probably cares so we should try > > our best to make sure everything is programmed correctly even in that > > case. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++--------- > > drivers/gpu/drm/i915/intel_display.h | 8 +++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_sprite.c | 12 ++++--- > > 5 files changed, 42 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > index 010269a12390..69fc7010190c 100644 > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > > } else { > > trace_intel_disable_plane(&plane->base, crtc); > > > > - plane->disable_plane(plane, crtc); > > + plane->disable_plane(plane, new_crtc_state); > > } > > } > > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 33d73915b73e..6088ae554e56 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2781,7 +2781,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, > > intel_pre_disable_primary_noatomic(&crtc->base); > > > > trace_intel_disable_plane(&plane->base, crtc); > > - plane->disable_plane(plane, crtc); > > + plane->disable_plane(plane, crtc_state); > > } > > > > static void > > @@ -3386,7 +3386,7 @@ static void i9xx_update_plane(struct intel_plane *plane, > > } > > > > static void i9xx_disable_plane(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > > @@ -5421,23 +5421,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > > intel_update_watermarks(crtc); > > } > > > > -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) > > +static void intel_crtc_disable_planes(struct intel_atomic_state *state, > > + struct intel_crtc *crtc) > > { > > - struct drm_device *dev = crtc->base.dev; > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + const struct intel_crtc_state *new_crtc_state = > > + intel_atomic_get_new_crtc_state(state, crtc); > > + unsigned int update_mask = new_crtc_state->update_planes; > > + const struct intel_plane_state *old_plane_state; > > struct intel_plane *plane; > > unsigned fb_bits = 0; > > + int i; > > > > intel_crtc_dpms_overlay_disable(crtc); > > > > - for_each_intel_plane_on_crtc(dev, crtc, plane) { > > - if (plane_mask & BIT(plane->id)) { > > - plane->disable_plane(plane, crtc); > > + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { > > + if (crtc->pipe != plane->pipe || > > + !(update_mask & BIT(plane->id))) > > + continue; > > + > > + plane->disable_plane(plane, new_crtc_state); > > > > + if (old_plane_state->base.visible) > > fb_bits |= plane->frontbuffer_bit; > > - } > > } > > > > - intel_frontbuffer_flip(to_i915(dev), fb_bits); > > + intel_frontbuffer_flip(dev_priv, fb_bits); > > } > > > > static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, > > @@ -9855,9 +9864,9 @@ static void i845_update_cursor(struct intel_plane *plane, > > } > > > > static void i845_disable_cursor(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > - i845_update_cursor(plane, NULL, NULL); > > + i845_update_cursor(plane, crtc_state, NULL); > > } > > > > static bool i845_cursor_get_hw_state(struct intel_plane *plane, > > @@ -10084,9 +10093,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, > > } > > > > static void i9xx_disable_cursor(struct intel_plane *plane, > > - struct intel_crtc *crtc) > > + const struct intel_crtc_state *crtc_state) > > { > > - i9xx_update_cursor(plane, NULL, NULL); > > + i9xx_update_cursor(plane, crtc_state, NULL); > > } > > > > static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, > > @@ -12840,7 +12849,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > > intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); > > > > if (old_crtc_state->active) { > > - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); > > + intel_crtc_disable_planes(intel_state, intel_crtc); > > > > /* > > * We need to disable pipe CRC before disabling the pipe, > > @@ -13695,7 +13704,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, > > to_intel_plane_state(plane->state)); > > } else { > > trace_intel_disable_plane(plane, to_intel_crtc(crtc)); > > - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); > > + intel_plane->disable_plane(intel_plane, crtc_state); > > } > > > > intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h > > index 5d50decbcbb5..df9e6ebb27de 100644 > > --- a/drivers/gpu/drm/i915/intel_display.h > > +++ b/drivers/gpu/drm/i915/intel_display.h > > @@ -382,6 +382,14 @@ struct intel_link_m_n { > > for_each_power_well_rev(__dev_priv, __power_well) \ > > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > > > +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ > > + for ((__i) = 0; \ > > + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > > + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ > > + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ > > + (__i)++) \ > > + for_each_if(plane) > > + > > #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->base.dev->mode_config.num_total_plane && \ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 7a55f5921d34..facd5cb0b540 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1015,7 +1015,7 @@ struct intel_plane { > > const struct intel_crtc_state *crtc_state, > > const struct intel_plane_state *plane_state); > > void (*disable_plane)(struct intel_plane *plane, > > - struct intel_crtc *crtc); > > + const struct intel_crtc_state *crtc_state); > > bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); > > int (*check_plane)(struct intel_crtc_state *crtc_state, > > struct intel_plane_state *plane_state); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 84c5f532fba5..2f97a298c24e 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -488,7 +488,8 @@ icl_update_slave(struct intel_plane *plane, > > } > > > > static void > > -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +skl_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum plane_id plane_id = plane->id; > > @@ -751,7 +752,8 @@ vlv_update_plane(struct intel_plane *plane, > > } > > > > static void > > -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +vlv_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > @@ -918,7 +920,8 @@ ivb_update_plane(struct intel_plane *plane, > > } > > > > static void > > -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +ivb_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > @@ -1084,7 +1087,8 @@ g4x_update_plane(struct intel_plane *plane, > > } > > > > static void > > -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > +g4x_disable_plane(struct intel_plane *plane, > > + const struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > enum pipe pipe = plane->pipe; > > -- > > 2.18.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 010269a12390..69fc7010190c 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, } else { trace_intel_disable_plane(&plane->base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, new_crtc_state); } } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 33d73915b73e..6088ae554e56 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2781,7 +2781,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, intel_pre_disable_primary_noatomic(&crtc->base); trace_intel_disable_plane(&plane->base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, crtc_state); } static void @@ -3386,7 +3386,7 @@ static void i9xx_update_plane(struct intel_plane *plane, } static void i9xx_disable_plane(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; @@ -5421,23 +5421,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, intel_update_watermarks(crtc); } -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) +static void intel_crtc_disable_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + unsigned int update_mask = new_crtc_state->update_planes; + const struct intel_plane_state *old_plane_state; struct intel_plane *plane; unsigned fb_bits = 0; + int i; intel_crtc_dpms_overlay_disable(crtc); - for_each_intel_plane_on_crtc(dev, crtc, plane) { - if (plane_mask & BIT(plane->id)) { - plane->disable_plane(plane, crtc); + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { + if (crtc->pipe != plane->pipe || + !(update_mask & BIT(plane->id))) + continue; + + plane->disable_plane(plane, new_crtc_state); + if (old_plane_state->base.visible) fb_bits |= plane->frontbuffer_bit; - } } - intel_frontbuffer_flip(to_i915(dev), fb_bits); + intel_frontbuffer_flip(dev_priv, fb_bits); } static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, @@ -9855,9 +9864,9 @@ static void i845_update_cursor(struct intel_plane *plane, } static void i845_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { - i845_update_cursor(plane, NULL, NULL); + i845_update_cursor(plane, crtc_state, NULL); } static bool i845_cursor_get_hw_state(struct intel_plane *plane, @@ -10084,9 +10093,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, } static void i9xx_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { - i9xx_update_cursor(plane, NULL, NULL); + i9xx_update_cursor(plane, crtc_state, NULL); } static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, @@ -12840,7 +12849,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); if (old_crtc_state->active) { - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); + intel_crtc_disable_planes(intel_state, intel_crtc); /* * We need to disable pipe CRC before disabling the pipe, @@ -13695,7 +13704,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, to_intel_plane_state(plane->state)); } else { trace_intel_disable_plane(plane, to_intel_crtc(crtc)); - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); + intel_plane->disable_plane(intel_plane, crtc_state); } intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index 5d50decbcbb5..df9e6ebb27de 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -382,6 +382,14 @@ struct intel_link_m_n { for_each_power_well_rev(__dev_priv, __power_well) \ for_each_if((__power_well)->desc->domains & (__domain_mask)) +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ + for ((__i) = 0; \ + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ + (__i)++) \ + for_each_if(plane) + #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->base.dev->mode_config.num_total_plane && \ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7a55f5921d34..facd5cb0b540 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1015,7 +1015,7 @@ struct intel_plane { const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); void (*disable_plane)(struct intel_plane *plane, - struct intel_crtc *crtc); + const struct intel_crtc_state *crtc_state); bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); int (*check_plane)(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 84c5f532fba5..2f97a298c24e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -488,7 +488,8 @@ icl_update_slave(struct intel_plane *plane, } static void -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +skl_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum plane_id plane_id = plane->id; @@ -751,7 +752,8 @@ vlv_update_plane(struct intel_plane *plane, } static void -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +vlv_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; @@ -918,7 +920,8 @@ ivb_update_plane(struct intel_plane *plane, } static void -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +ivb_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; @@ -1084,7 +1087,8 @@ g4x_update_plane(struct intel_plane *plane, } static void -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +g4x_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe;