| Message ID | 20170228141837.22949-5-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Dienstag, den 28.02.2017, 15:18 +0100 schrieb Philipp Zabel: > The DP (display processor) channel disable code tried to busy wait for > the DP sync flow end interrupt status bit when disabling the partial > plane without a full modeset. That never worked reliably, and it was > disabled completely by the recent "gpu: ipu-v3: remove IRQ dance on DC > channel disable" patch, causing ipu_wait_interrupt to always time out > after 50 ms, which in turn would trigger a timeout in > drm_atomic_helper_wait_for_vblanks. > > This patch changes ipu_plane_atomic_disable to only queue a DP channel > register update at the next frame boundary and set a flag, which can be > done without any waiting whatsoever. The imx_drm_atomic_commit_tail then > calls a new ipu_plane_disable_deferred function that does the actual > IDMAC teardown of the planes that are flagged for deferred disabling, > after waiting for the vblank. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > Changes since v2: > - Add missing export for ipu_plane_disable_deferred > - Check if there are any planes to be disabled and only then wait for > vblanks and call the deferred disable function > --- > drivers/gpu/drm/imx/imx-drm-core.c | 18 ++++++++++++++++++ > drivers/gpu/drm/imx/ipuv3-crtc.c | 22 +++++++++++++++++++++- > drivers/gpu/drm/imx/ipuv3-plane.c | 25 ++++++++++++++++++------- > drivers/gpu/drm/imx/ipuv3-plane.h | 5 +++++ > drivers/gpu/ipu-v3/ipu-dp.c | 3 --- > 5 files changed, 62 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c > index 0a5e4fbb906bf..94f9c25e1c67b 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -30,6 +30,7 @@ > #include <video/imx-ipu-v3.h> > > #include "imx-drm.h" > +#include "ipuv3-plane.h" > > #define MAX_CRTC 4 > > @@ -160,6 +161,10 @@ static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { > static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) > { > struct drm_device *dev = state->dev; > + struct drm_plane *plane; > + struct drm_plane_state *plane_state; > + bool plane_disabling = false; > + int i; > > drm_atomic_helper_commit_modeset_disables(dev, state); > > @@ -169,6 +174,19 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) > > drm_atomic_helper_commit_modeset_enables(dev, state); > > + for_each_plane_in_state(state, plane, plane_state, i) { > + if (drm_atomic_plane_disabling(plane, plane_state)) > + plane_disabling = true; > + } > + > + if (plane_disabling) { > + drm_atomic_helper_wait_for_vblanks(dev, state); > + > + for_each_plane_in_state(state, plane, plane_state, i) > + ipu_plane_disable_deferred(plane); > + > + } > + > drm_atomic_helper_commit_hw_done(state); > } > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c > index 6be515a9fb694..0f15f11f26e0c 100644 > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > @@ -60,6 +60,26 @@ static void ipu_crtc_enable(struct drm_crtc *crtc) > ipu_di_enable(ipu_crtc->di); > } > > +static void ipu_crtc_disable_planes(struct ipu_crtc *ipu_crtc, > + struct drm_crtc_state *old_crtc_state) > +{ > + bool disable_partial = false; > + bool disable_full = false; > + struct drm_plane *plane; > + > + drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) { > + if (plane == &ipu_crtc->plane[0]->base) > + disable_full = true; > + if (&ipu_crtc->plane[1] && plane == &ipu_crtc->plane[1]->base) > + disable_partial = true; > + } > + > + if (disable_partial) > + ipu_plane_disable(ipu_crtc->plane[1], true); > + if (disable_full) > + ipu_plane_disable(ipu_crtc->plane[0], false); > +} > + > static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state) > { > @@ -73,7 +93,7 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, > * attached IDMACs will be left in undefined state, possibly hanging > * the IPU or even system. > */ > - drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); > + ipu_crtc_disable_planes(ipu_crtc, old_crtc_state); > ipu_dc_disable(ipu); > > spin_lock_irq(&crtc->dev->event_lock); > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c > index 55991d46ced50..a37735298615e 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -172,23 +172,30 @@ static void ipu_plane_enable(struct ipu_plane *ipu_plane) > ipu_dp_enable_channel(ipu_plane->dp); > } > > -static int ipu_disable_plane(struct drm_plane *plane) > +void ipu_plane_disable(struct ipu_plane *ipu_plane, bool disable_dp_channel) > { > - struct ipu_plane *ipu_plane = to_ipu_plane(plane); > - > DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); > > ipu_idmac_wait_busy(ipu_plane->ipu_ch, 50); > > - if (ipu_plane->dp) > - ipu_dp_disable_channel(ipu_plane->dp, true); > + if (ipu_plane->dp && disable_dp_channel) > + ipu_dp_disable_channel(ipu_plane->dp, false); > ipu_idmac_disable_channel(ipu_plane->ipu_ch); > ipu_dmfc_disable_channel(ipu_plane->dmfc); > if (ipu_plane->dp) > ipu_dp_disable(ipu_plane->ipu); > +} > > - return 0; > +void ipu_plane_disable_deferred(struct drm_plane *plane) > +{ > + struct ipu_plane *ipu_plane = to_ipu_plane(plane); > + > + if (ipu_plane->disabling) { > + ipu_plane->disabling = false; > + ipu_plane_disable(ipu_plane, false); > + } > } > +EXPORT_SYMBOL_GPL(ipu_plane_disable_deferred); > > static void ipu_plane_destroy(struct drm_plane *plane) > { > @@ -356,7 +363,11 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, > static void ipu_plane_atomic_disable(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > - ipu_disable_plane(plane); > + struct ipu_plane *ipu_plane = to_ipu_plane(plane); > + > + if (ipu_plane->dp) > + ipu_dp_disable_channel(ipu_plane->dp, true); > + ipu_plane->disabling = true; > } > > static void ipu_plane_atomic_update(struct drm_plane *plane, > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h > index 338b88a74eb6e..0e2a723ff9816 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.h > +++ b/drivers/gpu/drm/imx/ipuv3-plane.h > @@ -23,6 +23,8 @@ struct ipu_plane { > > int dma; > int dp_flow; > + > + bool disabling; > }; > > struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, > @@ -42,4 +44,7 @@ void ipu_plane_put_resources(struct ipu_plane *plane); > > int ipu_plane_irq(struct ipu_plane *plane); > > +void ipu_plane_disable(struct ipu_plane *ipu_plane, bool disable_dp_channel); > +void ipu_plane_disable_deferred(struct drm_plane *plane); > + > #endif > diff --git a/drivers/gpu/ipu-v3/ipu-dp.c b/drivers/gpu/ipu-v3/ipu-dp.c > index 0e09c98248a0d..9b2b3fa479c46 100644 > --- a/drivers/gpu/ipu-v3/ipu-dp.c > +++ b/drivers/gpu/ipu-v3/ipu-dp.c > @@ -277,9 +277,6 @@ void ipu_dp_disable_channel(struct ipu_dp *dp, bool sync) > writel(0, flow->base + DP_FG_POS); > ipu_srm_dp_update(priv->ipu, sync); > > - if (ipu_idmac_channel_busy(priv->ipu, IPUV3_CHANNEL_MEM_BG_SYNC)) > - ipu_wait_interrupt(priv->ipu, IPU_IRQ_DP_SF_END, 50); > - > mutex_unlock(&priv->mutex); > } > EXPORT_SYMBOL_GPL(ipu_dp_disable_channel);
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 0a5e4fbb906bf..94f9c25e1c67b 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -30,6 +30,7 @@ #include <video/imx-ipu-v3.h> #include "imx-drm.h" +#include "ipuv3-plane.h" #define MAX_CRTC 4 @@ -160,6 +161,10 @@ static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = { static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; + struct drm_plane *plane; + struct drm_plane_state *plane_state; + bool plane_disabling = false; + int i; drm_atomic_helper_commit_modeset_disables(dev, state); @@ -169,6 +174,19 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state) drm_atomic_helper_commit_modeset_enables(dev, state); + for_each_plane_in_state(state, plane, plane_state, i) { + if (drm_atomic_plane_disabling(plane, plane_state)) + plane_disabling = true; + } + + if (plane_disabling) { + drm_atomic_helper_wait_for_vblanks(dev, state); + + for_each_plane_in_state(state, plane, plane_state, i) + ipu_plane_disable_deferred(plane); + + } + drm_atomic_helper_commit_hw_done(state); } diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c index 6be515a9fb694..0f15f11f26e0c 100644 --- a/drivers/gpu/drm/imx/ipuv3-crtc.c +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c @@ -60,6 +60,26 @@ static void ipu_crtc_enable(struct drm_crtc *crtc) ipu_di_enable(ipu_crtc->di); } +static void ipu_crtc_disable_planes(struct ipu_crtc *ipu_crtc, + struct drm_crtc_state *old_crtc_state) +{ + bool disable_partial = false; + bool disable_full = false; + struct drm_plane *plane; + + drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) { + if (plane == &ipu_crtc->plane[0]->base) + disable_full = true; + if (&ipu_crtc->plane[1] && plane == &ipu_crtc->plane[1]->base) + disable_partial = true; + } + + if (disable_partial) + ipu_plane_disable(ipu_crtc->plane[1], true); + if (disable_full) + ipu_plane_disable(ipu_crtc->plane[0], false); +} + static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -73,7 +93,7 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, * attached IDMACs will be left in undefined state, possibly hanging * the IPU or even system. */ - drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); + ipu_crtc_disable_planes(ipu_crtc, old_crtc_state); ipu_dc_disable(ipu); spin_lock_irq(&crtc->dev->event_lock); diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 55991d46ced50..a37735298615e 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -172,23 +172,30 @@ static void ipu_plane_enable(struct ipu_plane *ipu_plane) ipu_dp_enable_channel(ipu_plane->dp); } -static int ipu_disable_plane(struct drm_plane *plane) +void ipu_plane_disable(struct ipu_plane *ipu_plane, bool disable_dp_channel) { - struct ipu_plane *ipu_plane = to_ipu_plane(plane); - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); ipu_idmac_wait_busy(ipu_plane->ipu_ch, 50); - if (ipu_plane->dp) - ipu_dp_disable_channel(ipu_plane->dp, true); + if (ipu_plane->dp && disable_dp_channel) + ipu_dp_disable_channel(ipu_plane->dp, false); ipu_idmac_disable_channel(ipu_plane->ipu_ch); ipu_dmfc_disable_channel(ipu_plane->dmfc); if (ipu_plane->dp) ipu_dp_disable(ipu_plane->ipu); +} - return 0; +void ipu_plane_disable_deferred(struct drm_plane *plane) +{ + struct ipu_plane *ipu_plane = to_ipu_plane(plane); + + if (ipu_plane->disabling) { + ipu_plane->disabling = false; + ipu_plane_disable(ipu_plane, false); + } } +EXPORT_SYMBOL_GPL(ipu_plane_disable_deferred); static void ipu_plane_destroy(struct drm_plane *plane) { @@ -356,7 +363,11 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, static void ipu_plane_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { - ipu_disable_plane(plane); + struct ipu_plane *ipu_plane = to_ipu_plane(plane); + + if (ipu_plane->dp) + ipu_dp_disable_channel(ipu_plane->dp, true); + ipu_plane->disabling = true; } static void ipu_plane_atomic_update(struct drm_plane *plane, diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h index 338b88a74eb6e..0e2a723ff9816 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.h +++ b/drivers/gpu/drm/imx/ipuv3-plane.h @@ -23,6 +23,8 @@ struct ipu_plane { int dma; int dp_flow; + + bool disabling; }; struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, @@ -42,4 +44,7 @@ void ipu_plane_put_resources(struct ipu_plane *plane); int ipu_plane_irq(struct ipu_plane *plane); +void ipu_plane_disable(struct ipu_plane *ipu_plane, bool disable_dp_channel); +void ipu_plane_disable_deferred(struct drm_plane *plane); + #endif diff --git a/drivers/gpu/ipu-v3/ipu-dp.c b/drivers/gpu/ipu-v3/ipu-dp.c index 0e09c98248a0d..9b2b3fa479c46 100644 --- a/drivers/gpu/ipu-v3/ipu-dp.c +++ b/drivers/gpu/ipu-v3/ipu-dp.c @@ -277,9 +277,6 @@ void ipu_dp_disable_channel(struct ipu_dp *dp, bool sync) writel(0, flow->base + DP_FG_POS); ipu_srm_dp_update(priv->ipu, sync); - if (ipu_idmac_channel_busy(priv->ipu, IPUV3_CHANNEL_MEM_BG_SYNC)) - ipu_wait_interrupt(priv->ipu, IPU_IRQ_DP_SF_END, 50); - mutex_unlock(&priv->mutex); } EXPORT_SYMBOL_GPL(ipu_dp_disable_channel);
The DP (display processor) channel disable code tried to busy wait for the DP sync flow end interrupt status bit when disabling the partial plane without a full modeset. That never worked reliably, and it was disabled completely by the recent "gpu: ipu-v3: remove IRQ dance on DC channel disable" patch, causing ipu_wait_interrupt to always time out after 50 ms, which in turn would trigger a timeout in drm_atomic_helper_wait_for_vblanks. This patch changes ipu_plane_atomic_disable to only queue a DP channel register update at the next frame boundary and set a flag, which can be done without any waiting whatsoever. The imx_drm_atomic_commit_tail then calls a new ipu_plane_disable_deferred function that does the actual IDMAC teardown of the planes that are flagged for deferred disabling, after waiting for the vblank. Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- Changes since v2: - Add missing export for ipu_plane_disable_deferred - Check if there are any planes to be disabled and only then wait for vblanks and call the deferred disable function --- drivers/gpu/drm/imx/imx-drm-core.c | 18 ++++++++++++++++++ drivers/gpu/drm/imx/ipuv3-crtc.c | 22 +++++++++++++++++++++- drivers/gpu/drm/imx/ipuv3-plane.c | 25 ++++++++++++++++++------- drivers/gpu/drm/imx/ipuv3-plane.h | 5 +++++ drivers/gpu/ipu-v3/ipu-dp.c | 3 --- 5 files changed, 62 insertions(+), 11 deletions(-)