* [PATCH 1/6] drm/rockchip: vop: no need wait vblank on crtc enable
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
@ 2017-07-31 9:49 ` Mark Yao
2017-08-03 12:36 ` Sandy Huang
2017-07-31 9:49 ` [PATCH 2/6] drm/rockchip: vop: fix iommu page fault when resume Mark Yao
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:49 UTC (permalink / raw)
To: linux-arm-kernel
Since atomic framework, crtc enable and disable are in pairs,
no need to wait vblank.
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 -----------------------------
1 file changed, 36 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 1d42049..0bfd563 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -893,42 +893,6 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
return;
}
- /*
- * If dclk rate is zero, mean that scanout is stop,
- * we don't need wait any more.
- */
- if (clk_get_rate(vop->dclk)) {
- /*
- * Rk3288 vop timing register is immediately, when configure
- * display timing on display time, may cause tearing.
- *
- * Vop standby will take effect at end of current frame,
- * if dsp hold valid irq happen, it means standby complete.
- *
- * mode set:
- * standby and wait complete --> |----
- * | display time
- * |----
- * |---> dsp hold irq
- * configure display timing --> |
- * standby exit |
- * | new frame start.
- */
-
- reinit_completion(&vop->dsp_hold_completion);
- vop_dsp_hold_valid_irq_enable(vop);
-
- spin_lock(&vop->reg_lock);
-
- VOP_REG_SET(vop, common, standby, 1);
-
- spin_unlock(&vop->reg_lock);
-
- wait_for_completion(&vop->dsp_hold_completion);
-
- vop_dsp_hold_valid_irq_disable(vop);
- }
-
pin_pol = BIT(DCLK_INVERT);
pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) ?
BIT(HSYNC_POSITIVE) : 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/6] drm/rockchip: vop: fix iommu page fault when resume
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
2017-07-31 9:49 ` [PATCH 1/6] drm/rockchip: vop: no need wait vblank on crtc enable Mark Yao
@ 2017-07-31 9:49 ` Mark Yao
2017-08-03 13:06 ` Sandy Huang
2017-07-31 9:49 ` [PATCH 3/6] drm/rockchip: vop: fix NV12 video display error Mark Yao
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:49 UTC (permalink / raw)
To: linux-arm-kernel
Iommu would get page fault with following path:
vop_disable:
1, disable all windows and set vop config done
2, vop enter to standy, all windows not works, but their registers
are not clean, when you read window's enable bit, may found the
window is enable.
vop_enable:
1, memcpy(vop->regsbak, vop->regs, len)
save current vop registers to vop->regsbak, then you can found
window is enable on regsbak.
2, VOP_WIN_SET(vop, win, gate, 1);
force enable window gate, but gate and enable are on same
hardware register, then window enable bit rewrite to vop hardware.
3, vop power on, and vop might try to scan destroyed buffer,
then iommu get page fault.
Move windows disable after vop regsbak restore, then vop regsbak mechanism
would keep tracing the modify, everything would be safe.
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 33 +++++++++++++----------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 0bfd563..0b5fd75 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -495,7 +495,7 @@ static void vop_line_flag_irq_disable(struct vop *vop)
static int vop_enable(struct drm_crtc *crtc)
{
struct vop *vop = to_vop(crtc);
- int ret;
+ int ret, i;
ret = pm_runtime_get_sync(vop->dev);
if (ret < 0) {
@@ -528,6 +528,20 @@ static int vop_enable(struct drm_crtc *crtc)
}
memcpy(vop->regs, vop->regsbak, vop->len);
+ /*
+ * We need to make sure that all windows are disabled before we
+ * enable the crtc. Otherwise we might try to scan from a destroyed
+ * buffer later.
+ */
+ for (i = 0; i < vop->data->win_size; i++) {
+ struct vop_win *vop_win = &vop->win[i];
+ const struct vop_win_data *win = vop_win->data;
+
+ spin_lock(&vop->reg_lock);
+ VOP_WIN_SET(vop, win, enable, 0);
+ spin_unlock(&vop->reg_lock);
+ }
+
vop_cfg_done(vop);
/*
@@ -561,28 +575,11 @@ static int vop_enable(struct drm_crtc *crtc)
static void vop_crtc_disable(struct drm_crtc *crtc)
{
struct vop *vop = to_vop(crtc);
- int i;
WARN_ON(vop->event);
rockchip_drm_psr_deactivate(&vop->crtc);
- /*
- * We need to make sure that all windows are disabled before we
- * disable that crtc. Otherwise we might try to scan from a destroyed
- * buffer later.
- */
- for (i = 0; i < vop->data->win_size; i++) {
- struct vop_win *vop_win = &vop->win[i];
- const struct vop_win_data *win = vop_win->data;
-
- spin_lock(&vop->reg_lock);
- VOP_WIN_SET(vop, win, enable, 0);
- spin_unlock(&vop->reg_lock);
- }
-
- vop_cfg_done(vop);
-
drm_crtc_vblank_off(crtc);
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/6] drm/rockchip: vop: fix iommu page fault when resume
2017-07-31 9:49 ` [PATCH 2/6] drm/rockchip: vop: fix iommu page fault when resume Mark Yao
@ 2017-08-03 13:06 ` Sandy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Sandy Huang @ 2017-08-03 13:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi mark,
? 2017/7/31 17:49, Mark Yao ??:
> Iommu would get page fault with following path:
> vop_disable:
> 1, disable all windows and set vop config done
> 2, vop enter to standy, all windows not works, but their registers
> are not clean, when you read window's enable bit, may found the
> window is enable.
>
> vop_enable:
> 1, memcpy(vop->regsbak, vop->regs, len)
> save current vop registers to vop->regsbak, then you can found
> window is enable on regsbak.
> 2, VOP_WIN_SET(vop, win, gate, 1);
> force enable window gate, but gate and enable are on same
> hardware register, then window enable bit rewrite to vop hardware.
> 3, vop power on, and vop might try to scan destroyed buffer,
> then iommu get page fault.
>
> Move windows disable after vop regsbak restore, then vop regsbak mechanism
> would keep tracing the modify, everything would be safe.
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 33 +++++++++++++----------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
Reviewed-by: Sandy huang <sandy.huang@rock-chips.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/6] drm/rockchip: vop: fix NV12 video display error
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
2017-07-31 9:49 ` [PATCH 1/6] drm/rockchip: vop: no need wait vblank on crtc enable Mark Yao
2017-07-31 9:49 ` [PATCH 2/6] drm/rockchip: vop: fix iommu page fault when resume Mark Yao
@ 2017-07-31 9:49 ` Mark Yao
2017-08-04 0:56 ` Sandy Huang
2017-07-31 9:49 ` [PATCH 4/6] drm/rockchip: vop: round_up pitches to word align Mark Yao
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:49 UTC (permalink / raw)
To: linux-arm-kernel
fixup the scale calculation formula on the case
src_height == (dst_height/2).
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index af1091f..56bbd2e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -299,6 +299,9 @@ static inline uint16_t scl_get_bili_dn_vskip(int src_h, int dst_h,
act_height = (src_h + vskiplines - 1) / vskiplines;
+ if (act_height == dst_h)
+ return GET_SCL_FT_BILI_DN(src_h, dst_h) / vskiplines;
+
return GET_SCL_FT_BILI_DN(act_height, dst_h);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/6] drm/rockchip: vop: fix NV12 video display error
2017-07-31 9:49 ` [PATCH 3/6] drm/rockchip: vop: fix NV12 video display error Mark Yao
@ 2017-08-04 0:56 ` Sandy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Sandy Huang @ 2017-08-04 0:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi mark,
? 2017/7/31 17:49, Mark Yao ??:
> fixup the scale calculation formula on the case
> src_height == (dst_height/2).
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index af1091f..56bbd2e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -299,6 +299,9 @@ static inline uint16_t scl_get_bili_dn_vskip(int src_h, int dst_h,
>
> act_height = (src_h + vskiplines - 1) / vskiplines;
>
> + if (act_height == dst_h)
> + return GET_SCL_FT_BILI_DN(src_h, dst_h) / vskiplines;
> +
> return GET_SCL_FT_BILI_DN(act_height, dst_h);
> }
>
>
Reviewed-by: Sandy huang <sandy.huang@rock-chips.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/6] drm/rockchip: vop: round_up pitches to word align
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
` (2 preceding siblings ...)
2017-07-31 9:49 ` [PATCH 3/6] drm/rockchip: vop: fix NV12 video display error Mark Yao
@ 2017-07-31 9:49 ` Mark Yao
2017-08-04 0:56 ` Sandy Huang
2017-07-31 9:49 ` [PATCH 5/6] drm/rockchip: vop: report error when check resource error Mark Yao
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:49 UTC (permalink / raw)
To: linux-arm-kernel
VOP pitch register is word align, need align to word.
VOP_WIN0_VIR:
bit[31:16] win0_vir_stride_uv
Number of words of Win0 uv Virtual width
bit[15:0] win0_vir_width
Number of words of Win0 yrgb Virtual width
ARGB888 : win0_vir_width
RGB888 : (win0_vir_width*3/4) + (win0_vir_width%3)
RGB565 : ceil(win0_vir_width/2)
YUV : ceil(win0_vir_width/4)
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 0b5fd75..fa0d9f7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -756,7 +756,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
spin_lock(&vop->reg_lock);
VOP_WIN_SET(vop, win, format, format);
- VOP_WIN_SET(vop, win, yrgb_vir, fb->pitches[0] >> 2);
+ VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
if (is_yuv_support(fb->format->format)) {
int hsub = drm_format_horz_chroma_subsampling(fb->format->format);
@@ -770,7 +770,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
offset += (src->y1 >> 16) * fb->pitches[1] / vsub;
dma_addr = rk_uv_obj->dma_addr + offset + fb->offsets[1];
- VOP_WIN_SET(vop, win, uv_vir, fb->pitches[1] >> 2);
+ VOP_WIN_SET(vop, win, uv_vir, DIV_ROUND_UP(fb->pitches[1], 4));
VOP_WIN_SET(vop, win, uv_mst, dma_addr);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/6] drm/rockchip: vop: round_up pitches to word align
2017-07-31 9:49 ` [PATCH 4/6] drm/rockchip: vop: round_up pitches to word align Mark Yao
@ 2017-08-04 0:56 ` Sandy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Sandy Huang @ 2017-08-04 0:56 UTC (permalink / raw)
To: linux-arm-kernel
? 2017/7/31 17:49, Mark Yao ??:
> VOP pitch register is word align, need align to word.
>
> VOP_WIN0_VIR:
> bit[31:16] win0_vir_stride_uv
> Number of words of Win0 uv Virtual width
> bit[15:0] win0_vir_width
> Number of words of Win0 yrgb Virtual width
> ARGB888 : win0_vir_width
> RGB888 : (win0_vir_width*3/4) + (win0_vir_width%3)
> RGB565 : ceil(win0_vir_width/2)
> YUV : ceil(win0_vir_width/4)
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Sandy huang <sandy.huang@rock-chips.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/6] drm/rockchip: vop: report error when check resource error
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
` (3 preceding siblings ...)
2017-07-31 9:49 ` [PATCH 4/6] drm/rockchip: vop: round_up pitches to word align Mark Yao
@ 2017-07-31 9:49 ` Mark Yao
2017-08-04 0:57 ` Sandy Huang
2017-07-31 9:50 ` [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev Mark Yao
2017-08-09 13:38 ` [PATCH 0/6] drm/rockchip: Some fixes Sean Paul
6 siblings, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:49 UTC (permalink / raw)
To: linux-arm-kernel
The user would be confused while facing a error commit without
any error report.
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa0d9f7..999c2e0 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -674,8 +674,10 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
* Src.x1 can be odd when do clip, but yuv plane start point
* need align with 2 pixel.
*/
- if (is_yuv_support(fb->format->format) && ((state->src.x1 >> 16) % 2))
+ if (is_yuv_support(fb->format->format) && ((state->src.x1 >> 16) % 2)) {
+ DRM_ERROR("Invalid Source: Yuv format not support odd xpos\n");
return -EINVAL;
+ }
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/6] drm/rockchip: vop: report error when check resource error
2017-07-31 9:49 ` [PATCH 5/6] drm/rockchip: vop: report error when check resource error Mark Yao
@ 2017-08-04 0:57 ` Sandy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Sandy Huang @ 2017-08-04 0:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi mark,
? 2017/7/31 17:49, Mark Yao ??:
> The user would be confused while facing a error commit without
> any error report.
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa0d9f7..999c2e0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -674,8 +674,10 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> * Src.x1 can be odd when do clip, but yuv plane start point
> * need align with 2 pixel.
> */
> - if (is_yuv_support(fb->format->format) && ((state->src.x1 >> 16) % 2))
> + if (is_yuv_support(fb->format->format) && ((state->src.x1 >> 16) % 2)) {
> + DRM_ERROR("Invalid Source: Yuv format not support odd xpos\n");
> return -EINVAL;
> + }
>
> return 0;
> }
>
Reviewed-by: Sandy huang <sandy.huang@rock-chips.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
` (4 preceding siblings ...)
2017-07-31 9:49 ` [PATCH 5/6] drm/rockchip: vop: report error when check resource error Mark Yao
@ 2017-07-31 9:50 ` Mark Yao
2017-07-31 11:57 ` Emil Velikov
2017-08-01 8:11 ` [PATCH v1.1] " Mark Yao
2017-08-09 13:38 ` [PATCH 0/6] drm/rockchip: Some fixes Sean Paul
6 siblings, 2 replies; 19+ messages in thread
From: Mark Yao @ 2017-07-31 9:50 UTC (permalink / raw)
To: linux-arm-kernel
Since fb_helper is not a pointer on rockchip_drm_private, it's no
need to check pointer.
Kms hotplug event may race into fbdev helper initial, and fb_helper->dev
may be NULL pointer, that would cause the bug:
[ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
[ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 0.736648] Modules linked in:
[ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
[ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
[ 0.738020] Workqueue: events cdn_dp_pd_event_work
[ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
[ 0.739109] PC is at mutex_lock+0x14/0x44
[ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
[ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
[ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
[ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
[ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
[ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
[ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
[ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
[ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
[ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 81f9548..e6bd0f4 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
struct rockchip_drm_private *private = dev->dev_private;
struct drm_fb_helper *fb_helper = &private->fbdev_helper;
- if (fb_helper)
+ if (fb_helper->dev)
drm_fb_helper_hotplug_event(fb_helper);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev
2017-07-31 9:50 ` [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev Mark Yao
@ 2017-07-31 11:57 ` Emil Velikov
2017-07-31 12:28 ` Daniel Vetter
2017-08-01 8:11 ` [PATCH v1.1] " Mark Yao
1 sibling, 1 reply; 19+ messages in thread
From: Emil Velikov @ 2017-07-31 11:57 UTC (permalink / raw)
To: linux-arm-kernel
On 31 July 2017 at 10:50, Mark Yao <mark.yao@rock-chips.com> wrote:
> Since fb_helper is not a pointer on rockchip_drm_private, it's no
> need to check pointer.
>
> Kms hotplug event may race into fbdev helper initial, and fb_helper->dev
> may be NULL pointer, that would cause the bug:
>
> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> [ 0.736648] Modules linked in:
> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
> [ 0.738020] Workqueue: events cdn_dp_pd_event_work
> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
> [ 0.739109] PC is at mutex_lock+0x14/0x44
> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
> [ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
> [ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
> [ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
> [ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
> [ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
> [ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
> [ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
> [ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
> [ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 81f9548..e6bd0f4 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
> struct rockchip_drm_private *private = dev->dev_private;
> struct drm_fb_helper *fb_helper = &private->fbdev_helper;
>
> - if (fb_helper)
> + if (fb_helper->dev)
> drm_fb_helper_hotplug_event(fb_helper);
Food for thought:
Quick grep shows that no other drivers have such a ->dev check. Does
this mean that either the issue is rockchip specific?
If not, one could look into resolving the problem directly in drm core.
Or at least update the other users, so they don't stumble upon the problem?
HTH
Emil
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev
2017-07-31 11:57 ` Emil Velikov
@ 2017-07-31 12:28 ` Daniel Vetter
2017-08-01 2:00 ` Mark yao
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2017-07-31 12:28 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 31, 2017 at 1:57 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 31 July 2017 at 10:50, Mark Yao <mark.yao@rock-chips.com> wrote:
>> Since fb_helper is not a pointer on rockchip_drm_private, it's no
>> need to check pointer.
>>
>> Kms hotplug event may race into fbdev helper initial, and fb_helper->dev
>> may be NULL pointer, that would cause the bug:
>>
>> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
>> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
>> [ 0.736648] Modules linked in:
>> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
>> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
>> [ 0.738020] Workqueue: events cdn_dp_pd_event_work
>> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
>> [ 0.739109] PC is at mutex_lock+0x14/0x44
>> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
>> [ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
>> [ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
>> [ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
>> [ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
>> [ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
>> [ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
>> [ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
>> [ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
>> [ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
>>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> index 81f9548..e6bd0f4 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> @@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
>> struct rockchip_drm_private *private = dev->dev_private;
>> struct drm_fb_helper *fb_helper = &private->fbdev_helper;
>>
>> - if (fb_helper)
>> + if (fb_helper->dev)
>> drm_fb_helper_hotplug_event(fb_helper);
> Food for thought:
>
> Quick grep shows that no other drivers have such a ->dev check. Does
> this mean that either the issue is rockchip specific?
> If not, one could look into resolving the problem directly in drm core.
>
> Or at least update the other users, so they don't stumble upon the problem?
The fbdev helpers support already handling hotplug events before you
have finalized the fbdev setup. Please read the kerneldoc for the
various fbdev functions, they explain what you should be doing. This
hack here should indeed not be needed.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev
2017-07-31 12:28 ` Daniel Vetter
@ 2017-08-01 2:00 ` Mark yao
0 siblings, 0 replies; 19+ messages in thread
From: Mark yao @ 2017-08-01 2:00 UTC (permalink / raw)
To: linux-arm-kernel
On 2017?07?31? 20:28, Daniel Vetter wrote:
> On Mon, Jul 31, 2017 at 1:57 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 31 July 2017 at 10:50, Mark Yao <mark.yao@rock-chips.com> wrote:
>>> Since fb_helper is not a pointer on rockchip_drm_private, it's no
>>> need to check pointer.
>>>
>>> Kms hotplug event may race into fbdev helper initial, and fb_helper->dev
>>> may be NULL pointer, that would cause the bug:
>>>
>>> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
>>> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
>>> [ 0.736648] Modules linked in:
>>> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
>>> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
>>> [ 0.738020] Workqueue: events cdn_dp_pd_event_work
>>> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
>>> [ 0.739109] PC is at mutex_lock+0x14/0x44
>>> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
>>> [ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
>>> [ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
>>> [ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
>>> [ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
>>> [ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
>>> [ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
>>> [ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
>>> [ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
>>> [ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
>>>
>>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>>> ---
>>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> index 81f9548..e6bd0f4 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> @@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
>>> struct rockchip_drm_private *private = dev->dev_private;
>>> struct drm_fb_helper *fb_helper = &private->fbdev_helper;
>>>
>>> - if (fb_helper)
>>> + if (fb_helper->dev)
>>> drm_fb_helper_hotplug_event(fb_helper);
>> Food for thought:
>>
>> Quick grep shows that no other drivers have such a ->dev check. Does
>> this mean that either the issue is rockchip specific?
>> If not, one could look into resolving the problem directly in drm core.
>>
>> Or at least update the other users, so they don't stumble upon the problem?
> The fbdev helpers support already handling hotplug events before you
> have finalized the fbdev setup. Please read the kerneldoc for the
> various fbdev functions, they explain what you should be doing. This
> hack here should indeed not be needed.
> -Daniel
Hi Daniel
Right, the doc[0] already detail this:
It is possible, though perhaps somewhat tricky, to implement race-free hotplug detection using
the fbdev helpers. The drm_fb_helper_prepare() helper must be called first to initialize the
minimum required to make hotplug detection work.Drivers also need to make sure to properly
set up the drm_mode_config.funcs member. After calling drm_kms_helper_poll_init() it is safe to
enable interrupts and start processing hotplug events.
The problem is drm/rockchip do the wrong initial, call drm_kms_helper_poll_init before fbdev setup.
will fix it at next version.
[0]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html#fbdev-helper-functions-reference
Best regards.
--
?ark Yao
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1.1] drm/rockchip: fix race with kms hotplug and fbdev
2017-07-31 9:50 ` [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev Mark Yao
2017-07-31 11:57 ` Emil Velikov
@ 2017-08-01 8:11 ` Mark Yao
2017-08-04 3:20 ` Sandy Huang
1 sibling, 1 reply; 19+ messages in thread
From: Mark Yao @ 2017-08-01 8:11 UTC (permalink / raw)
To: linux-arm-kernel
According to the kerneldoc[0], should do fbdev setup before calling
drm_kms_helper_poll_init(), otherwise, Kms hotplug event may race
into fbdev helper initial, and fb_helper->dev may be NULL pointer,
that would cause the bug:
[ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
[ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 0.736648] Modules linked in:
[ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
[ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
[ 0.738020] Workqueue: events cdn_dp_pd_event_work
[ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
[ 0.739109] PC is at mutex_lock+0x14/0x44
[ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
[ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
[ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
[ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
[ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
[ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
[ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
[ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
[ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
[ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
[0]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
Changes in v1.1:
- According to the kerneldoc, fix the race bug in generic way.
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 848edcf..c41f48a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -161,22 +161,21 @@ static int rockchip_drm_bind(struct device *dev)
*/
drm_dev->irq_enabled = true;
- /* init kms poll for handling hpd */
- drm_kms_helper_poll_init(drm_dev);
-
ret = rockchip_drm_fbdev_init(drm_dev);
if (ret)
- goto err_kms_helper_poll_fini;
+ goto err_unbind_all;
+
+ /* init kms poll for handling hpd */
+ drm_kms_helper_poll_init(drm_dev);
ret = drm_dev_register(drm_dev, 0);
if (ret)
- goto err_fbdev_fini;
+ goto err_kms_helper_poll_fini;
return 0;
-err_fbdev_fini:
- rockchip_drm_fbdev_fini(drm_dev);
err_kms_helper_poll_fini:
drm_kms_helper_poll_fini(drm_dev);
+ rockchip_drm_fbdev_fini(drm_dev);
err_unbind_all:
component_unbind_all(dev, drm_dev);
err_mode_config_cleanup:
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v1.1] drm/rockchip: fix race with kms hotplug and fbdev
2017-08-01 8:11 ` [PATCH v1.1] " Mark Yao
@ 2017-08-04 3:20 ` Sandy Huang
0 siblings, 0 replies; 19+ messages in thread
From: Sandy Huang @ 2017-08-04 3:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
? 2017/8/1 16:11, Mark Yao ??:
> According to the kerneldoc[0], should do fbdev setup before calling
> drm_kms_helper_poll_init(), otherwise, Kms hotplug event may race
> into fbdev helper initial, and fb_helper->dev may be NULL pointer,
> that would cause the bug:
> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000
> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> [ 0.736648] Modules linked in:
> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20
> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT)
> [ 0.738020] Workqueue: events cdn_dp_pd_event_work
> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000
> [ 0.739109] PC is at mutex_lock+0x14/0x44
> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114
> [ 0.756253] [<ffffff8008a344f4>] mutex_lock+0x14/0x44
> [ 0.756260] [<ffffff8008445708>] drm_fb_helper_hotplug_event+0x30/0x114
> [ 0.756271] [<ffffff8008473c84>] rockchip_drm_output_poll_changed+0x18/0x20
> [ 0.756280] [<ffffff8008439fcc>] drm_kms_helper_hotplug_event+0x28/0x34
> [ 0.756286] [<ffffff800846c444>] cdn_dp_pd_event_work+0x394/0x3c4
> [ 0.756295] [<ffffff80080b2b38>] process_one_work+0x218/0x3e0
> [ 0.756302] [<ffffff80080b3538>] worker_thread+0x2e8/0x404
> [ 0.756308] [<ffffff80080b7e70>] kthread+0xe8/0xf0
> [ 0.756316] [<ffffff8008082690>] ret_from_fork+0x10/0x40
>
> [0]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> Changes in v1.1:
> - According to the kerneldoc, fix the race bug in generic way.
>
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
Reviewed-by: Sandy huang <sandy.huang@rock-chips.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 0/6] drm/rockchip: Some fixes
2017-07-31 9:49 [PATCH 0/6] drm/rockchip: Some fixes Mark Yao
` (5 preceding siblings ...)
2017-07-31 9:50 ` [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev Mark Yao
@ 2017-08-09 13:38 ` Sean Paul
6 siblings, 0 replies; 19+ messages in thread
From: Sean Paul @ 2017-08-09 13:38 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 31, 2017 at 5:49 AM, Mark Yao <mark.yao@rock-chips.com> wrote:
> Here are some fixes port from rockchip_linux project[0],
>
> Tested on rk3399 and rk3288 board.
>
> [0]: https://github.com/rockchip-linux/kernel
>
> Mark Yao (6):
> drm/rockchip: vop: no need wait vblank on crtc enable
> drm/rockchip: vop: fix iommu page fault when resume
> drm/rockchip: vop: fix NV12 video display error
> drm/rockchip: vop: round_up pitches to word align
> drm/rockchip: vop: report error when check resource error
> drm/rockchip: fix race with kms hotplug and fbdev
Hi Mark,
I noticed you applied this set to both -misc-next and -misc-fixes. In
the future, please do not apply patches in both places.
Instead, consult the flowchart under "Where Do I Apply My Patch?" at
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
and choose the most appropriate place. If you are unsure, please reach
out to a -misc maintainer.
Thank you,
Sean
>
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 77 ++++++++---------------------
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 3 ++
> 3 files changed, 24 insertions(+), 58 deletions(-)
>
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread