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@xxxxxxxxxxxxxx>
Reviewed-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> 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);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
