diff mbox

[v3] drm/exynos: dsi: check whether dsi is enabled before sending data

Message ID 1433819945-7346-1-git-send-email-human.hwang@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hyungwon Hwang June 9, 2015, 3:19 a.m. UTC 
exynos_dsi_host_transfer() can be called through a panel driver while
DSI is turning down. It is possible because the function checks only
whether DSI is initialized or not, and there is a moment which DSI is
set by uninitialized, but DSI is still turning down. To prevent it,
DSI must be set by disabled before starting to be turned down, and
exynos_dsi_host_transfer() must check whether DSI is enabled or not.

Kernel dump:
[ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 0xffffff800015e018
[ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP
[ 4721.352031] Modules linked in:
[ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: G        W       4.0.4-01017-g7964a87 #1
[ 4721.353989] Hardware name: Samsung DRACO board (DT)
[ 4721.358852] task: ffffffc0a0b70000 ti: ffffffc0a00ec000 task.ti: ffffffc0a00ec000
[ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c
[ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8
[ 4721.376731] pc : [<ffffffc000432bcc>] lr : [<ffffffc000434590>] pstate: 60000145
[ 4721.384107] sp : ffffffc0a00efbe0
[ 4721.387405] x29: ffffffc0a00efbe0 x28: ffffffc0a00ec000
[ 4721.392699] x27: ffffffc000968000 x26: 0000000000000040
[ 4721.397994] x25: ffffffc000f74dc0 x24: ffffffc0a00efec8
[ 4721.403290] x23: ffffffc0a4815400 x22: ffffffc0009f2729
[ 4721.408584] x21: ffffffc0a00efcc8 x20: ffffffc0a4a2a848
[ 4721.413879] x19: ffffffc0a4a2a818 x18: 0000000000000004
[ 4721.419173] x17: 0000007faa5cddf0 x16: ffffffc0001a40a8
[ 4721.424469] x15: 0000000000000009 x14: 000000000000000d
[ 4721.429762] x13: 6e6e6f63206b726f x12: 0000000000000010
[ 4721.435058] x11: 0101010101010101 x10: 0000000000000000
[ 4721.440353] x9 : 000000000000000a x8 : 8386838282818381
[ 4721.445648] x7 : ffffffc0a201efe8 x6 : 0000000000000000
[ 4721.450943] x5 : 00000000fffffffa x4 : ffffffc0a201f170
[ 4721.456237] x3 : ffffff800015e000 x2 : ffffff800015e018
[ 4721.461531] x1 : 000000000000000f x0 : ffffffc0a4a2a818
[ 4721.466826]
[ 4721.468305] Process deviced (pid: 300, stack limit = 0xffffffc0a00ec028)
[ 4721.474989] Stack: (0xffffffc0a00efbe0 to 0xffffffc0a00f0000)
[ 4721.480720] fbe0: a00efca0 ffffffc0 0042c944 ffffffc0 a0f2d680 ffffffc0 00000024 00000000
[ 4721.488895] fc00: a4b6d000 ffffffc0 009f2729 ffffffc0 a4815400 ffffffc0 a00efec8 ffffffc0

Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
---
Changes for v2:
 - Previous version of this patch makes a problem in initializing the DSI. This
   patch fixes it, by moving the point which DSI is set by enabled to the
   point before drm_panel_prepare() called. This is where the setting must
   be done, because to call drm_panel_prepare() successfully, DSI must be
   enabled. Also this patch adds the condition to TE irq handler. DSI must be
   enabled and initialized, not just enabled before calling te_handler in
   the display driver.
Changes for v3:
 - New state DSIM_STATE_VIDOUT_AVAILABLE for representing video output
   availability is introduced. Because DSIM_STATE_ENABLED becomes to
   represent whether DSI can be used for data transfer or not, this
   state can be used for checking whether video ouput is available or
   not anymore. So new state have to be introduced. The stete
   DSIM_STATE_VIDOUT_AVAILABLE represents whether DSI is prepared for
   outputting video to a panel or not.

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

--
1.9.1

Comments

Inki Dae June 11, 2015, 2:52 p.m. UTC | #1 
On 2015? 06? 09? 12:19, Hyungwon Hwang wrote:
> exynos_dsi_host_transfer() can be called through a panel driver while
> DSI is turning down. It is possible because the function checks only
> whether DSI is initialized or not, and there is a moment which DSI is
> set by uninitialized, but DSI is still turning down. To prevent it,
> DSI must be set by disabled before starting to be turned down, and
> exynos_dsi_host_transfer() must check whether DSI is enabled or not.

Applied.

Thanks,
Inki Dae

> 
> Kernel dump:
> [ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 0xffffff800015e018
> [ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP
> [ 4721.352031] Modules linked in:
> [ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: G        W       4.0.4-01017-g7964a87 #1
> [ 4721.353989] Hardware name: Samsung DRACO board (DT)
> [ 4721.358852] task: ffffffc0a0b70000 ti: ffffffc0a00ec000 task.ti: ffffffc0a00ec000
> [ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c
> [ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8
> [ 4721.376731] pc : [<ffffffc000432bcc>] lr : [<ffffffc000434590>] pstate: 60000145
> [ 4721.384107] sp : ffffffc0a00efbe0
> [ 4721.387405] x29: ffffffc0a00efbe0 x28: ffffffc0a00ec000
> [ 4721.392699] x27: ffffffc000968000 x26: 0000000000000040
> [ 4721.397994] x25: ffffffc000f74dc0 x24: ffffffc0a00efec8
> [ 4721.403290] x23: ffffffc0a4815400 x22: ffffffc0009f2729
> [ 4721.408584] x21: ffffffc0a00efcc8 x20: ffffffc0a4a2a848
> [ 4721.413879] x19: ffffffc0a4a2a818 x18: 0000000000000004
> [ 4721.419173] x17: 0000007faa5cddf0 x16: ffffffc0001a40a8
> [ 4721.424469] x15: 0000000000000009 x14: 000000000000000d
> [ 4721.429762] x13: 6e6e6f63206b726f x12: 0000000000000010
> [ 4721.435058] x11: 0101010101010101 x10: 0000000000000000
> [ 4721.440353] x9 : 000000000000000a x8 : 8386838282818381
> [ 4721.445648] x7 : ffffffc0a201efe8 x6 : 0000000000000000
> [ 4721.450943] x5 : 00000000fffffffa x4 : ffffffc0a201f170
> [ 4721.456237] x3 : ffffff800015e000 x2 : ffffff800015e018
> [ 4721.461531] x1 : 000000000000000f x0 : ffffffc0a4a2a818
> [ 4721.466826]
> [ 4721.468305] Process deviced (pid: 300, stack limit = 0xffffffc0a00ec028)
> [ 4721.474989] Stack: (0xffffffc0a00efbe0 to 0xffffffc0a00f0000)
> [ 4721.480720] fbe0: a00efca0 ffffffc0 0042c944 ffffffc0 a0f2d680 ffffffc0 00000024 00000000
> [ 4721.488895] fc00: a4b6d000 ffffffc0 009f2729 ffffffc0 a4815400 ffffffc0 a00efec8 ffffffc0
> 
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> ---
> Changes for v2:
>  - Previous version of this patch makes a problem in initializing the DSI. This
>    patch fixes it, by moving the point which DSI is set by enabled to the
>    point before drm_panel_prepare() called. This is where the setting must
>    be done, because to call drm_panel_prepare() successfully, DSI must be
>    enabled. Also this patch adds the condition to TE irq handler. DSI must be
>    enabled and initialized, not just enabled before calling te_handler in
>    the display driver.
> Changes for v3:
>  - New state DSIM_STATE_VIDOUT_AVAILABLE for representing video output
>    availability is introduced. Because DSIM_STATE_ENABLED becomes to
>    represent whether DSI can be used for data transfer or not, this
>    state can be used for checking whether video ouput is available or
>    not anymore. So new state have to be introduced. The stete
>    DSIM_STATE_VIDOUT_AVAILABLE represents whether DSI is prepared for
>    outputting video to a panel or not.
> 
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 1cfc4be07..9250356 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -241,6 +241,7 @@ struct exynos_dsi_transfer {
>  #define DSIM_STATE_ENABLED		BIT(0)
>  #define DSIM_STATE_INITIALIZED		BIT(1)
>  #define DSIM_STATE_CMD_LPM		BIT(2)
> +#define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)
> 
>  struct exynos_dsi_driver_data {
>  	unsigned int *regs;
> @@ -1268,7 +1269,7 @@ static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
>  	struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
>  	struct drm_encoder *encoder = dsi->display.encoder;
> 
> -	if (dsi->state & DSIM_STATE_ENABLED)
> +	if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE)
>  		exynos_drm_crtc_te_handler(encoder->crtc);
> 
>  	return IRQ_HANDLED;
> @@ -1408,6 +1409,9 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>  	struct exynos_dsi_transfer xfer;
>  	int ret;
> 
> +	if (!(dsi->state & DSIM_STATE_ENABLED))
> +		return -EINVAL;
> +
>  	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>  		ret = exynos_dsi_init(dsi);
>  		if (ret)
> @@ -1520,8 +1524,11 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>  	if (ret < 0)
>  		return ret;
> 
> +	dsi->state |= DSIM_STATE_ENABLED;
> +
>  	ret = drm_panel_prepare(dsi->panel);
>  	if (ret < 0) {
> +		dsi->state &= ~DSIM_STATE_ENABLED;
>  		exynos_dsi_poweroff(dsi);
>  		return ret;
>  	}
> @@ -1529,8 +1536,6 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>  	exynos_dsi_set_display_mode(dsi);
>  	exynos_dsi_set_display_enable(dsi, true);
> 
> -	dsi->state |= DSIM_STATE_ENABLED;
> -
>  	ret = drm_panel_enable(dsi->panel);
>  	if (ret < 0) {
>  		dsi->state &= ~DSIM_STATE_ENABLED;
> @@ -1540,6 +1545,8 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>  		return ret;
>  	}
> 
> +	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> +
>  	return 0;
>  }
> 
> @@ -1548,12 +1555,15 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
>  	if (!(dsi->state & DSIM_STATE_ENABLED))
>  		return;
> 
> +	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> +
>  	drm_panel_disable(dsi->panel);
>  	exynos_dsi_set_display_enable(dsi, false);
>  	drm_panel_unprepare(dsi->panel);
> -	exynos_dsi_poweroff(dsi);
> 
>  	dsi->state &= ~DSIM_STATE_ENABLED;
> +
> +	exynos_dsi_poweroff(dsi);
>  }
> 
>  static void exynos_dsi_dpms(struct exynos_drm_display *display, int mode)
> --
> 1.9.1
> 
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1cfc4be07..9250356 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -241,6 +241,7 @@  struct exynos_dsi_transfer {
 #define DSIM_STATE_ENABLED		BIT(0)
 #define DSIM_STATE_INITIALIZED		BIT(1)
 #define DSIM_STATE_CMD_LPM		BIT(2)
+#define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)

 struct exynos_dsi_driver_data {
 	unsigned int *regs;
@@ -1268,7 +1269,7 @@  static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
 	struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
 	struct drm_encoder *encoder = dsi->display.encoder;

-	if (dsi->state & DSIM_STATE_ENABLED)
+	if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE)
 		exynos_drm_crtc_te_handler(encoder->crtc);

 	return IRQ_HANDLED;
@@ -1408,6 +1409,9 @@  static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
 	struct exynos_dsi_transfer xfer;
 	int ret;

+	if (!(dsi->state & DSIM_STATE_ENABLED))
+		return -EINVAL;
+
 	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
 		ret = exynos_dsi_init(dsi);
 		if (ret)
@@ -1520,8 +1524,11 @@  static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;

+	dsi->state |= DSIM_STATE_ENABLED;
+
 	ret = drm_panel_prepare(dsi->panel);
 	if (ret < 0) {
+		dsi->state &= ~DSIM_STATE_ENABLED;
 		exynos_dsi_poweroff(dsi);
 		return ret;
 	}
@@ -1529,8 +1536,6 @@  static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);

-	dsi->state |= DSIM_STATE_ENABLED;
-
 	ret = drm_panel_enable(dsi->panel);
 	if (ret < 0) {
 		dsi->state &= ~DSIM_STATE_ENABLED;
@@ -1540,6 +1545,8 @@  static int exynos_dsi_enable(struct exynos_dsi *dsi)
 		return ret;
 	}

+	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
+
 	return 0;
 }

@@ -1548,12 +1555,15 @@  static void exynos_dsi_disable(struct exynos_dsi *dsi)
 	if (!(dsi->state & DSIM_STATE_ENABLED))
 		return;

+	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
+
 	drm_panel_disable(dsi->panel);
 	exynos_dsi_set_display_enable(dsi, false);
 	drm_panel_unprepare(dsi->panel);
-	exynos_dsi_poweroff(dsi);

 	dsi->state &= ~DSIM_STATE_ENABLED;
+
+	exynos_dsi_poweroff(dsi);
 }

 static void exynos_dsi_dpms(struct exynos_drm_display *display, int mode)