[PATCH 0/7] Reorder i.MX IPU display enable/disable sequence

[PATCH 0/7] Reorder i.MX IPU display enable/disable sequence

[Philipp Zabel]

Repeatedly enabling and disabling the display currently can lead to a state
in which the IPU doesn't produce a valid signal anymore because we disable
IPU submodules before they can finish their interaction.

This series reorders the enable/disable sequence so that we first wait for the
DC/DP to finish processing the current frame, then stop the DI and IDMAC, and
only then disable clocks to the submodules. Also from now on we disable the DC
when it is not in use.

regards
Philipp

Philipp Zabel (7):
  imx-drm: ipu-common: add ipu_map_irq to request non-IDMAC interrupts
  imx-drm: ipu-common: Add helpers to check for a busy IDMAC channel and
    to busywait for an interrupt
  imx-drm: ipu-dmfc: Wait for FIFOs to run empty before disabling
  imx-drm: ipu-dc: Wait for DC_FC_1 / DP_SF_END interrupt
  imx-drm: ipu-dp: Split disabling the DP foreground channel from
    disabling the DP module
  imx-drm: imx-dp: When disabling the DP foreground channel, wait until
    the DP background channel is finished before disabling the IDMAC
    channel
  imx-drm: ipuv3-crtc: Change display enable/disable order

 drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |  3 ++
 drivers/staging/imx-drm/ipu-v3/ipu-common.c | 41 ++++++++++++++---
 drivers/staging/imx-drm/ipu-v3/ipu-dc.c     | 71 ++++++++++++++++++++---------
 drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c   | 25 +++++++++-
 drivers/staging/imx-drm/ipu-v3/ipu-dp.c     | 71 +++++++++++++++++++----------
 drivers/staging/imx-drm/ipu-v3/ipu-prv.h    |  3 ++
 drivers/staging/imx-drm/ipuv3-crtc.c        | 10 ++--
 drivers/staging/imx-drm/ipuv3-plane.c       |  4 ++
 8 files changed, 173 insertions(+), 55 deletions(-)

-- 
1.9.1

[PATCH 1/7] imx-drm: ipu-common: add ipu_map_irq to request non-IDMAC interrupts

[Philipp Zabel]

This allows to request the DC related interrupts.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |  1 +
 drivers/staging/imx-drm/ipu-v3/ipu-common.c | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
index c4d14ea..2966e42 100644
--- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
+++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
@@ -76,6 +76,7 @@ enum ipu_channel_irq {
 	IPU_IRQ_EOS = 192,
 };
 
+int ipu_map_irq(struct ipu_soc *ipu, int irq);
 int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
 		enum ipu_channel_irq irq);
 
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
index ca85d3d..0585517 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
@@ -933,15 +933,22 @@ static void ipu_err_irq_handler(unsigned int irq, struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
-		enum ipu_channel_irq irq_type)
+int ipu_map_irq(struct ipu_soc *ipu, int irq)
 {
-	int irq = irq_linear_revmap(ipu->domain, irq_type + channel->num);
+	int virq;
 
-	if (!irq)
-		irq = irq_create_mapping(ipu->domain, irq_type + channel->num);
+	virq = irq_linear_revmap(ipu->domain, irq);
+	if (!virq)
+		virq = irq_create_mapping(ipu->domain, irq);
+
+	return virq;
+}
+EXPORT_SYMBOL_GPL(ipu_map_irq);
 
-	return irq;
+int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
+		enum ipu_channel_irq irq_type)
+{
+	return ipu_map_irq(ipu, irq_type + channel->num);
 }
 EXPORT_SYMBOL_GPL(ipu_idmac_channel_irq);
 
-- 
1.9.1

[PATCH 2/7] imx-drm: ipu-common: Add helpers to check for a busy IDMAC channel and to busywait for an interrupt

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/ipu-common.c | 22 ++++++++++++++++++++++
 drivers/staging/imx-drm/ipu-v3/ipu-prv.h    |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
index 0585517..8fb4c20 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
@@ -697,6 +697,12 @@ int ipu_idmac_enable_channel(struct ipuv3_channel *channel)
 }
 EXPORT_SYMBOL_GPL(ipu_idmac_enable_channel);
 
+bool ipu_idmac_channel_busy(struct ipu_soc *ipu, unsigned int chno)
+{
+	return (ipu_idmac_read(ipu, IDMAC_CHA_BUSY(chno)) & idma_mask(chno));
+}
+EXPORT_SYMBOL_GPL(ipu_idmac_channel_busy);
+
 int ipu_idmac_wait_busy(struct ipuv3_channel *channel, int ms)
 {
 	struct ipu_soc *ipu = channel->ipu;
@@ -714,6 +720,22 @@ int ipu_idmac_wait_busy(struct ipuv3_channel *channel, int ms)
 }
 EXPORT_SYMBOL_GPL(ipu_idmac_wait_busy);
 
+int ipu_wait_interrupt(struct ipu_soc *ipu, int irq, int ms)
+{
+	unsigned long timeout;
+
+	timeout = jiffies + msecs_to_jiffies(ms);
+	ipu_cm_write(ipu, BIT(irq % 32), IPU_INT_STAT(irq / 32));
+	while (!(ipu_cm_read(ipu, IPU_INT_STAT(irq / 32) & BIT(irq % 32)))) {
+		if (time_after(jiffies, timeout))
+			return -ETIMEDOUT;
+		cpu_relax();
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_wait_interrupt);
+
 int ipu_idmac_disable_channel(struct ipuv3_channel *channel)
 {
 	struct ipu_soc *ipu = channel->ipu;
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-prv.h b/drivers/staging/imx-drm/ipu-v3/ipu-prv.h
index 4df0050..bfc1b33 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-prv.h
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-prv.h
@@ -185,6 +185,9 @@ void ipu_srm_dp_sync_update(struct ipu_soc *ipu);
 int ipu_module_enable(struct ipu_soc *ipu, u32 mask);
 int ipu_module_disable(struct ipu_soc *ipu, u32 mask);
 
+bool ipu_idmac_channel_busy(struct ipu_soc *ipu, unsigned int chno);
+int ipu_wait_interrupt(struct ipu_soc *ipu, int irq, int ms);
+
 int ipu_di_init(struct ipu_soc *ipu, struct device *dev, int id,
 		unsigned long base, u32 module, struct clk *ipu_clk);
 void ipu_di_exit(struct ipu_soc *ipu, int id);
-- 
1.9.1

[PATCH 3/7] imx-drm: ipu-dmfc: Wait for FIFOs to run empty before disabling

[Philipp Zabel]

Disabling the DMFC module while there is still data in the FIFOs could
cause the "new frame before end of frame" error state when the DMFC is
enabled again.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
index 4521301..59f182b 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
@@ -28,7 +28,12 @@
 #define DMFC_GENERAL1		0x0014
 #define DMFC_GENERAL2		0x0018
 #define DMFC_IC_CTRL		0x001c
-#define DMFC_STAT		0x0020
+#define DMFC_WR_CHAN_ALT	0x0020
+#define DMFC_WR_CHAN_DEF_ALT	0x0024
+#define DMFC_DP_CHAN_ALT	0x0028
+#define DMFC_DP_CHAN_DEF_ALT	0x002c
+#define DMFC_GENERAL1_ALT	0x0030
+#define DMFC_STAT		0x0034
 
 #define DMFC_WR_CHAN_1_28		0
 #define DMFC_WR_CHAN_2_41		8
@@ -133,6 +138,20 @@ int ipu_dmfc_enable_channel(struct dmfc_channel *dmfc)
 }
 EXPORT_SYMBOL_GPL(ipu_dmfc_enable_channel);
 
+static void ipu_dmfc_wait_fifos(struct ipu_dmfc_priv *priv)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
+
+	while ((readl(priv->base + DMFC_STAT) & 0x02fff000) != 0x02fff000) {
+		if (time_after(jiffies, timeout)) {
+			dev_warn(priv->dev,
+				 "Timeout waiting for DMFC FIFOs to clear\n");
+			break;
+		}
+		cpu_relax();
+	}
+}
+
 void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc)
 {
 	struct ipu_dmfc_priv *priv = dmfc->priv;
@@ -141,8 +160,10 @@ void ipu_dmfc_disable_channel(struct dmfc_channel *dmfc)
 
 	priv->use_count--;
 
-	if (!priv->use_count)
+	if (!priv->use_count) {
+		ipu_dmfc_wait_fifos(priv);
 		ipu_module_disable(priv->ipu, IPU_CONF_DMFC_EN);
+	}
 
 	if (priv->use_count < 0)
 		priv->use_count = 0;
-- 
1.9.1

[PATCH 4/7] imx-drm: ipu-dc: Wait for DC_FC_1 / DP_SF_END interrupt

[Philipp Zabel]

Wait for the DC Frame Complete or DP Sync Flow End interrupts
before disabling DC channels.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/ipu-dc.c | 71 +++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d5de8bb..13b7538 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -18,6 +18,7 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 
 #include "../imx-drm.h"
@@ -110,6 +111,9 @@ struct ipu_dc_priv {
 	struct device		*dev;
 	struct ipu_dc		channels[IPU_DC_NUM_CHANNELS];
 	struct mutex		mutex;
+	struct completion	comp;
+	int			dc_irq;
+	int			dp_irq;
 };
 
 static void dc_link_event(struct ipu_dc *dc, int event, int addr, int priority)
@@ -239,38 +243,46 @@ void ipu_dc_enable_channel(struct ipu_dc *dc)
 }
 EXPORT_SYMBOL_GPL(ipu_dc_enable_channel);
 
+static irqreturn_t dc_irq_handler(int irq, void *dev_id)
+{
+	struct ipu_dc *dc = dev_id;
+	u32 reg;
+
+	reg = readl(dc->base + DC_WR_CH_CONF);
+	reg &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
+	writel(reg, dc->base + DC_WR_CH_CONF);
+
+	/* The Freescale BSP kernel clears DIx_COUNTER_RELEASE here */
+
+	disable_irq(irq);
+	complete(&dc->priv->comp);
+	return IRQ_HANDLED;
+}
+
 void ipu_dc_disable_channel(struct ipu_dc *dc)
 {
 	struct ipu_dc_priv *priv = dc->priv;
+	int irq, ret;
 	u32 val;
-	int irq = 0, timeout = 50;
 
+	/* TODO: Handle MEM_FG_SYNC differently from MEM_BG_SYNC */
 	if (dc->chno == 1)
-		irq = IPU_IRQ_DC_FC_1;
+		irq = priv->dc_irq;
 	else if (dc->chno == 5)
-		irq = IPU_IRQ_DP_SF_END;
+		irq = priv->dp_irq;
 	else
 		return;
 
-	/* should wait for the interrupt here */
-	mdelay(50);
+	init_completion(&priv->comp);
+	enable_irq(irq);
+	ret = wait_for_completion_timeout(&priv->comp, msecs_to_jiffies(50));
+	if (ret <= 0) {
+		dev_warn(priv->dev, "DC stop timeout after 50 ms\n");
 
-	if (dc->di == 0)
-		val = 0x00000002;
-	else
-		val = 0x00000020;
-
-	/* Wait for DC triple buffer to empty */
-	while ((readl(priv->dc_reg + DC_STAT) & val) != val) {
-		usleep_range(2000, 20000);
-		timeout -= 2;
-		if (timeout <= 0)
-			break;
+		val = readl(dc->base + DC_WR_CH_CONF);
+		val &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
+		writel(val, dc->base + DC_WR_CH_CONF);
 	}
-
-	val = readl(dc->base + DC_WR_CH_CONF);
-	val &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
-	writel(val, dc->base + DC_WR_CH_CONF);
 }
 EXPORT_SYMBOL_GPL(ipu_dc_disable_channel);
 
@@ -340,7 +352,7 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
 	struct ipu_dc_priv *priv;
 	static int channel_offsets[] = { 0, 0x1c, 0x38, 0x54, 0x58, 0x5c,
 		0x78, 0, 0x94, 0xb4};
-	int i;
+	int i, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -361,6 +373,23 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
 		priv->channels[i].base = priv->dc_reg + channel_offsets[i];
 	}
 
+	priv->dc_irq = ipu_map_irq(ipu, IPU_IRQ_DC_FC_1);
+	if (!priv->dc_irq)
+		return -EINVAL;
+	ret = devm_request_irq(dev, priv->dc_irq, dc_irq_handler, 0, NULL,
+			       &priv->channels[1]);
+	if (ret < 0)
+		return ret;
+	disable_irq(priv->dc_irq);
+	priv->dp_irq = ipu_map_irq(ipu, IPU_IRQ_DP_SF_END);
+	if (!priv->dp_irq)
+		return -EINVAL;
+	ret = devm_request_irq(dev, priv->dp_irq, dc_irq_handler, 0, NULL,
+			       &priv->channels[5]);
+	if (ret < 0)
+		return ret;
+	disable_irq(priv->dp_irq);
+
 	writel(DC_WR_CH_CONF_WORD_SIZE_24 | DC_WR_CH_CONF_DISP_ID_PARALLEL(1) |
 			DC_WR_CH_CONF_PROG_DI_ID,
 			priv->channels[1].base + DC_WR_CH_CONF);
-- 
1.9.1

Re: [PATCH 4/7] imx-drm: ipu-dc: Wait for DC_FC_1 / DP_SF_END interrupt

[Russell King - ARM Linux]

On Mon, Apr 14, 2014 at 05:21:32PM +0200, Philipp Zabel wrote:
> Wait for the DC Frame Complete or DP Sync Flow End interrupts
> before disabling DC channels.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/staging/imx-drm/ipu-v3/ipu-dc.c | 71 +++++++++++++++++++++++----------
>  1 file changed, 50 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> index d5de8bb..13b7538 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> @@ -18,6 +18,7 @@
>  #include <linux/types.h>
>  #include <linux/errno.h>
>  #include <linux/delay.h>
> +#include <linux/interrupt.h>
>  #include <linux/io.h>
>  
>  #include "../imx-drm.h"
> @@ -110,6 +111,9 @@ struct ipu_dc_priv {
>  	struct device		*dev;
>  	struct ipu_dc		channels[IPU_DC_NUM_CHANNELS];
>  	struct mutex		mutex;
> +	struct completion	comp;
> +	int			dc_irq;
> +	int			dp_irq;
>  };
>  
>  static void dc_link_event(struct ipu_dc *dc, int event, int addr, int priority)
> @@ -239,38 +243,46 @@ void ipu_dc_enable_channel(struct ipu_dc *dc)
>  }
>  EXPORT_SYMBOL_GPL(ipu_dc_enable_channel);
>  
> +static irqreturn_t dc_irq_handler(int irq, void *dev_id)
> +{
> +	struct ipu_dc *dc = dev_id;
> +	u32 reg;
> +
> +	reg = readl(dc->base + DC_WR_CH_CONF);
> +	reg &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
> +	writel(reg, dc->base + DC_WR_CH_CONF);
> +
> +	/* The Freescale BSP kernel clears DIx_COUNTER_RELEASE here */
> +
> +	disable_irq(irq);

Shouldn't this be disable_irq_nosync() ?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 5/7] imx-drm: ipu-dp: Split disabling the DP foreground channel from disabling the DP module

[Philipp Zabel]

The former has to be done before disabling the DMFC, the latter has to be
done afterwards. Otherwise the DMFC FIFOs never get cleared properly.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |  2 +
 drivers/staging/imx-drm/ipu-v3/ipu-dp.c     | 68 +++++++++++++++++++----------
 drivers/staging/imx-drm/ipuv3-plane.c       |  4 ++
 3 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
index 2966e42..8678ad1 100644
--- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
+++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
@@ -153,8 +153,10 @@ void ipu_dmfc_put(struct dmfc_channel *dmfc);
 
 struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow);
 void ipu_dp_put(struct ipu_dp *);
+int ipu_dp_enable(struct ipu_soc *ipu);
 int ipu_dp_enable_channel(struct ipu_dp *dp);
 void ipu_dp_disable_channel(struct ipu_dp *dp);
+void ipu_dp_disable(struct ipu_soc *ipu);
 int ipu_dp_setup_channel(struct ipu_dp *dp,
 		enum ipu_color_space in, enum ipu_color_space out);
 int ipu_dp_set_window_pos(struct ipu_dp *, u16 x_pos, u16 y_pos);
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 58f87c8..6980fa1 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -215,10 +215,9 @@ int ipu_dp_setup_channel(struct ipu_dp *dp,
 }
 EXPORT_SYMBOL_GPL(ipu_dp_setup_channel);
 
-int ipu_dp_enable_channel(struct ipu_dp *dp)
+int ipu_dp_enable(struct ipu_soc *ipu)
 {
-	struct ipu_flow *flow = to_flow(dp);
-	struct ipu_dp_priv *priv = flow->priv;
+	struct ipu_dp_priv *priv = ipu->dp_priv;
 
 	mutex_lock(&priv->mutex);
 
@@ -227,15 +226,28 @@ int ipu_dp_enable_channel(struct ipu_dp *dp)
 
 	priv->use_count++;
 
-	if (dp->foreground) {
-		u32 reg;
+	mutex_unlock(&priv->mutex);
 
-		reg = readl(flow->base + DP_COM_CONF);
-		reg |= DP_COM_CONF_FG_EN;
-		writel(reg, flow->base + DP_COM_CONF);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_dp_enable);
 
-		ipu_srm_dp_sync_update(priv->ipu);
-	}
+int ipu_dp_enable_channel(struct ipu_dp *dp)
+{
+	struct ipu_flow *flow = to_flow(dp);
+	struct ipu_dp_priv *priv = flow->priv;
+	u32 reg;
+
+	if (!dp->foreground)
+		return 0;
+
+	mutex_lock(&priv->mutex);
+
+	reg = readl(flow->base + DP_COM_CONF);
+	reg |= DP_COM_CONF_FG_EN;
+	writel(reg, flow->base + DP_COM_CONF);
+
+	ipu_srm_dp_sync_update(priv->ipu);
 
 	mutex_unlock(&priv->mutex);
 
@@ -247,25 +259,35 @@ void ipu_dp_disable_channel(struct ipu_dp *dp)
 {
 	struct ipu_flow *flow = to_flow(dp);
 	struct ipu_dp_priv *priv = flow->priv;
+	u32 reg, csc;
+
+	if (!dp->foreground)
+		return;
 
 	mutex_lock(&priv->mutex);
 
-	priv->use_count--;
+	reg = readl(flow->base + DP_COM_CONF);
+	csc = reg & DP_COM_CONF_CSC_DEF_MASK;
+	if (csc == DP_COM_CONF_CSC_DEF_FG)
+		reg &= ~DP_COM_CONF_CSC_DEF_MASK;
 
-	if (dp->foreground) {
-		u32 reg, csc;
+	reg &= ~DP_COM_CONF_FG_EN;
+	writel(reg, flow->base + DP_COM_CONF);
 
-		reg = readl(flow->base + DP_COM_CONF);
-		csc = reg & DP_COM_CONF_CSC_DEF_MASK;
-		if (csc == DP_COM_CONF_CSC_DEF_FG)
-			reg &= ~DP_COM_CONF_CSC_DEF_MASK;
+	writel(0, flow->base + DP_FG_POS);
+	ipu_srm_dp_sync_update(priv->ipu);
 
-		reg &= ~DP_COM_CONF_FG_EN;
-		writel(reg, flow->base + DP_COM_CONF);
+	mutex_unlock(&priv->mutex);
+}
+EXPORT_SYMBOL_GPL(ipu_dp_disable_channel);
 
-		writel(0, flow->base + DP_FG_POS);
-		ipu_srm_dp_sync_update(priv->ipu);
-	}
+void ipu_dp_disable(struct ipu_soc *ipu)
+{
+	struct ipu_dp_priv *priv = ipu->dp_priv;
+
+	mutex_lock(&priv->mutex);
+
+	priv->use_count--;
 
 	if (!priv->use_count)
 		ipu_module_disable(priv->ipu, IPU_CONF_DP_EN);
@@ -275,7 +297,7 @@ void ipu_dp_disable_channel(struct ipu_dp *dp)
 
 	mutex_unlock(&priv->mutex);
 }
-EXPORT_SYMBOL_GPL(ipu_dp_disable_channel);
+EXPORT_SYMBOL_GPL(ipu_dp_disable);
 
 struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow)
 {
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index 27a8d73..5697e59 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -239,6 +239,8 @@ err_out:
 
 void ipu_plane_enable(struct ipu_plane *ipu_plane)
 {
+	if (ipu_plane->dp)
+		ipu_dp_enable(ipu_plane->ipu);
 	ipu_dmfc_enable_channel(ipu_plane->dmfc);
 	ipu_idmac_enable_channel(ipu_plane->ipu_ch);
 	if (ipu_plane->dp)
@@ -257,6 +259,8 @@ void ipu_plane_disable(struct ipu_plane *ipu_plane)
 		ipu_dp_disable_channel(ipu_plane->dp);
 	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);
 }
 
 static void ipu_plane_dpms(struct ipu_plane *ipu_plane, int mode)
-- 
1.9.1

[PATCH 6/7] imx-drm: imx-dp: When disabling the DP foreground channel, wait until the DP background channel is finished before disabling the IDMAC channel

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 6980fa1..d90f82a 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -277,6 +277,9 @@ void ipu_dp_disable_channel(struct ipu_dp *dp)
 	writel(0, flow->base + DP_FG_POS);
 	ipu_srm_dp_sync_update(priv->ipu);
 
+	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);
-- 
1.9.1

[PATCH 7/7] imx-drm: ipuv3-crtc: Change display enable/disable order

[Philipp Zabel]

Now that ipu_dc_disable_channel correctly waits for the channel to finish,
we can reorder the enable/disable order to first stop the DC and DI and
only then disable the IDMAC. Enabling is done the other way around: IDMAC
first, then DC, then DI.

This avoids an issue where sometimes the channel would not correctly start,
leading to non-working LVDS displays.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/imx-drm/ipuv3-crtc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index c48f640..c771ac1 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -63,9 +63,11 @@ static void ipu_fb_enable(struct ipu_crtc *ipu_crtc)
 	if (ipu_crtc->enabled)
 		return;
 
-	ipu_di_enable(ipu_crtc->di);
-	ipu_dc_enable_channel(ipu_crtc->dc);
+	/* TODO: Enable DC module here, right now it is never disabled */
 	ipu_plane_enable(ipu_crtc->plane[0]);
+	/* Start DC channel and DI after IDMAC */
+	ipu_dc_enable_channel(ipu_crtc->dc);
+	ipu_di_enable(ipu_crtc->di);
 
 	ipu_crtc->enabled = 1;
 }
@@ -75,9 +77,11 @@ static void ipu_fb_disable(struct ipu_crtc *ipu_crtc)
 	if (!ipu_crtc->enabled)
 		return;
 
-	ipu_plane_disable(ipu_crtc->plane[0]);
+	/* Stop DC channel and DI before IDMAC */
 	ipu_dc_disable_channel(ipu_crtc->dc);
 	ipu_di_disable(ipu_crtc->di);
+	ipu_plane_disable(ipu_crtc->plane[0]);
+	/* TODO: Disable DC module here */
 
 	ipu_crtc->enabled = 0;
 }
-- 
1.9.1

Re: [PATCH 0/7] Reorder i.MX IPU display enable/disable sequence

[Russell King - ARM Linux]

On Mon, Apr 14, 2014 at 05:21:28PM +0200, Philipp Zabel wrote:
> Repeatedly enabling and disabling the display currently can lead to a state
> in which the IPU doesn't produce a valid signal anymore because we disable
> IPU submodules before they can finish their interaction.

Well done at finding the cause of this - I've encounted this several
times, while testing imx-hdmi and it's good that it should hopefully
be fixed!

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 0/7] Reorder i.MX IPU display enable/disable sequence

[Russell King - ARM Linux]

On Mon, Apr 14, 2014 at 05:21:28PM +0200, Philipp Zabel wrote:
> Repeatedly enabling and disabling the display currently can lead to a state
> in which the IPU doesn't produce a valid signal anymore because we disable
> IPU submodules before they can finish their interaction.

I'm afraid to say that after adding these patches, I find that when Xorg
starts, the thing appears to lock up.

Bisecting points at:

imx-drm: ipu-dc: Wait for DC_FC_1 / DP_SF_END interrupt

being the culpret - booting with the patches up to and including:

imx-drm: ipu-dmfc: Wait for FIFOs to run empty before disabling

works fine.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.