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(-)
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);
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);
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;
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);
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() ?
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)
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);
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; }
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!
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.
dri-devel@lists.freedesktop.org
-
Philipp Zabel -
Russell King - ARM Linux