* [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-12 18:02 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-12 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: linux-rockchip, daniel, airlied, heiko, hjc, Chris Morgan
From: Chris Morgan <macromorgan@hotmail.com>
If I use more than one VP to output on an RK3566 based device I
receive the following error (and then everything freezes):
[ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
[ 0.839191] Mem abort info:
[ 0.839442] ESR = 0x0000000096000005
[ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
[ 0.840256] SET = 0, FnV = 0
[ 0.840530] EA = 0, S1PTW = 0
[ 0.840821] FSC = 0x05: level 1 translation fault
[ 0.841254] Data abort info:
[ 0.841512] ISV = 0, ISS = 0x00000005
[ 0.841864] CM = 0, WnR = 0
[ 0.842130] [0000000000000250] user address but active_mm is swapper
[ 0.842704] Internal error: Oops: 96000005 [#1] SMP
[ 0.843139] Modules linked in:
[ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
[ 0.844013] Hardware name: RG503 (DT)
[ 0.844343] Workqueue: events_unbound deferred_probe_work_func
[ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
[ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
[ 0.846399] sp : ffffffc00a7a3710
[ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
[ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
[ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
[ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
[ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
[ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
[ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
[ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
[ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
[ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
[ 0.853048] Call trace:
[ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
[ 0.853706] drm_crtc_init_with_planes+0x68/0x94
[ 0.854118] vop2_bind+0x89c/0x920
[ 0.854429] component_bind_all+0x18c/0x290
[ 0.854805] rockchip_drm_bind+0xe4/0x1f0
[ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
[ 0.855639] __component_add+0x110/0x168
[ 0.855991] component_add+0x1c/0x28
[ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
[ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
[ 0.857184] mipi_dsi_attach+0x30/0x44
[ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
[ 0.857896] ams495qa01_probe+0x2d4/0x33c
[ 0.858257] spi_probe+0x8c/0xb8
[ 0.858550] really_probe+0x1e0/0x3b8
[ 0.858881] __driver_probe_device+0x16c/0x184
[ 0.859278] driver_probe_device+0x4c/0xfc
[ 0.859646] __device_attach_driver+0xf0/0x170
[ 0.860043] bus_for_each_drv+0xa4/0xcc
[ 0.860389] __device_attach+0xfc/0x1a8
[ 0.860733] device_initial_probe+0x1c/0x28
[ 0.861108] bus_probe_device+0x38/0x9c
[ 0.861452] deferred_probe_work_func+0xdc/0xf0
[ 0.861855] process_one_work+0x1b0/0x260
[ 0.862217] process_scheduled_works+0x4c/0x50
[ 0.862614] worker_thread+0x1f0/0x26c
[ 0.862949] kthread+0xc4/0xd4
[ 0.863227] ret_from_fork+0x10/0x20
[ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
[ 0.864095] ---[ end trace 0000000000000000 ]---
A cursory reading of the datasheet suggests it's possible to have
simultaneous output to 2 distinct outputs. On page 13 it states:
Support two simultaneous displays(same source) in the following interfaces:
- MIPI_DSI_TX
- LVDS
- HDMI
- eDP
In order to achieve this though, I need to output to VP0 and VP1 (or
any 2 distinct VPs really). This is so I can have the ref clock set
to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
example above it's 33.5MHz). Currently, only by removing this code
block is such a thing possible, though I'm not sure if long-term
there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index e4631f515ba4..f18d7f6f9f86 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
struct vop2_win *win = &vop2->win[i];
u32 possible_crtcs;
- if (vop2->data->soc_id == 3566) {
- /*
- * On RK3566 these windows don't have an independent
- * framebuffer. They share the framebuffer with smart0,
- * esmart0 and cluster0 respectively.
- */
- switch (win->data->phys_id) {
- case ROCKCHIP_VOP2_SMART1:
- case ROCKCHIP_VOP2_ESMART1:
- case ROCKCHIP_VOP2_CLUSTER1:
- continue;
- }
- }
-
if (win->type == DRM_PLANE_TYPE_PRIMARY) {
vp = find_vp_without_primary(vop2);
if (vp) {
--
2.25.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-12 18:02 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-12 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: airlied, hjc, linux-rockchip, Chris Morgan
From: Chris Morgan <macromorgan@hotmail.com>
If I use more than one VP to output on an RK3566 based device I
receive the following error (and then everything freezes):
[ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
[ 0.839191] Mem abort info:
[ 0.839442] ESR = 0x0000000096000005
[ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
[ 0.840256] SET = 0, FnV = 0
[ 0.840530] EA = 0, S1PTW = 0
[ 0.840821] FSC = 0x05: level 1 translation fault
[ 0.841254] Data abort info:
[ 0.841512] ISV = 0, ISS = 0x00000005
[ 0.841864] CM = 0, WnR = 0
[ 0.842130] [0000000000000250] user address but active_mm is swapper
[ 0.842704] Internal error: Oops: 96000005 [#1] SMP
[ 0.843139] Modules linked in:
[ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
[ 0.844013] Hardware name: RG503 (DT)
[ 0.844343] Workqueue: events_unbound deferred_probe_work_func
[ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
[ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
[ 0.846399] sp : ffffffc00a7a3710
[ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
[ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
[ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
[ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
[ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
[ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
[ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
[ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
[ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
[ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
[ 0.853048] Call trace:
[ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
[ 0.853706] drm_crtc_init_with_planes+0x68/0x94
[ 0.854118] vop2_bind+0x89c/0x920
[ 0.854429] component_bind_all+0x18c/0x290
[ 0.854805] rockchip_drm_bind+0xe4/0x1f0
[ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
[ 0.855639] __component_add+0x110/0x168
[ 0.855991] component_add+0x1c/0x28
[ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
[ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
[ 0.857184] mipi_dsi_attach+0x30/0x44
[ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
[ 0.857896] ams495qa01_probe+0x2d4/0x33c
[ 0.858257] spi_probe+0x8c/0xb8
[ 0.858550] really_probe+0x1e0/0x3b8
[ 0.858881] __driver_probe_device+0x16c/0x184
[ 0.859278] driver_probe_device+0x4c/0xfc
[ 0.859646] __device_attach_driver+0xf0/0x170
[ 0.860043] bus_for_each_drv+0xa4/0xcc
[ 0.860389] __device_attach+0xfc/0x1a8
[ 0.860733] device_initial_probe+0x1c/0x28
[ 0.861108] bus_probe_device+0x38/0x9c
[ 0.861452] deferred_probe_work_func+0xdc/0xf0
[ 0.861855] process_one_work+0x1b0/0x260
[ 0.862217] process_scheduled_works+0x4c/0x50
[ 0.862614] worker_thread+0x1f0/0x26c
[ 0.862949] kthread+0xc4/0xd4
[ 0.863227] ret_from_fork+0x10/0x20
[ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
[ 0.864095] ---[ end trace 0000000000000000 ]---
A cursory reading of the datasheet suggests it's possible to have
simultaneous output to 2 distinct outputs. On page 13 it states:
Support two simultaneous displays(same source) in the following interfaces:
- MIPI_DSI_TX
- LVDS
- HDMI
- eDP
In order to achieve this though, I need to output to VP0 and VP1 (or
any 2 distinct VPs really). This is so I can have the ref clock set
to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
example above it's 33.5MHz). Currently, only by removing this code
block is such a thing possible, though I'm not sure if long-term
there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index e4631f515ba4..f18d7f6f9f86 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
struct vop2_win *win = &vop2->win[i];
u32 possible_crtcs;
- if (vop2->data->soc_id == 3566) {
- /*
- * On RK3566 these windows don't have an independent
- * framebuffer. They share the framebuffer with smart0,
- * esmart0 and cluster0 respectively.
- */
- switch (win->data->phys_id) {
- case ROCKCHIP_VOP2_SMART1:
- case ROCKCHIP_VOP2_ESMART1:
- case ROCKCHIP_VOP2_CLUSTER1:
- continue;
- }
- }
-
if (win->type == DRM_PLANE_TYPE_PRIMARY) {
vp = find_vp_without_primary(vop2);
if (vp) {
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-12 18:02 ` Chris Morgan
@ 2022-09-13 6:55 ` Michael Riesch
-1 siblings, 0 replies; 19+ messages in thread
From: Michael Riesch @ 2022-09-13 6:55 UTC (permalink / raw)
To: Chris Morgan, dri-devel
Cc: linux-rockchip, daniel, airlied, heiko, hjc, Chris Morgan, Sascha Hauer
Hi,
On 9/12/22 20:02, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
Cc: Sascha -> any thoughts on this one?
Best regards,
Michael
> If I use more than one VP to output on an RK3566 based device I
> receive the following error (and then everything freezes):
>
> [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> [ 0.839191] Mem abort info:
> [ 0.839442] ESR = 0x0000000096000005
> [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 0.840256] SET = 0, FnV = 0
> [ 0.840530] EA = 0, S1PTW = 0
> [ 0.840821] FSC = 0x05: level 1 translation fault
> [ 0.841254] Data abort info:
> [ 0.841512] ISV = 0, ISS = 0x00000005
> [ 0.841864] CM = 0, WnR = 0
> [ 0.842130] [0000000000000250] user address but active_mm is swapper
> [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> [ 0.843139] Modules linked in:
> [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> [ 0.844013] Hardware name: RG503 (DT)
> [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> [ 0.846399] sp : ffffffc00a7a3710
> [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> [ 0.853048] Call trace:
> [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> [ 0.854118] vop2_bind+0x89c/0x920
> [ 0.854429] component_bind_all+0x18c/0x290
> [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> [ 0.855639] __component_add+0x110/0x168
> [ 0.855991] component_add+0x1c/0x28
> [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> [ 0.857184] mipi_dsi_attach+0x30/0x44
> [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> [ 0.858257] spi_probe+0x8c/0xb8
> [ 0.858550] really_probe+0x1e0/0x3b8
> [ 0.858881] __driver_probe_device+0x16c/0x184
> [ 0.859278] driver_probe_device+0x4c/0xfc
> [ 0.859646] __device_attach_driver+0xf0/0x170
> [ 0.860043] bus_for_each_drv+0xa4/0xcc
> [ 0.860389] __device_attach+0xfc/0x1a8
> [ 0.860733] device_initial_probe+0x1c/0x28
> [ 0.861108] bus_probe_device+0x38/0x9c
> [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> [ 0.861855] process_one_work+0x1b0/0x260
> [ 0.862217] process_scheduled_works+0x4c/0x50
> [ 0.862614] worker_thread+0x1f0/0x26c
> [ 0.862949] kthread+0xc4/0xd4
> [ 0.863227] ret_from_fork+0x10/0x20
> [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> [ 0.864095] ---[ end trace 0000000000000000 ]---
>
> A cursory reading of the datasheet suggests it's possible to have
> simultaneous output to 2 distinct outputs. On page 13 it states:
>
> Support two simultaneous displays(same source) in the following interfaces:
> - MIPI_DSI_TX
> - LVDS
> - HDMI
> - eDP
>
> In order to achieve this though, I need to output to VP0 and VP1 (or
> any 2 distinct VPs really). This is so I can have the ref clock set
> to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> example above it's 33.5MHz). Currently, only by removing this code
> block is such a thing possible, though I'm not sure if long-term
> there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
>
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index e4631f515ba4..f18d7f6f9f86 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> struct vop2_win *win = &vop2->win[i];
> u32 possible_crtcs;
>
> - if (vop2->data->soc_id == 3566) {
> - /*
> - * On RK3566 these windows don't have an independent
> - * framebuffer. They share the framebuffer with smart0,
> - * esmart0 and cluster0 respectively.
> - */
> - switch (win->data->phys_id) {
> - case ROCKCHIP_VOP2_SMART1:
> - case ROCKCHIP_VOP2_ESMART1:
> - case ROCKCHIP_VOP2_CLUSTER1:
> - continue;
> - }
> - }
> -
> if (win->type == DRM_PLANE_TYPE_PRIMARY) {
> vp = find_vp_without_primary(vop2);
> if (vp) {
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-13 6:55 ` Michael Riesch
0 siblings, 0 replies; 19+ messages in thread
From: Michael Riesch @ 2022-09-13 6:55 UTC (permalink / raw)
To: Chris Morgan, dri-devel
Cc: airlied, hjc, linux-rockchip, Chris Morgan, Sascha Hauer
Hi,
On 9/12/22 20:02, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
Cc: Sascha -> any thoughts on this one?
Best regards,
Michael
> If I use more than one VP to output on an RK3566 based device I
> receive the following error (and then everything freezes):
>
> [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> [ 0.839191] Mem abort info:
> [ 0.839442] ESR = 0x0000000096000005
> [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 0.840256] SET = 0, FnV = 0
> [ 0.840530] EA = 0, S1PTW = 0
> [ 0.840821] FSC = 0x05: level 1 translation fault
> [ 0.841254] Data abort info:
> [ 0.841512] ISV = 0, ISS = 0x00000005
> [ 0.841864] CM = 0, WnR = 0
> [ 0.842130] [0000000000000250] user address but active_mm is swapper
> [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> [ 0.843139] Modules linked in:
> [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> [ 0.844013] Hardware name: RG503 (DT)
> [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> [ 0.846399] sp : ffffffc00a7a3710
> [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> [ 0.853048] Call trace:
> [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> [ 0.854118] vop2_bind+0x89c/0x920
> [ 0.854429] component_bind_all+0x18c/0x290
> [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> [ 0.855639] __component_add+0x110/0x168
> [ 0.855991] component_add+0x1c/0x28
> [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> [ 0.857184] mipi_dsi_attach+0x30/0x44
> [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> [ 0.858257] spi_probe+0x8c/0xb8
> [ 0.858550] really_probe+0x1e0/0x3b8
> [ 0.858881] __driver_probe_device+0x16c/0x184
> [ 0.859278] driver_probe_device+0x4c/0xfc
> [ 0.859646] __device_attach_driver+0xf0/0x170
> [ 0.860043] bus_for_each_drv+0xa4/0xcc
> [ 0.860389] __device_attach+0xfc/0x1a8
> [ 0.860733] device_initial_probe+0x1c/0x28
> [ 0.861108] bus_probe_device+0x38/0x9c
> [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> [ 0.861855] process_one_work+0x1b0/0x260
> [ 0.862217] process_scheduled_works+0x4c/0x50
> [ 0.862614] worker_thread+0x1f0/0x26c
> [ 0.862949] kthread+0xc4/0xd4
> [ 0.863227] ret_from_fork+0x10/0x20
> [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> [ 0.864095] ---[ end trace 0000000000000000 ]---
>
> A cursory reading of the datasheet suggests it's possible to have
> simultaneous output to 2 distinct outputs. On page 13 it states:
>
> Support two simultaneous displays(same source) in the following interfaces:
> - MIPI_DSI_TX
> - LVDS
> - HDMI
> - eDP
>
> In order to achieve this though, I need to output to VP0 and VP1 (or
> any 2 distinct VPs really). This is so I can have the ref clock set
> to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> example above it's 33.5MHz). Currently, only by removing this code
> block is such a thing possible, though I'm not sure if long-term
> there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
>
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index e4631f515ba4..f18d7f6f9f86 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> struct vop2_win *win = &vop2->win[i];
> u32 possible_crtcs;
>
> - if (vop2->data->soc_id == 3566) {
> - /*
> - * On RK3566 these windows don't have an independent
> - * framebuffer. They share the framebuffer with smart0,
> - * esmart0 and cluster0 respectively.
> - */
> - switch (win->data->phys_id) {
> - case ROCKCHIP_VOP2_SMART1:
> - case ROCKCHIP_VOP2_ESMART1:
> - case ROCKCHIP_VOP2_CLUSTER1:
> - continue;
> - }
> - }
> -
> if (win->type == DRM_PLANE_TYPE_PRIMARY) {
> vp = find_vp_without_primary(vop2);
> if (vp) {
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-13 6:55 ` Michael Riesch
(?)
@ 2022-09-13 9:10 ` Piotr Oniszczuk
2022-09-13 15:36 ` Chris Morgan
-1 siblings, 1 reply; 19+ messages in thread
From: Piotr Oniszczuk @ 2022-09-13 9:10 UTC (permalink / raw)
To: Michael Riesch, Chris Morgan
Cc: David Airlie, Chris Morgan, Sandy Huang, dri-devel,
open list:ARM/Rockchip SoC...,
Sascha Hauer
[-- Attachment #1: Type: text/plain, Size: 6449 bytes --]
> Wiadomość napisana przez Michael Riesch <michael.riesch@wolfvision.net> w dniu 13.09.2022, o godz. 08:55:
>
> Hi,
>
> On 9/12/22 20:02, Chris Morgan wrote:
>> From: Chris Morgan <macromorgan@hotmail.com>
>
> Cc: Sascha -> any thoughts on this one?
>
> Best regards,
> Michael
>
>> If I use more than one VP to output on an RK3566 based device I
>> receive the following error (and then everything freezes):
>>
>> [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
>> [ 0.839191] Mem abort info:
>> [ 0.839442] ESR = 0x0000000096000005
>> [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 0.840256] SET = 0, FnV = 0
>> [ 0.840530] EA = 0, S1PTW = 0
>> [ 0.840821] FSC = 0x05: level 1 translation fault
>> [ 0.841254] Data abort info:
>> [ 0.841512] ISV = 0, ISS = 0x00000005
>> [ 0.841864] CM = 0, WnR = 0
>> [ 0.842130] [0000000000000250] user address but active_mm is swapper
>> [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
>> [ 0.843139] Modules linked in:
>> [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
>> [ 0.844013] Hardware name: RG503 (DT)
>> [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
>> [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
>> [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
>> [ 0.846399] sp : ffffffc00a7a3710
>> [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
>> [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
>> [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
>> [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
>> [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
>> [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
>> [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
>> [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
>> [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
>> [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
>> [ 0.853048] Call trace:
>> [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
>> [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
>> [ 0.854118] vop2_bind+0x89c/0x920
>> [ 0.854429] component_bind_all+0x18c/0x290
>> [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
>> [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
>> [ 0.855639] __component_add+0x110/0x168
>> [ 0.855991] component_add+0x1c/0x28
>> [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
>> [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
>> [ 0.857184] mipi_dsi_attach+0x30/0x44
>> [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
>> [ 0.857896] ams495qa01_probe+0x2d4/0x33c
>> [ 0.858257] spi_probe+0x8c/0xb8
>> [ 0.858550] really_probe+0x1e0/0x3b8
>> [ 0.858881] __driver_probe_device+0x16c/0x184
>> [ 0.859278] driver_probe_device+0x4c/0xfc
>> [ 0.859646] __device_attach_driver+0xf0/0x170
>> [ 0.860043] bus_for_each_drv+0xa4/0xcc
>> [ 0.860389] __device_attach+0xfc/0x1a8
>> [ 0.860733] device_initial_probe+0x1c/0x28
>> [ 0.861108] bus_probe_device+0x38/0x9c
>> [ 0.861452] deferred_probe_work_func+0xdc/0xf0
>> [ 0.861855] process_one_work+0x1b0/0x260
>> [ 0.862217] process_scheduled_works+0x4c/0x50
>> [ 0.862614] worker_thread+0x1f0/0x26c
>> [ 0.862949] kthread+0xc4/0xd4
>> [ 0.863227] ret_from_fork+0x10/0x20
>> [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
>> [ 0.864095] ---[ end trace 0000000000000000 ]---
>>
>> A cursory reading of the datasheet suggests it's possible to have
>> simultaneous output to 2 distinct outputs. On page 13 it states:
>>
>> Support two simultaneous displays(same source) in the following interfaces:
>> - MIPI_DSI_TX
>> - LVDS
>> - HDMI
>> - eDP
>>
>> In order to achieve this though, I need to output to VP0 and VP1 (or
>> any 2 distinct VPs really). This is so I can have the ref clock set
>> to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
>> example above it's 33.5MHz). Currently, only by removing this code
>> block is such a thing possible, though I'm not sure if long-term
>> there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
>> along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
>>
>> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>>
>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> ---
>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
>> 1 file changed, 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index e4631f515ba4..f18d7f6f9f86 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
>> struct vop2_win *win = &vop2->win[i];
>> u32 possible_crtcs;
>>
>> - if (vop2->data->soc_id == 3566) {
>> - /*
>> - * On RK3566 these windows don't have an independent
>> - * framebuffer. They share the framebuffer with smart0,
>> - * esmart0 and cluster0 respectively.
>> - */
>> - switch (win->data->phys_id) {
>> - case ROCKCHIP_VOP2_SMART1:
>> - case ROCKCHIP_VOP2_ESMART1:
>> - case ROCKCHIP_VOP2_CLUSTER1:
>> - continue;
>> - }
>> - }
>> -
>> if (win->type == DRM_PLANE_TYPE_PRIMARY) {
>> vp = find_vp_without_primary(vop2);
>> if (vp) {
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Chris, Michael,
IIRC this is fix for issue reported by me here: https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c <https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c>
[-- Attachment #2: Type: text/html, Size: 12538 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-13 9:10 ` Piotr Oniszczuk
@ 2022-09-13 15:36 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-13 15:36 UTC (permalink / raw)
To: Piotr Oniszczuk
Cc: David Airlie, Sandy Huang, dri-devel, Chris Morgan,
open list:ARM/Rockchip SoC...,
Michael Riesch, Sascha Hauer
On Tue, Sep 13, 2022 at 11:10:16AM +0200, Piotr Oniszczuk wrote:
>
>
> > Wiadomość napisana przez Michael Riesch <michael.riesch@wolfvision.net> w dniu 13.09.2022, o godz. 08:55:
> >
> > Hi,
> >
> > On 9/12/22 20:02, Chris Morgan wrote:
> >> From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Cc: Sascha -> any thoughts on this one?
> >
> > Best regards,
> > Michael
> >
> >> If I use more than one VP to output on an RK3566 based device I
> >> receive the following error (and then everything freezes):
> >>
> >> [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> >> [ 0.839191] Mem abort info:
> >> [ 0.839442] ESR = 0x0000000096000005
> >> [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> >> [ 0.840256] SET = 0, FnV = 0
> >> [ 0.840530] EA = 0, S1PTW = 0
> >> [ 0.840821] FSC = 0x05: level 1 translation fault
> >> [ 0.841254] Data abort info:
> >> [ 0.841512] ISV = 0, ISS = 0x00000005
> >> [ 0.841864] CM = 0, WnR = 0
> >> [ 0.842130] [0000000000000250] user address but active_mm is swapper
> >> [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> >> [ 0.843139] Modules linked in:
> >> [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> >> [ 0.844013] Hardware name: RG503 (DT)
> >> [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> >> [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> >> [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> >> [ 0.846399] sp : ffffffc00a7a3710
> >> [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> >> [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> >> [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> >> [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> >> [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> >> [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> >> [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> >> [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> >> [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> >> [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> >> [ 0.853048] Call trace:
> >> [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> >> [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> >> [ 0.854118] vop2_bind+0x89c/0x920
> >> [ 0.854429] component_bind_all+0x18c/0x290
> >> [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> >> [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> >> [ 0.855639] __component_add+0x110/0x168
> >> [ 0.855991] component_add+0x1c/0x28
> >> [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> >> [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> >> [ 0.857184] mipi_dsi_attach+0x30/0x44
> >> [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> >> [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> >> [ 0.858257] spi_probe+0x8c/0xb8
> >> [ 0.858550] really_probe+0x1e0/0x3b8
> >> [ 0.858881] __driver_probe_device+0x16c/0x184
> >> [ 0.859278] driver_probe_device+0x4c/0xfc
> >> [ 0.859646] __device_attach_driver+0xf0/0x170
> >> [ 0.860043] bus_for_each_drv+0xa4/0xcc
> >> [ 0.860389] __device_attach+0xfc/0x1a8
> >> [ 0.860733] device_initial_probe+0x1c/0x28
> >> [ 0.861108] bus_probe_device+0x38/0x9c
> >> [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> >> [ 0.861855] process_one_work+0x1b0/0x260
> >> [ 0.862217] process_scheduled_works+0x4c/0x50
> >> [ 0.862614] worker_thread+0x1f0/0x26c
> >> [ 0.862949] kthread+0xc4/0xd4
> >> [ 0.863227] ret_from_fork+0x10/0x20
> >> [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> >> [ 0.864095] ---[ end trace 0000000000000000 ]---
> >>
> >> A cursory reading of the datasheet suggests it's possible to have
> >> simultaneous output to 2 distinct outputs. On page 13 it states:
> >>
> >> Support two simultaneous displays(same source) in the following interfaces:
> >> - MIPI_DSI_TX
> >> - LVDS
> >> - HDMI
> >> - eDP
> >>
> >> In order to achieve this though, I need to output to VP0 and VP1 (or
> >> any 2 distinct VPs really). This is so I can have the ref clock set
> >> to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> >> example above it's 33.5MHz). Currently, only by removing this code
> >> block is such a thing possible, though I'm not sure if long-term
> >> there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> >> along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> >>
> >> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >>
> >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >> ---
> >> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> >> 1 file changed, 14 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> index e4631f515ba4..f18d7f6f9f86 100644
> >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> >> struct vop2_win *win = &vop2->win[i];
> >> u32 possible_crtcs;
> >>
> >> - if (vop2->data->soc_id == 3566) {
> >> - /*
> >> - * On RK3566 these windows don't have an independent
> >> - * framebuffer. They share the framebuffer with smart0,
> >> - * esmart0 and cluster0 respectively.
> >> - */
> >> - switch (win->data->phys_id) {
> >> - case ROCKCHIP_VOP2_SMART1:
> >> - case ROCKCHIP_VOP2_ESMART1:
> >> - case ROCKCHIP_VOP2_CLUSTER1:
> >> - continue;
> >> - }
> >> - }
> >> -
> >> if (win->type == DRM_PLANE_TYPE_PRIMARY) {
> >> vp = find_vp_without_primary(vop2);
> >> if (vp) {
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
> Chris, Michael,
>
> IIRC this is fix for issue reported by me here: https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c <https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c>
I think this actually clarifies a bit of what I'm seeing. Specifically
I see nothing sometimes when I try to do a kmscube on my DSI panel.
Order of video ports matters, as if the DSI is VP0 and the HDMI is VP1
I see the output on the screen, but if HDMI is VP0 and DSI is VP1 I
see nothing on the screen.
As to the null pointer exception, some previous tracing suggested it
was occuring because a 2nd crtc would attempt to be registered with
planes when the plane was null. Basically with 1 primary plane we
would only ever assign it to the first video port. However, we'd try
to run drm_crtc_init_with_planes on the second video port later with
no plane and it would throw the above null pointer dereference error.
I'm wondering (talking out loud mostly because I honestly am not
very familiar with DRM) if in this case we should have 1 CRTC per
primary plane instead of 1 CRTC per video port?
Thank you, sorry for all the trouble this is causing.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-13 15:36 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-13 15:36 UTC (permalink / raw)
To: Piotr Oniszczuk
Cc: Michael Riesch, Chris Morgan, dri-devel,
open list:ARM/Rockchip SoC...,
Daniel Vetter, David Airlie, Heiko Stübner, Sandy Huang,
Sascha Hauer
On Tue, Sep 13, 2022 at 11:10:16AM +0200, Piotr Oniszczuk wrote:
>
>
> > Wiadomość napisana przez Michael Riesch <michael.riesch@wolfvision.net> w dniu 13.09.2022, o godz. 08:55:
> >
> > Hi,
> >
> > On 9/12/22 20:02, Chris Morgan wrote:
> >> From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Cc: Sascha -> any thoughts on this one?
> >
> > Best regards,
> > Michael
> >
> >> If I use more than one VP to output on an RK3566 based device I
> >> receive the following error (and then everything freezes):
> >>
> >> [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> >> [ 0.839191] Mem abort info:
> >> [ 0.839442] ESR = 0x0000000096000005
> >> [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> >> [ 0.840256] SET = 0, FnV = 0
> >> [ 0.840530] EA = 0, S1PTW = 0
> >> [ 0.840821] FSC = 0x05: level 1 translation fault
> >> [ 0.841254] Data abort info:
> >> [ 0.841512] ISV = 0, ISS = 0x00000005
> >> [ 0.841864] CM = 0, WnR = 0
> >> [ 0.842130] [0000000000000250] user address but active_mm is swapper
> >> [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> >> [ 0.843139] Modules linked in:
> >> [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> >> [ 0.844013] Hardware name: RG503 (DT)
> >> [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> >> [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> >> [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> >> [ 0.846399] sp : ffffffc00a7a3710
> >> [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> >> [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> >> [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> >> [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> >> [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> >> [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> >> [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> >> [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> >> [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> >> [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> >> [ 0.853048] Call trace:
> >> [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> >> [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> >> [ 0.854118] vop2_bind+0x89c/0x920
> >> [ 0.854429] component_bind_all+0x18c/0x290
> >> [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> >> [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> >> [ 0.855639] __component_add+0x110/0x168
> >> [ 0.855991] component_add+0x1c/0x28
> >> [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> >> [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> >> [ 0.857184] mipi_dsi_attach+0x30/0x44
> >> [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> >> [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> >> [ 0.858257] spi_probe+0x8c/0xb8
> >> [ 0.858550] really_probe+0x1e0/0x3b8
> >> [ 0.858881] __driver_probe_device+0x16c/0x184
> >> [ 0.859278] driver_probe_device+0x4c/0xfc
> >> [ 0.859646] __device_attach_driver+0xf0/0x170
> >> [ 0.860043] bus_for_each_drv+0xa4/0xcc
> >> [ 0.860389] __device_attach+0xfc/0x1a8
> >> [ 0.860733] device_initial_probe+0x1c/0x28
> >> [ 0.861108] bus_probe_device+0x38/0x9c
> >> [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> >> [ 0.861855] process_one_work+0x1b0/0x260
> >> [ 0.862217] process_scheduled_works+0x4c/0x50
> >> [ 0.862614] worker_thread+0x1f0/0x26c
> >> [ 0.862949] kthread+0xc4/0xd4
> >> [ 0.863227] ret_from_fork+0x10/0x20
> >> [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> >> [ 0.864095] ---[ end trace 0000000000000000 ]---
> >>
> >> A cursory reading of the datasheet suggests it's possible to have
> >> simultaneous output to 2 distinct outputs. On page 13 it states:
> >>
> >> Support two simultaneous displays(same source) in the following interfaces:
> >> - MIPI_DSI_TX
> >> - LVDS
> >> - HDMI
> >> - eDP
> >>
> >> In order to achieve this though, I need to output to VP0 and VP1 (or
> >> any 2 distinct VPs really). This is so I can have the ref clock set
> >> to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> >> example above it's 33.5MHz). Currently, only by removing this code
> >> block is such a thing possible, though I'm not sure if long-term
> >> there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> >> along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> >>
> >> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >>
> >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >> ---
> >> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> >> 1 file changed, 14 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> index e4631f515ba4..f18d7f6f9f86 100644
> >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> >> struct vop2_win *win = &vop2->win[i];
> >> u32 possible_crtcs;
> >>
> >> - if (vop2->data->soc_id == 3566) {
> >> - /*
> >> - * On RK3566 these windows don't have an independent
> >> - * framebuffer. They share the framebuffer with smart0,
> >> - * esmart0 and cluster0 respectively.
> >> - */
> >> - switch (win->data->phys_id) {
> >> - case ROCKCHIP_VOP2_SMART1:
> >> - case ROCKCHIP_VOP2_ESMART1:
> >> - case ROCKCHIP_VOP2_CLUSTER1:
> >> - continue;
> >> - }
> >> - }
> >> -
> >> if (win->type == DRM_PLANE_TYPE_PRIMARY) {
> >> vp = find_vp_without_primary(vop2);
> >> if (vp) {
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
> Chris, Michael,
>
> IIRC this is fix for issue reported by me here: https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c <https://lore.kernel.org/linux-arm-kernel/20220405090509.GP4012@pengutronix.de/t/#mf29d19089fefcd27995a93c886f65132b6c75c7c>
I think this actually clarifies a bit of what I'm seeing. Specifically
I see nothing sometimes when I try to do a kmscube on my DSI panel.
Order of video ports matters, as if the DSI is VP0 and the HDMI is VP1
I see the output on the screen, but if HDMI is VP0 and DSI is VP1 I
see nothing on the screen.
As to the null pointer exception, some previous tracing suggested it
was occuring because a 2nd crtc would attempt to be registered with
planes when the plane was null. Basically with 1 primary plane we
would only ever assign it to the first video port. However, we'd try
to run drm_crtc_init_with_planes on the second video port later with
no plane and it would throw the above null pointer dereference error.
I'm wondering (talking out loud mostly because I honestly am not
very familiar with DRM) if in this case we should have 1 CRTC per
primary plane instead of 1 CRTC per video port?
Thank you, sorry for all the trouble this is causing.
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-13 6:55 ` Michael Riesch
@ 2022-09-14 6:49 ` Sascha Hauer
-1 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-14 6:49 UTC (permalink / raw)
To: Michael Riesch
Cc: Chris Morgan, dri-devel, linux-rockchip, daniel, airlied, heiko,
hjc, Chris Morgan
On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> Hi,
>
> On 9/12/22 20:02, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
>
> Cc: Sascha -> any thoughts on this one?
>
> Best regards,
> Michael
>
> > If I use more than one VP to output on an RK3566 based device I
> > receive the following error (and then everything freezes):
> >
> > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > [ 0.839191] Mem abort info:
> > [ 0.839442] ESR = 0x0000000096000005
> > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 0.840256] SET = 0, FnV = 0
> > [ 0.840530] EA = 0, S1PTW = 0
> > [ 0.840821] FSC = 0x05: level 1 translation fault
> > [ 0.841254] Data abort info:
> > [ 0.841512] ISV = 0, ISS = 0x00000005
> > [ 0.841864] CM = 0, WnR = 0
> > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > [ 0.843139] Modules linked in:
> > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > [ 0.844013] Hardware name: RG503 (DT)
> > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > [ 0.846399] sp : ffffffc00a7a3710
> > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > [ 0.853048] Call trace:
> > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > [ 0.854118] vop2_bind+0x89c/0x920
> > [ 0.854429] component_bind_all+0x18c/0x290
> > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > [ 0.855639] __component_add+0x110/0x168
> > [ 0.855991] component_add+0x1c/0x28
> > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > [ 0.858257] spi_probe+0x8c/0xb8
> > [ 0.858550] really_probe+0x1e0/0x3b8
> > [ 0.858881] __driver_probe_device+0x16c/0x184
> > [ 0.859278] driver_probe_device+0x4c/0xfc
> > [ 0.859646] __device_attach_driver+0xf0/0x170
> > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > [ 0.860389] __device_attach+0xfc/0x1a8
> > [ 0.860733] device_initial_probe+0x1c/0x28
> > [ 0.861108] bus_probe_device+0x38/0x9c
> > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > [ 0.861855] process_one_work+0x1b0/0x260
> > [ 0.862217] process_scheduled_works+0x4c/0x50
> > [ 0.862614] worker_thread+0x1f0/0x26c
> > [ 0.862949] kthread+0xc4/0xd4
> > [ 0.863227] ret_from_fork+0x10/0x20
> > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > [ 0.864095] ---[ end trace 0000000000000000 ]---
> >
> > A cursory reading of the datasheet suggests it's possible to have
> > simultaneous output to 2 distinct outputs. On page 13 it states:
> >
> > Support two simultaneous displays(same source) in the following interfaces:
> > - MIPI_DSI_TX
> > - LVDS
> > - HDMI
> > - eDP
> >
> > In order to achieve this though, I need to output to VP0 and VP1 (or
> > any 2 distinct VPs really). This is so I can have the ref clock set
> > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > example above it's 33.5MHz). Currently, only by removing this code
> > block is such a thing possible, though I'm not sure if long-term
> > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> >
> > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > 1 file changed, 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index e4631f515ba4..f18d7f6f9f86 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > struct vop2_win *win = &vop2->win[i];
> > u32 possible_crtcs;
> >
> > - if (vop2->data->soc_id == 3566) {
> > - /*
> > - * On RK3566 these windows don't have an independent
> > - * framebuffer. They share the framebuffer with smart0,
> > - * esmart0 and cluster0 respectively.
> > - */
> > - switch (win->data->phys_id) {
> > - case ROCKCHIP_VOP2_SMART1:
> > - case ROCKCHIP_VOP2_ESMART1:
> > - case ROCKCHIP_VOP2_CLUSTER1:
> > - continue;
> > - }
> > - }
Let me say that a "window" in the Rockchip terminology is what a plane
is in the DRM terminology, and a video port (vp) corresponds to a crtc.
On the RK3566 some windows do not have their own framebuffer, for
example the smart1 window always shows what the smart0 window shows.
This "feature" makes these windows unusable as planes, so the idea is to
simply not use them.
If skipping these windows from initialization results in a NULL pointer
deref then there's something wrong in the driver logic, but it's not the
lines you are removing here that are wrong. Nothing should prevent you
from using multiple VPs when the unusable windows on RK3566 are not
registered.
I looked over the VOP2 driver and haven't found the cause for the crash.
I found another issue though which might bite you later. At some point
we do a:
drm_plane_create_zpos_property(&win->base, win->win_id, 0,
vop2->registered_num_wins - 1);
registered_num_wins is the total number of windows present in the
system. What we should use here instead is the number of windows that
are actually registered which would be 3 windows less in the RK3566
case.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-14 6:49 ` Sascha Hauer
0 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-14 6:49 UTC (permalink / raw)
To: Michael Riesch
Cc: airlied, Chris Morgan, hjc, dri-devel, Chris Morgan, linux-rockchip
On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> Hi,
>
> On 9/12/22 20:02, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
>
> Cc: Sascha -> any thoughts on this one?
>
> Best regards,
> Michael
>
> > If I use more than one VP to output on an RK3566 based device I
> > receive the following error (and then everything freezes):
> >
> > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > [ 0.839191] Mem abort info:
> > [ 0.839442] ESR = 0x0000000096000005
> > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 0.840256] SET = 0, FnV = 0
> > [ 0.840530] EA = 0, S1PTW = 0
> > [ 0.840821] FSC = 0x05: level 1 translation fault
> > [ 0.841254] Data abort info:
> > [ 0.841512] ISV = 0, ISS = 0x00000005
> > [ 0.841864] CM = 0, WnR = 0
> > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > [ 0.843139] Modules linked in:
> > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > [ 0.844013] Hardware name: RG503 (DT)
> > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > [ 0.846399] sp : ffffffc00a7a3710
> > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > [ 0.853048] Call trace:
> > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > [ 0.854118] vop2_bind+0x89c/0x920
> > [ 0.854429] component_bind_all+0x18c/0x290
> > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > [ 0.855639] __component_add+0x110/0x168
> > [ 0.855991] component_add+0x1c/0x28
> > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > [ 0.858257] spi_probe+0x8c/0xb8
> > [ 0.858550] really_probe+0x1e0/0x3b8
> > [ 0.858881] __driver_probe_device+0x16c/0x184
> > [ 0.859278] driver_probe_device+0x4c/0xfc
> > [ 0.859646] __device_attach_driver+0xf0/0x170
> > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > [ 0.860389] __device_attach+0xfc/0x1a8
> > [ 0.860733] device_initial_probe+0x1c/0x28
> > [ 0.861108] bus_probe_device+0x38/0x9c
> > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > [ 0.861855] process_one_work+0x1b0/0x260
> > [ 0.862217] process_scheduled_works+0x4c/0x50
> > [ 0.862614] worker_thread+0x1f0/0x26c
> > [ 0.862949] kthread+0xc4/0xd4
> > [ 0.863227] ret_from_fork+0x10/0x20
> > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > [ 0.864095] ---[ end trace 0000000000000000 ]---
> >
> > A cursory reading of the datasheet suggests it's possible to have
> > simultaneous output to 2 distinct outputs. On page 13 it states:
> >
> > Support two simultaneous displays(same source) in the following interfaces:
> > - MIPI_DSI_TX
> > - LVDS
> > - HDMI
> > - eDP
> >
> > In order to achieve this though, I need to output to VP0 and VP1 (or
> > any 2 distinct VPs really). This is so I can have the ref clock set
> > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > example above it's 33.5MHz). Currently, only by removing this code
> > block is such a thing possible, though I'm not sure if long-term
> > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> >
> > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > 1 file changed, 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index e4631f515ba4..f18d7f6f9f86 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > struct vop2_win *win = &vop2->win[i];
> > u32 possible_crtcs;
> >
> > - if (vop2->data->soc_id == 3566) {
> > - /*
> > - * On RK3566 these windows don't have an independent
> > - * framebuffer. They share the framebuffer with smart0,
> > - * esmart0 and cluster0 respectively.
> > - */
> > - switch (win->data->phys_id) {
> > - case ROCKCHIP_VOP2_SMART1:
> > - case ROCKCHIP_VOP2_ESMART1:
> > - case ROCKCHIP_VOP2_CLUSTER1:
> > - continue;
> > - }
> > - }
Let me say that a "window" in the Rockchip terminology is what a plane
is in the DRM terminology, and a video port (vp) corresponds to a crtc.
On the RK3566 some windows do not have their own framebuffer, for
example the smart1 window always shows what the smart0 window shows.
This "feature" makes these windows unusable as planes, so the idea is to
simply not use them.
If skipping these windows from initialization results in a NULL pointer
deref then there's something wrong in the driver logic, but it's not the
lines you are removing here that are wrong. Nothing should prevent you
from using multiple VPs when the unusable windows on RK3566 are not
registered.
I looked over the VOP2 driver and haven't found the cause for the crash.
I found another issue though which might bite you later. At some point
we do a:
drm_plane_create_zpos_property(&win->base, win->win_id, 0,
vop2->registered_num_wins - 1);
registered_num_wins is the total number of windows present in the
system. What we should use here instead is the number of windows that
are actually registered which would be 3 windows less in the RK3566
case.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-14 6:49 ` Sascha Hauer
@ 2022-09-14 13:04 ` Chris Morgan
-1 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-14 13:04 UTC (permalink / raw)
To: Sascha Hauer
Cc: Michael Riesch, Chris Morgan, dri-devel, linux-rockchip, daniel,
airlied, heiko, hjc
On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > Hi,
> >
> > On 9/12/22 20:02, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Cc: Sascha -> any thoughts on this one?
> >
> > Best regards,
> > Michael
> >
> > > If I use more than one VP to output on an RK3566 based device I
> > > receive the following error (and then everything freezes):
> > >
> > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > [ 0.839191] Mem abort info:
> > > [ 0.839442] ESR = 0x0000000096000005
> > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > [ 0.840256] SET = 0, FnV = 0
> > > [ 0.840530] EA = 0, S1PTW = 0
> > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > [ 0.841254] Data abort info:
> > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > [ 0.841864] CM = 0, WnR = 0
> > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > [ 0.843139] Modules linked in:
> > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > [ 0.844013] Hardware name: RG503 (DT)
> > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > [ 0.846399] sp : ffffffc00a7a3710
> > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > [ 0.853048] Call trace:
> > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > [ 0.854118] vop2_bind+0x89c/0x920
> > > [ 0.854429] component_bind_all+0x18c/0x290
> > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > [ 0.855639] __component_add+0x110/0x168
> > > [ 0.855991] component_add+0x1c/0x28
> > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > [ 0.858257] spi_probe+0x8c/0xb8
> > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > [ 0.861855] process_one_work+0x1b0/0x260
> > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > [ 0.862949] kthread+0xc4/0xd4
> > > [ 0.863227] ret_from_fork+0x10/0x20
> > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > >
> > > A cursory reading of the datasheet suggests it's possible to have
> > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > >
> > > Support two simultaneous displays(same source) in the following interfaces:
> > > - MIPI_DSI_TX
> > > - LVDS
> > > - HDMI
> > > - eDP
> > >
> > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > example above it's 33.5MHz). Currently, only by removing this code
> > > block is such a thing possible, though I'm not sure if long-term
> > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > >
> > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > >
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > 1 file changed, 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > index e4631f515ba4..f18d7f6f9f86 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > struct vop2_win *win = &vop2->win[i];
> > > u32 possible_crtcs;
> > >
> > > - if (vop2->data->soc_id == 3566) {
> > > - /*
> > > - * On RK3566 these windows don't have an independent
> > > - * framebuffer. They share the framebuffer with smart0,
> > > - * esmart0 and cluster0 respectively.
> > > - */
> > > - switch (win->data->phys_id) {
> > > - case ROCKCHIP_VOP2_SMART1:
> > > - case ROCKCHIP_VOP2_ESMART1:
> > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > - continue;
> > > - }
> > > - }
>
> Let me say that a "window" in the Rockchip terminology is what a plane
> is in the DRM terminology, and a video port (vp) corresponds to a crtc.
>
> On the RK3566 some windows do not have their own framebuffer, for
> example the smart1 window always shows what the smart0 window shows.
> This "feature" makes these windows unusable as planes, so the idea is to
> simply not use them.
>
> If skipping these windows from initialization results in a NULL pointer
> deref then there's something wrong in the driver logic, but it's not the
> lines you are removing here that are wrong. Nothing should prevent you
> from using multiple VPs when the unusable windows on RK3566 are not
> registered.
I'll tell you what I "think" is causing the null pointer deref, but like
always I reserve the right to be wrong.
The driver checks for the primary planes and assigns them to active
video ports. For the rk3566 it's skipping the 3 planes mentioned
above for the RK3566. As a result if there is more than 1 active video
port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
doesn't set vp->primary_plane = win for that port. Slightly below that
it iterates through the active video ports again and runs
drm_crtc_init_with_planes on them. Only in this case one of the ports
doesn't have a plane, which is where it does the null pointer deref,
because the plane is null.
>
> I looked over the VOP2 driver and haven't found the cause for the crash.
> I found another issue though which might bite you later. At some point
> we do a:
>
> drm_plane_create_zpos_property(&win->base, win->win_id, 0,
> vop2->registered_num_wins - 1);
>
> registered_num_wins is the total number of windows present in the
> system. What we should use here instead is the number of windows that
> are actually registered which would be 3 windows less in the RK3566
> case.
Should we just set it right in rockchip_vop2_reg.c? The vop2_win_data
it is using for the rk3566 is identical to the rk3568, but should it
be?
I'll level with you, I'm out of my element trying to solve a problem
with a board that has both a DSI and HDMI output. If I try to use
the DSI and HDMI output on the same video port, the HDMI sets the
DCLK_VOPx (where x is whichever port) to 24000000, and as a result
the panel is displayed wrong. I'm pretty sure I'm not going to be
able to simultaneously display an image on the DSI panel and the
HDMI (though if I could that would be cool), but I'd like to have
them both enabled so I can switch between them. For now I can get
it to work if I put them on different video ports, of course that
means I have to either disable the HDMI or remove the code listed
in this "fix". It should be noted that I can now confirm that while
this fixes the null pointer dereference it brings back some of the
issues you found earlier in testing, so obviously this isn't the
right way to do it and I'm withdrawing this as a patch.
Thank you.
>
> Sascha
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C9417cf2acb5f4ce2823b08da961d4ab1%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637987349870888766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7KQyfEKfXdLQDdz6y5HIffVY5Tj8%2BthwPWMJRS6eTDE%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-14 13:04 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-14 13:04 UTC (permalink / raw)
To: Sascha Hauer
Cc: airlied, hjc, dri-devel, Chris Morgan, linux-rockchip, Michael Riesch
On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > Hi,
> >
> > On 9/12/22 20:02, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Cc: Sascha -> any thoughts on this one?
> >
> > Best regards,
> > Michael
> >
> > > If I use more than one VP to output on an RK3566 based device I
> > > receive the following error (and then everything freezes):
> > >
> > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > [ 0.839191] Mem abort info:
> > > [ 0.839442] ESR = 0x0000000096000005
> > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > [ 0.840256] SET = 0, FnV = 0
> > > [ 0.840530] EA = 0, S1PTW = 0
> > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > [ 0.841254] Data abort info:
> > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > [ 0.841864] CM = 0, WnR = 0
> > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > [ 0.843139] Modules linked in:
> > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > [ 0.844013] Hardware name: RG503 (DT)
> > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > [ 0.846399] sp : ffffffc00a7a3710
> > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > [ 0.853048] Call trace:
> > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > [ 0.854118] vop2_bind+0x89c/0x920
> > > [ 0.854429] component_bind_all+0x18c/0x290
> > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > [ 0.855639] __component_add+0x110/0x168
> > > [ 0.855991] component_add+0x1c/0x28
> > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > [ 0.858257] spi_probe+0x8c/0xb8
> > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > [ 0.861855] process_one_work+0x1b0/0x260
> > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > [ 0.862949] kthread+0xc4/0xd4
> > > [ 0.863227] ret_from_fork+0x10/0x20
> > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > >
> > > A cursory reading of the datasheet suggests it's possible to have
> > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > >
> > > Support two simultaneous displays(same source) in the following interfaces:
> > > - MIPI_DSI_TX
> > > - LVDS
> > > - HDMI
> > > - eDP
> > >
> > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > example above it's 33.5MHz). Currently, only by removing this code
> > > block is such a thing possible, though I'm not sure if long-term
> > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > >
> > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > >
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > 1 file changed, 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > index e4631f515ba4..f18d7f6f9f86 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > struct vop2_win *win = &vop2->win[i];
> > > u32 possible_crtcs;
> > >
> > > - if (vop2->data->soc_id == 3566) {
> > > - /*
> > > - * On RK3566 these windows don't have an independent
> > > - * framebuffer. They share the framebuffer with smart0,
> > > - * esmart0 and cluster0 respectively.
> > > - */
> > > - switch (win->data->phys_id) {
> > > - case ROCKCHIP_VOP2_SMART1:
> > > - case ROCKCHIP_VOP2_ESMART1:
> > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > - continue;
> > > - }
> > > - }
>
> Let me say that a "window" in the Rockchip terminology is what a plane
> is in the DRM terminology, and a video port (vp) corresponds to a crtc.
>
> On the RK3566 some windows do not have their own framebuffer, for
> example the smart1 window always shows what the smart0 window shows.
> This "feature" makes these windows unusable as planes, so the idea is to
> simply not use them.
>
> If skipping these windows from initialization results in a NULL pointer
> deref then there's something wrong in the driver logic, but it's not the
> lines you are removing here that are wrong. Nothing should prevent you
> from using multiple VPs when the unusable windows on RK3566 are not
> registered.
I'll tell you what I "think" is causing the null pointer deref, but like
always I reserve the right to be wrong.
The driver checks for the primary planes and assigns them to active
video ports. For the rk3566 it's skipping the 3 planes mentioned
above for the RK3566. As a result if there is more than 1 active video
port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
doesn't set vp->primary_plane = win for that port. Slightly below that
it iterates through the active video ports again and runs
drm_crtc_init_with_planes on them. Only in this case one of the ports
doesn't have a plane, which is where it does the null pointer deref,
because the plane is null.
>
> I looked over the VOP2 driver and haven't found the cause for the crash.
> I found another issue though which might bite you later. At some point
> we do a:
>
> drm_plane_create_zpos_property(&win->base, win->win_id, 0,
> vop2->registered_num_wins - 1);
>
> registered_num_wins is the total number of windows present in the
> system. What we should use here instead is the number of windows that
> are actually registered which would be 3 windows less in the RK3566
> case.
Should we just set it right in rockchip_vop2_reg.c? The vop2_win_data
it is using for the rk3566 is identical to the rk3568, but should it
be?
I'll level with you, I'm out of my element trying to solve a problem
with a board that has both a DSI and HDMI output. If I try to use
the DSI and HDMI output on the same video port, the HDMI sets the
DCLK_VOPx (where x is whichever port) to 24000000, and as a result
the panel is displayed wrong. I'm pretty sure I'm not going to be
able to simultaneously display an image on the DSI panel and the
HDMI (though if I could that would be cool), but I'd like to have
them both enabled so I can switch between them. For now I can get
it to work if I put them on different video ports, of course that
means I have to either disable the HDMI or remove the code listed
in this "fix". It should be noted that I can now confirm that while
this fixes the null pointer dereference it brings back some of the
issues you found earlier in testing, so obviously this isn't the
right way to do it and I'm withdrawing this as a patch.
Thank you.
>
> Sascha
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C9417cf2acb5f4ce2823b08da961d4ab1%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637987349870888766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7KQyfEKfXdLQDdz6y5HIffVY5Tj8%2BthwPWMJRS6eTDE%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-14 13:04 ` Chris Morgan
@ 2022-09-14 13:35 ` Sascha Hauer
-1 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-14 13:35 UTC (permalink / raw)
To: Chris Morgan
Cc: Michael Riesch, Chris Morgan, dri-devel, linux-rockchip, daniel,
airlied, heiko, hjc
On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > Hi,
> > >
> > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > >
> > > Cc: Sascha -> any thoughts on this one?
> > >
> > > Best regards,
> > > Michael
> > >
> > > > If I use more than one VP to output on an RK3566 based device I
> > > > receive the following error (and then everything freezes):
> > > >
> > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > [ 0.839191] Mem abort info:
> > > > [ 0.839442] ESR = 0x0000000096000005
> > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > [ 0.840256] SET = 0, FnV = 0
> > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > [ 0.841254] Data abort info:
> > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > [ 0.841864] CM = 0, WnR = 0
> > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > [ 0.843139] Modules linked in:
> > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > [ 0.853048] Call trace:
> > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > [ 0.855639] __component_add+0x110/0x168
> > > > [ 0.855991] component_add+0x1c/0x28
> > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > [ 0.862949] kthread+0xc4/0xd4
> > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > >
> > > > A cursory reading of the datasheet suggests it's possible to have
> > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > >
> > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > - MIPI_DSI_TX
> > > > - LVDS
> > > > - HDMI
> > > > - eDP
> > > >
> > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > block is such a thing possible, though I'm not sure if long-term
> > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > >
> > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > >
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > 1 file changed, 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > struct vop2_win *win = &vop2->win[i];
> > > > u32 possible_crtcs;
> > > >
> > > > - if (vop2->data->soc_id == 3566) {
> > > > - /*
> > > > - * On RK3566 these windows don't have an independent
> > > > - * framebuffer. They share the framebuffer with smart0,
> > > > - * esmart0 and cluster0 respectively.
> > > > - */
> > > > - switch (win->data->phys_id) {
> > > > - case ROCKCHIP_VOP2_SMART1:
> > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > - continue;
> > > > - }
> > > > - }
> >
> > Let me say that a "window" in the Rockchip terminology is what a plane
> > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> >
> > On the RK3566 some windows do not have their own framebuffer, for
> > example the smart1 window always shows what the smart0 window shows.
> > This "feature" makes these windows unusable as planes, so the idea is to
> > simply not use them.
> >
> > If skipping these windows from initialization results in a NULL pointer
> > deref then there's something wrong in the driver logic, but it's not the
> > lines you are removing here that are wrong. Nothing should prevent you
> > from using multiple VPs when the unusable windows on RK3566 are not
> > registered.
>
> I'll tell you what I "think" is causing the null pointer deref, but like
> always I reserve the right to be wrong.
>
> The driver checks for the primary planes and assigns them to active
> video ports. For the rk3566 it's skipping the 3 planes mentioned
> above for the RK3566. As a result if there is more than 1 active video
> port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> doesn't set vp->primary_plane = win for that port. Slightly below that
> it iterates through the active video ports again and runs
> drm_crtc_init_with_planes on them. Only in this case one of the ports
> doesn't have a plane, which is where it does the null pointer deref,
> because the plane is null.
Yes, you're right, that is likely the problem.
On RK3568 we have four windows that could serve as primary plane, so
there's enough for each of the three VPs. On RK3566 we only have two
windows, so once we use more than two active VPs we are running out of
primary planes.
I am writing these numbers because I think Esmart0-win0 could be used as
primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
rk3568_vop_win_data[]. See the attached patch, if that's the problem it
should resolve your issue.
That is not enough though once three VPs are active on RK3566. The
Cluster windows could likely be used as primary planes as well, they are
probably just not marked as such because they only support AFBC modes.
Sascha
-------------------------8<-------------------------
From f98f53565ce868811581afbb18451dfea2646b07 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 14 Sep 2022 15:27:51 +0200
Subject: [PATCH] drm/rockchip: vop2: Register Esmart0-win0 as primary plane
Esmart0-win0 could serve as primary plane, so mark it as such. On
RK3568 this window will never be used as primary plane, because the
three windows at the beginning of the rk3568_vop_win_data[] array
will be used. On RK3566 however, two of the windows at the beginning
of the rk3568_vop_win_data[] array cannot not be used, so we end
up without primary planes when multiple VPs are active.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 9bf0637bf8e26..9d30aa73b5422 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -188,7 +188,7 @@ static const struct vop2_win_data rk3568_vop_win_data[] = {
.base = 0x1800,
.layer_sel_id = 2,
.supported_rotations = DRM_MODE_REFLECT_Y,
- .type = DRM_PLANE_TYPE_OVERLAY,
+ .type = DRM_PLANE_TYPE_PRIMARY,
.max_upscale_factor = 8,
.max_downscale_factor = 8,
.dly = { 20, 47, 41 },
--
2.30.2
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-14 13:35 ` Sascha Hauer
0 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-14 13:35 UTC (permalink / raw)
To: Chris Morgan
Cc: airlied, hjc, dri-devel, Chris Morgan, linux-rockchip, Michael Riesch
On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > Hi,
> > >
> > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > >
> > > Cc: Sascha -> any thoughts on this one?
> > >
> > > Best regards,
> > > Michael
> > >
> > > > If I use more than one VP to output on an RK3566 based device I
> > > > receive the following error (and then everything freezes):
> > > >
> > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > [ 0.839191] Mem abort info:
> > > > [ 0.839442] ESR = 0x0000000096000005
> > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > [ 0.840256] SET = 0, FnV = 0
> > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > [ 0.841254] Data abort info:
> > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > [ 0.841864] CM = 0, WnR = 0
> > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > [ 0.843139] Modules linked in:
> > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > [ 0.853048] Call trace:
> > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > [ 0.855639] __component_add+0x110/0x168
> > > > [ 0.855991] component_add+0x1c/0x28
> > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > [ 0.862949] kthread+0xc4/0xd4
> > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > >
> > > > A cursory reading of the datasheet suggests it's possible to have
> > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > >
> > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > - MIPI_DSI_TX
> > > > - LVDS
> > > > - HDMI
> > > > - eDP
> > > >
> > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > block is such a thing possible, though I'm not sure if long-term
> > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > >
> > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > >
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > 1 file changed, 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > struct vop2_win *win = &vop2->win[i];
> > > > u32 possible_crtcs;
> > > >
> > > > - if (vop2->data->soc_id == 3566) {
> > > > - /*
> > > > - * On RK3566 these windows don't have an independent
> > > > - * framebuffer. They share the framebuffer with smart0,
> > > > - * esmart0 and cluster0 respectively.
> > > > - */
> > > > - switch (win->data->phys_id) {
> > > > - case ROCKCHIP_VOP2_SMART1:
> > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > - continue;
> > > > - }
> > > > - }
> >
> > Let me say that a "window" in the Rockchip terminology is what a plane
> > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> >
> > On the RK3566 some windows do not have their own framebuffer, for
> > example the smart1 window always shows what the smart0 window shows.
> > This "feature" makes these windows unusable as planes, so the idea is to
> > simply not use them.
> >
> > If skipping these windows from initialization results in a NULL pointer
> > deref then there's something wrong in the driver logic, but it's not the
> > lines you are removing here that are wrong. Nothing should prevent you
> > from using multiple VPs when the unusable windows on RK3566 are not
> > registered.
>
> I'll tell you what I "think" is causing the null pointer deref, but like
> always I reserve the right to be wrong.
>
> The driver checks for the primary planes and assigns them to active
> video ports. For the rk3566 it's skipping the 3 planes mentioned
> above for the RK3566. As a result if there is more than 1 active video
> port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> doesn't set vp->primary_plane = win for that port. Slightly below that
> it iterates through the active video ports again and runs
> drm_crtc_init_with_planes on them. Only in this case one of the ports
> doesn't have a plane, which is where it does the null pointer deref,
> because the plane is null.
Yes, you're right, that is likely the problem.
On RK3568 we have four windows that could serve as primary plane, so
there's enough for each of the three VPs. On RK3566 we only have two
windows, so once we use more than two active VPs we are running out of
primary planes.
I am writing these numbers because I think Esmart0-win0 could be used as
primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
rk3568_vop_win_data[]. See the attached patch, if that's the problem it
should resolve your issue.
That is not enough though once three VPs are active on RK3566. The
Cluster windows could likely be used as primary planes as well, they are
probably just not marked as such because they only support AFBC modes.
Sascha
-------------------------8<-------------------------
From f98f53565ce868811581afbb18451dfea2646b07 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 14 Sep 2022 15:27:51 +0200
Subject: [PATCH] drm/rockchip: vop2: Register Esmart0-win0 as primary plane
Esmart0-win0 could serve as primary plane, so mark it as such. On
RK3568 this window will never be used as primary plane, because the
three windows at the beginning of the rk3568_vop_win_data[] array
will be used. On RK3566 however, two of the windows at the beginning
of the rk3568_vop_win_data[] array cannot not be used, so we end
up without primary planes when multiple VPs are active.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 9bf0637bf8e26..9d30aa73b5422 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -188,7 +188,7 @@ static const struct vop2_win_data rk3568_vop_win_data[] = {
.base = 0x1800,
.layer_sel_id = 2,
.supported_rotations = DRM_MODE_REFLECT_Y,
- .type = DRM_PLANE_TYPE_OVERLAY,
+ .type = DRM_PLANE_TYPE_PRIMARY,
.max_upscale_factor = 8,
.max_downscale_factor = 8,
.dly = { 20, 47, 41 },
--
2.30.2
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-14 13:35 ` Sascha Hauer
@ 2022-09-15 15:13 ` Chris Morgan
-1 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-15 15:13 UTC (permalink / raw)
To: Sascha Hauer
Cc: airlied, hjc, dri-devel, Chris Morgan, linux-rockchip, Michael Riesch
On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > Hi,
> > > >
> > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > >
> > > > Cc: Sascha -> any thoughts on this one?
> > > >
> > > > Best regards,
> > > > Michael
> > > >
> > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > receive the following error (and then everything freezes):
> > > > >
> > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > [ 0.839191] Mem abort info:
> > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > [ 0.841254] Data abort info:
> > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > [ 0.843139] Modules linked in:
> > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > [ 0.853048] Call trace:
> > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > >
> > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > >
> > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > - MIPI_DSI_TX
> > > > > - LVDS
> > > > > - HDMI
> > > > > - eDP
> > > > >
> > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > >
> > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > >
> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > ---
> > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > 1 file changed, 14 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > struct vop2_win *win = &vop2->win[i];
> > > > > u32 possible_crtcs;
> > > > >
> > > > > - if (vop2->data->soc_id == 3566) {
> > > > > - /*
> > > > > - * On RK3566 these windows don't have an independent
> > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > - * esmart0 and cluster0 respectively.
> > > > > - */
> > > > > - switch (win->data->phys_id) {
> > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > - continue;
> > > > > - }
> > > > > - }
> > >
> > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > >
> > > On the RK3566 some windows do not have their own framebuffer, for
> > > example the smart1 window always shows what the smart0 window shows.
> > > This "feature" makes these windows unusable as planes, so the idea is to
> > > simply not use them.
> > >
> > > If skipping these windows from initialization results in a NULL pointer
> > > deref then there's something wrong in the driver logic, but it's not the
> > > lines you are removing here that are wrong. Nothing should prevent you
> > > from using multiple VPs when the unusable windows on RK3566 are not
> > > registered.
> >
> > I'll tell you what I "think" is causing the null pointer deref, but like
> > always I reserve the right to be wrong.
> >
> > The driver checks for the primary planes and assigns them to active
> > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > above for the RK3566. As a result if there is more than 1 active video
> > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > doesn't set vp->primary_plane = win for that port. Slightly below that
> > it iterates through the active video ports again and runs
> > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > doesn't have a plane, which is where it does the null pointer deref,
> > because the plane is null.
>
> Yes, you're right, that is likely the problem.
>
> On RK3568 we have four windows that could serve as primary plane, so
> there's enough for each of the three VPs. On RK3566 we only have two
> windows, so once we use more than two active VPs we are running out of
> primary planes.
The datasheet states that it supports up to 2 simultaneous displays, so
I think a hard limit of 2 VPs is fine for the rk3566.
>
> I am writing these numbers because I think Esmart0-win0 could be used as
> primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> should resolve your issue.
This patch did indeed resolve my issue, and also allowed me to get both
HDMI and DSI working simultaneously. It's not perfect (right now I'm
displaying a 640x480 image on a 1080p display since my DSI panel runs
at 640x480), but it works and that's great.
>
> That is not enough though once three VPs are active on RK3566. The
> Cluster windows could likely be used as primary planes as well, they are
> probably just not marked as such because they only support AFBC modes.
>
> Sascha
>
> -------------------------8<-------------------------
>
> From f98f53565ce868811581afbb18451dfea2646b07 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Wed, 14 Sep 2022 15:27:51 +0200
> Subject: [PATCH] drm/rockchip: vop2: Register Esmart0-win0 as primary plane
>
> Esmart0-win0 could serve as primary plane, so mark it as such. On
> RK3568 this window will never be used as primary plane, because the
> three windows at the beginning of the rk3568_vop_win_data[] array
> will be used. On RK3566 however, two of the windows at the beginning
> of the rk3568_vop_win_data[] array cannot not be used, so we end
> up without primary planes when multiple VPs are active.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> index 9bf0637bf8e26..9d30aa73b5422 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> @@ -188,7 +188,7 @@ static const struct vop2_win_data rk3568_vop_win_data[] = {
> .base = 0x1800,
> .layer_sel_id = 2,
> .supported_rotations = DRM_MODE_REFLECT_Y,
> - .type = DRM_PLANE_TYPE_OVERLAY,
> + .type = DRM_PLANE_TYPE_PRIMARY,
> .max_upscale_factor = 8,
> .max_downscale_factor = 8,
> .dly = { 20, 47, 41 },
> --
> 2.30.2
>
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7Cc218aa7fea7b48286e2508da965601a4%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637987593394560183%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b5aS8NGLknpQIax9gC474MTU9CHKJOgnUq28AXFH6MM%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-15 15:13 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-15 15:13 UTC (permalink / raw)
To: Sascha Hauer
Cc: Michael Riesch, Chris Morgan, dri-devel, linux-rockchip, daniel,
airlied, heiko, hjc
On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > Hi,
> > > >
> > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > >
> > > > Cc: Sascha -> any thoughts on this one?
> > > >
> > > > Best regards,
> > > > Michael
> > > >
> > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > receive the following error (and then everything freezes):
> > > > >
> > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > [ 0.839191] Mem abort info:
> > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > [ 0.841254] Data abort info:
> > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > [ 0.843139] Modules linked in:
> > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > [ 0.853048] Call trace:
> > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > >
> > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > >
> > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > - MIPI_DSI_TX
> > > > > - LVDS
> > > > > - HDMI
> > > > > - eDP
> > > > >
> > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > >
> > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > >
> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > ---
> > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > 1 file changed, 14 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > struct vop2_win *win = &vop2->win[i];
> > > > > u32 possible_crtcs;
> > > > >
> > > > > - if (vop2->data->soc_id == 3566) {
> > > > > - /*
> > > > > - * On RK3566 these windows don't have an independent
> > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > - * esmart0 and cluster0 respectively.
> > > > > - */
> > > > > - switch (win->data->phys_id) {
> > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > - continue;
> > > > > - }
> > > > > - }
> > >
> > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > >
> > > On the RK3566 some windows do not have their own framebuffer, for
> > > example the smart1 window always shows what the smart0 window shows.
> > > This "feature" makes these windows unusable as planes, so the idea is to
> > > simply not use them.
> > >
> > > If skipping these windows from initialization results in a NULL pointer
> > > deref then there's something wrong in the driver logic, but it's not the
> > > lines you are removing here that are wrong. Nothing should prevent you
> > > from using multiple VPs when the unusable windows on RK3566 are not
> > > registered.
> >
> > I'll tell you what I "think" is causing the null pointer deref, but like
> > always I reserve the right to be wrong.
> >
> > The driver checks for the primary planes and assigns them to active
> > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > above for the RK3566. As a result if there is more than 1 active video
> > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > doesn't set vp->primary_plane = win for that port. Slightly below that
> > it iterates through the active video ports again and runs
> > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > doesn't have a plane, which is where it does the null pointer deref,
> > because the plane is null.
>
> Yes, you're right, that is likely the problem.
>
> On RK3568 we have four windows that could serve as primary plane, so
> there's enough for each of the three VPs. On RK3566 we only have two
> windows, so once we use more than two active VPs we are running out of
> primary planes.
The datasheet states that it supports up to 2 simultaneous displays, so
I think a hard limit of 2 VPs is fine for the rk3566.
>
> I am writing these numbers because I think Esmart0-win0 could be used as
> primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> should resolve your issue.
This patch did indeed resolve my issue, and also allowed me to get both
HDMI and DSI working simultaneously. It's not perfect (right now I'm
displaying a 640x480 image on a 1080p display since my DSI panel runs
at 640x480), but it works and that's great.
>
> That is not enough though once three VPs are active on RK3566. The
> Cluster windows could likely be used as primary planes as well, they are
> probably just not marked as such because they only support AFBC modes.
>
> Sascha
>
> -------------------------8<-------------------------
>
> From f98f53565ce868811581afbb18451dfea2646b07 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Wed, 14 Sep 2022 15:27:51 +0200
> Subject: [PATCH] drm/rockchip: vop2: Register Esmart0-win0 as primary plane
>
> Esmart0-win0 could serve as primary plane, so mark it as such. On
> RK3568 this window will never be used as primary plane, because the
> three windows at the beginning of the rk3568_vop_win_data[] array
> will be used. On RK3566 however, two of the windows at the beginning
> of the rk3568_vop_win_data[] array cannot not be used, so we end
> up without primary planes when multiple VPs are active.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> index 9bf0637bf8e26..9d30aa73b5422 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> @@ -188,7 +188,7 @@ static const struct vop2_win_data rk3568_vop_win_data[] = {
> .base = 0x1800,
> .layer_sel_id = 2,
> .supported_rotations = DRM_MODE_REFLECT_Y,
> - .type = DRM_PLANE_TYPE_OVERLAY,
> + .type = DRM_PLANE_TYPE_PRIMARY,
> .max_upscale_factor = 8,
> .max_downscale_factor = 8,
> .dly = { 20, 47, 41 },
> --
> 2.30.2
>
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7Cc218aa7fea7b48286e2508da965601a4%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637987593394560183%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b5aS8NGLknpQIax9gC474MTU9CHKJOgnUq28AXFH6MM%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-15 15:13 ` Chris Morgan
@ 2022-09-26 8:17 ` Sascha Hauer
-1 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-26 8:17 UTC (permalink / raw)
To: Chris Morgan
Cc: airlied, hjc, dri-devel, Chris Morgan, linux-rockchip, Michael Riesch
On Thu, Sep 15, 2022 at 10:13:44AM -0500, Chris Morgan wrote:
> On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> > On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > > Hi,
> > > > >
> > > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > >
> > > > > Cc: Sascha -> any thoughts on this one?
> > > > >
> > > > > Best regards,
> > > > > Michael
> > > > >
> > > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > > receive the following error (and then everything freezes):
> > > > > >
> > > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > > [ 0.839191] Mem abort info:
> > > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > > [ 0.841254] Data abort info:
> > > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > > [ 0.843139] Modules linked in:
> > > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > > [ 0.853048] Call trace:
> > > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > > >
> > > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > > >
> > > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > > - MIPI_DSI_TX
> > > > > > - LVDS
> > > > > > - HDMI
> > > > > > - eDP
> > > > > >
> > > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > > >
> > > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > > >
> > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > > 1 file changed, 14 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > > struct vop2_win *win = &vop2->win[i];
> > > > > > u32 possible_crtcs;
> > > > > >
> > > > > > - if (vop2->data->soc_id == 3566) {
> > > > > > - /*
> > > > > > - * On RK3566 these windows don't have an independent
> > > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > > - * esmart0 and cluster0 respectively.
> > > > > > - */
> > > > > > - switch (win->data->phys_id) {
> > > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > > - continue;
> > > > > > - }
> > > > > > - }
> > > >
> > > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > > >
> > > > On the RK3566 some windows do not have their own framebuffer, for
> > > > example the smart1 window always shows what the smart0 window shows.
> > > > This "feature" makes these windows unusable as planes, so the idea is to
> > > > simply not use them.
> > > >
> > > > If skipping these windows from initialization results in a NULL pointer
> > > > deref then there's something wrong in the driver logic, but it's not the
> > > > lines you are removing here that are wrong. Nothing should prevent you
> > > > from using multiple VPs when the unusable windows on RK3566 are not
> > > > registered.
> > >
> > > I'll tell you what I "think" is causing the null pointer deref, but like
> > > always I reserve the right to be wrong.
> > >
> > > The driver checks for the primary planes and assigns them to active
> > > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > > above for the RK3566. As a result if there is more than 1 active video
> > > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > > doesn't set vp->primary_plane = win for that port. Slightly below that
> > > it iterates through the active video ports again and runs
> > > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > > doesn't have a plane, which is where it does the null pointer deref,
> > > because the plane is null.
> >
> > Yes, you're right, that is likely the problem.
> >
> > On RK3568 we have four windows that could serve as primary plane, so
> > there's enough for each of the three VPs. On RK3566 we only have two
> > windows, so once we use more than two active VPs we are running out of
> > primary planes.
>
> The datasheet states that it supports up to 2 simultaneous displays, so
> I think a hard limit of 2 VPs is fine for the rk3566.
>
> >
> > I am writing these numbers because I think Esmart0-win0 could be used as
> > primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> > rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> > should resolve your issue.
>
> This patch did indeed resolve my issue, and also allowed me to get both
> HDMI and DSI working simultaneously. It's not perfect (right now I'm
> displaying a 640x480 image on a 1080p display since my DSI panel runs
> at 640x480), but it works and that's great.
I just sent this patch for inclusion. Could you provide your formal
tested-by: tag?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-26 8:17 ` Sascha Hauer
0 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2022-09-26 8:17 UTC (permalink / raw)
To: Chris Morgan
Cc: Michael Riesch, Chris Morgan, dri-devel, linux-rockchip, daniel,
airlied, heiko, hjc
On Thu, Sep 15, 2022 at 10:13:44AM -0500, Chris Morgan wrote:
> On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> > On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > > Hi,
> > > > >
> > > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > >
> > > > > Cc: Sascha -> any thoughts on this one?
> > > > >
> > > > > Best regards,
> > > > > Michael
> > > > >
> > > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > > receive the following error (and then everything freezes):
> > > > > >
> > > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > > [ 0.839191] Mem abort info:
> > > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > > [ 0.841254] Data abort info:
> > > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > > [ 0.843139] Modules linked in:
> > > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > > [ 0.853048] Call trace:
> > > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > > >
> > > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > > >
> > > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > > - MIPI_DSI_TX
> > > > > > - LVDS
> > > > > > - HDMI
> > > > > > - eDP
> > > > > >
> > > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > > >
> > > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > > >
> > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > > 1 file changed, 14 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > > struct vop2_win *win = &vop2->win[i];
> > > > > > u32 possible_crtcs;
> > > > > >
> > > > > > - if (vop2->data->soc_id == 3566) {
> > > > > > - /*
> > > > > > - * On RK3566 these windows don't have an independent
> > > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > > - * esmart0 and cluster0 respectively.
> > > > > > - */
> > > > > > - switch (win->data->phys_id) {
> > > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > > - continue;
> > > > > > - }
> > > > > > - }
> > > >
> > > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > > >
> > > > On the RK3566 some windows do not have their own framebuffer, for
> > > > example the smart1 window always shows what the smart0 window shows.
> > > > This "feature" makes these windows unusable as planes, so the idea is to
> > > > simply not use them.
> > > >
> > > > If skipping these windows from initialization results in a NULL pointer
> > > > deref then there's something wrong in the driver logic, but it's not the
> > > > lines you are removing here that are wrong. Nothing should prevent you
> > > > from using multiple VPs when the unusable windows on RK3566 are not
> > > > registered.
> > >
> > > I'll tell you what I "think" is causing the null pointer deref, but like
> > > always I reserve the right to be wrong.
> > >
> > > The driver checks for the primary planes and assigns them to active
> > > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > > above for the RK3566. As a result if there is more than 1 active video
> > > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > > doesn't set vp->primary_plane = win for that port. Slightly below that
> > > it iterates through the active video ports again and runs
> > > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > > doesn't have a plane, which is where it does the null pointer deref,
> > > because the plane is null.
> >
> > Yes, you're right, that is likely the problem.
> >
> > On RK3568 we have four windows that could serve as primary plane, so
> > there's enough for each of the three VPs. On RK3566 we only have two
> > windows, so once we use more than two active VPs we are running out of
> > primary planes.
>
> The datasheet states that it supports up to 2 simultaneous displays, so
> I think a hard limit of 2 VPs is fine for the rk3566.
>
> >
> > I am writing these numbers because I think Esmart0-win0 could be used as
> > primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> > rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> > should resolve your issue.
>
> This patch did indeed resolve my issue, and also allowed me to get both
> HDMI and DSI working simultaneously. It's not perfect (right now I'm
> displaying a 640x480 image on a 1080p display since my DSI panel runs
> at 640x480), but it works and that's great.
I just sent this patch for inclusion. Could you provide your formal
tested-by: tag?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
2022-09-26 8:17 ` Sascha Hauer
@ 2022-09-26 12:58 ` Chris Morgan
-1 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-26 12:58 UTC (permalink / raw)
To: Sascha Hauer
Cc: Michael Riesch, Chris Morgan, dri-devel, linux-rockchip, daniel,
airlied, heiko, hjc
On Mon, Sep 26, 2022 at 10:17:40AM +0200, Sascha Hauer wrote:
> On Thu, Sep 15, 2022 at 10:13:44AM -0500, Chris Morgan wrote:
> > On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> > > On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > > > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > >
> > > > > > Cc: Sascha -> any thoughts on this one?
> > > > > >
> > > > > > Best regards,
> > > > > > Michael
> > > > > >
> > > > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > > > receive the following error (and then everything freezes):
> > > > > > >
> > > > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > > > [ 0.839191] Mem abort info:
> > > > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > > > [ 0.841254] Data abort info:
> > > > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > > > [ 0.843139] Modules linked in:
> > > > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > > > [ 0.853048] Call trace:
> > > > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > > > >
> > > > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > > > >
> > > > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > > > - MIPI_DSI_TX
> > > > > > > - LVDS
> > > > > > > - HDMI
> > > > > > > - eDP
> > > > > > >
> > > > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > > > >
> > > > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > > > >
> > > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > > > 1 file changed, 14 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > > > struct vop2_win *win = &vop2->win[i];
> > > > > > > u32 possible_crtcs;
> > > > > > >
> > > > > > > - if (vop2->data->soc_id == 3566) {
> > > > > > > - /*
> > > > > > > - * On RK3566 these windows don't have an independent
> > > > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > > > - * esmart0 and cluster0 respectively.
> > > > > > > - */
> > > > > > > - switch (win->data->phys_id) {
> > > > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > > > - continue;
> > > > > > > - }
> > > > > > > - }
> > > > >
> > > > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > > > >
> > > > > On the RK3566 some windows do not have their own framebuffer, for
> > > > > example the smart1 window always shows what the smart0 window shows.
> > > > > This "feature" makes these windows unusable as planes, so the idea is to
> > > > > simply not use them.
> > > > >
> > > > > If skipping these windows from initialization results in a NULL pointer
> > > > > deref then there's something wrong in the driver logic, but it's not the
> > > > > lines you are removing here that are wrong. Nothing should prevent you
> > > > > from using multiple VPs when the unusable windows on RK3566 are not
> > > > > registered.
> > > >
> > > > I'll tell you what I "think" is causing the null pointer deref, but like
> > > > always I reserve the right to be wrong.
> > > >
> > > > The driver checks for the primary planes and assigns them to active
> > > > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > > > above for the RK3566. As a result if there is more than 1 active video
> > > > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > > > doesn't set vp->primary_plane = win for that port. Slightly below that
> > > > it iterates through the active video ports again and runs
> > > > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > > > doesn't have a plane, which is where it does the null pointer deref,
> > > > because the plane is null.
> > >
> > > Yes, you're right, that is likely the problem.
> > >
> > > On RK3568 we have four windows that could serve as primary plane, so
> > > there's enough for each of the three VPs. On RK3566 we only have two
> > > windows, so once we use more than two active VPs we are running out of
> > > primary planes.
> >
> > The datasheet states that it supports up to 2 simultaneous displays, so
> > I think a hard limit of 2 VPs is fine for the rk3566.
> >
> > >
> > > I am writing these numbers because I think Esmart0-win0 could be used as
> > > primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> > > rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> > > should resolve your issue.
> >
> > This patch did indeed resolve my issue, and also allowed me to get both
> > HDMI and DSI working simultaneously. It's not perfect (right now I'm
> > displaying a 640x480 image on a 1080p display since my DSI panel runs
> > at 640x480), but it works and that's great.
>
> I just sent this patch for inclusion. Could you provide your formal
> tested-by: tag?
>
> Sascha
Done. Thank you once again for all your help.
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C0a2e9af8accf4b7e720708da9f979700%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637997770679554061%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=k56y1R3snN7uokq64C0Yx0K%2F0CbYktI%2BCeGyX5XHM%2Fk%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs
@ 2022-09-26 12:58 ` Chris Morgan
0 siblings, 0 replies; 19+ messages in thread
From: Chris Morgan @ 2022-09-26 12:58 UTC (permalink / raw)
To: Sascha Hauer
Cc: airlied, hjc, dri-devel, Chris Morgan, linux-rockchip, Michael Riesch
On Mon, Sep 26, 2022 at 10:17:40AM +0200, Sascha Hauer wrote:
> On Thu, Sep 15, 2022 at 10:13:44AM -0500, Chris Morgan wrote:
> > On Wed, Sep 14, 2022 at 03:35:30PM +0200, Sascha Hauer wrote:
> > > On Wed, Sep 14, 2022 at 08:04:18AM -0500, Chris Morgan wrote:
> > > > On Wed, Sep 14, 2022 at 08:49:27AM +0200, Sascha Hauer wrote:
> > > > > On Tue, Sep 13, 2022 at 08:55:22AM +0200, Michael Riesch wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 9/12/22 20:02, Chris Morgan wrote:
> > > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > >
> > > > > > Cc: Sascha -> any thoughts on this one?
> > > > > >
> > > > > > Best regards,
> > > > > > Michael
> > > > > >
> > > > > > > If I use more than one VP to output on an RK3566 based device I
> > > > > > > receive the following error (and then everything freezes):
> > > > > > >
> > > > > > > [ 0.838375] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000250
> > > > > > > [ 0.839191] Mem abort info:
> > > > > > > [ 0.839442] ESR = 0x0000000096000005
> > > > > > > [ 0.839785] EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > > > [ 0.840256] SET = 0, FnV = 0
> > > > > > > [ 0.840530] EA = 0, S1PTW = 0
> > > > > > > [ 0.840821] FSC = 0x05: level 1 translation fault
> > > > > > > [ 0.841254] Data abort info:
> > > > > > > [ 0.841512] ISV = 0, ISS = 0x00000005
> > > > > > > [ 0.841864] CM = 0, WnR = 0
> > > > > > > [ 0.842130] [0000000000000250] user address but active_mm is swapper
> > > > > > > [ 0.842704] Internal error: Oops: 96000005 [#1] SMP
> > > > > > > [ 0.843139] Modules linked in:
> > > > > > > [ 0.843420] CPU: 0 PID: 37 Comm: kworker/u8:1 Not tainted 6.0.0-rc5+ #40
> > > > > > > [ 0.844013] Hardware name: RG503 (DT)
> > > > > > > [ 0.844343] Workqueue: events_unbound deferred_probe_work_func
> > > > > > > [ 0.844871] pstate: 80000009 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > > > > [ 0.845487] pc : __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > > [ 0.845956] lr : drm_crtc_init_with_planes+0x68/0x94
> > > > > > > [ 0.846399] sp : ffffffc00a7a3710
> > > > > > > [ 0.846695] x29: ffffffc00a7a3710 x28: ffffff8000fb4080 x27: ffffffc00a7a37a0
> > > > > > > [ 0.847332] x26: ffffffc0097d01c7 x25: ffffff8000fb44d8 x24: ffffffc0097d01c7
> > > > > > > [ 0.847967] x23: ffffffc009311870 x22: 0000000000000000 x21: 0000000000000008
> > > > > > > [ 0.848603] x20: ffffff80010d0800 x19: ffffff8000fb44e8 x18: 0000000000000000
> > > > > > > [ 0.849237] x17: 08000000000000d1 x16: 0800000000000091 x15: 08000000000000c1
> > > > > > > [ 0.849874] x14: 0000000000000000 x13: 3432564e3631564e x12: 3231564e36314742
> > > > > > > [ 0.850509] x11: 3631475234324742 x10: 000000000000002d x9 : ffffffc008682004
> > > > > > > [ 0.851144] x8 : 00000000006f7475 x7 : 00000000fffffff0 x6 : ffffffc00a7a37a0
> > > > > > > [ 0.851778] x5 : ffffffc0097d01c7 x4 : ffffffc009311870 x3 : 0000000000000000
> > > > > > > [ 0.852412] x2 : 0000000000000008 x1 : ffffff8000fb44e8 x0 : ffffff80010d0800
> > > > > > > [ 0.853048] Call trace:
> > > > > > > [ 0.853270] __drm_crtc_init_with_planes+0x48/0x2e4
> > > > > > > [ 0.853706] drm_crtc_init_with_planes+0x68/0x94
> > > > > > > [ 0.854118] vop2_bind+0x89c/0x920
> > > > > > > [ 0.854429] component_bind_all+0x18c/0x290
> > > > > > > [ 0.854805] rockchip_drm_bind+0xe4/0x1f0
> > > > > > > [ 0.855166] try_to_bring_up_aggregate_device+0x9c/0x290
> > > > > > > [ 0.855639] __component_add+0x110/0x168
> > > > > > > [ 0.855991] component_add+0x1c/0x28
> > > > > > > [ 0.856312] dw_mipi_dsi_rockchip_host_attach+0x98/0x10c
> > > > > > > [ 0.856785] dw_mipi_dsi_host_attach+0xbc/0xd0
> > > > > > > [ 0.857184] mipi_dsi_attach+0x30/0x44
> > > > > > > [ 0.857521] devm_mipi_dsi_attach+0x2c/0x70
> > > > > > > [ 0.857896] ams495qa01_probe+0x2d4/0x33c
> > > > > > > [ 0.858257] spi_probe+0x8c/0xb8
> > > > > > > [ 0.858550] really_probe+0x1e0/0x3b8
> > > > > > > [ 0.858881] __driver_probe_device+0x16c/0x184
> > > > > > > [ 0.859278] driver_probe_device+0x4c/0xfc
> > > > > > > [ 0.859646] __device_attach_driver+0xf0/0x170
> > > > > > > [ 0.860043] bus_for_each_drv+0xa4/0xcc
> > > > > > > [ 0.860389] __device_attach+0xfc/0x1a8
> > > > > > > [ 0.860733] device_initial_probe+0x1c/0x28
> > > > > > > [ 0.861108] bus_probe_device+0x38/0x9c
> > > > > > > [ 0.861452] deferred_probe_work_func+0xdc/0xf0
> > > > > > > [ 0.861855] process_one_work+0x1b0/0x260
> > > > > > > [ 0.862217] process_scheduled_works+0x4c/0x50
> > > > > > > [ 0.862614] worker_thread+0x1f0/0x26c
> > > > > > > [ 0.862949] kthread+0xc4/0xd4
> > > > > > > [ 0.863227] ret_from_fork+0x10/0x20
> > > > > > > [ 0.863553] Code: aa0503fa f9002bfb aa0603fb b40000a2 (b9424840)
> > > > > > > [ 0.864095] ---[ end trace 0000000000000000 ]---
> > > > > > >
> > > > > > > A cursory reading of the datasheet suggests it's possible to have
> > > > > > > simultaneous output to 2 distinct outputs. On page 13 it states:
> > > > > > >
> > > > > > > Support two simultaneous displays(same source) in the following interfaces:
> > > > > > > - MIPI_DSI_TX
> > > > > > > - LVDS
> > > > > > > - HDMI
> > > > > > > - eDP
> > > > > > >
> > > > > > > In order to achieve this though, I need to output to VP0 and VP1 (or
> > > > > > > any 2 distinct VPs really). This is so I can have the ref clock set
> > > > > > > to 24MHz for the HDMI and the pixel clock of the DSI panel (for the
> > > > > > > example above it's 33.5MHz). Currently, only by removing this code
> > > > > > > block is such a thing possible, though I'm not sure if long-term
> > > > > > > there should only be 1 CRTC for the rk3566 (and 2 CRTCs for 3568)
> > > > > > > along with a max of 2 encoders for rk3566 (and 3 encoders for 3568).
> > > > > > >
> > > > > > > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> > > > > > >
> > > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 --------------
> > > > > > > 1 file changed, 14 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > index e4631f515ba4..f18d7f6f9f86 100644
> > > > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > > > > > @@ -2289,20 +2289,6 @@ static int vop2_create_crtc(struct vop2 *vop2)
> > > > > > > struct vop2_win *win = &vop2->win[i];
> > > > > > > u32 possible_crtcs;
> > > > > > >
> > > > > > > - if (vop2->data->soc_id == 3566) {
> > > > > > > - /*
> > > > > > > - * On RK3566 these windows don't have an independent
> > > > > > > - * framebuffer. They share the framebuffer with smart0,
> > > > > > > - * esmart0 and cluster0 respectively.
> > > > > > > - */
> > > > > > > - switch (win->data->phys_id) {
> > > > > > > - case ROCKCHIP_VOP2_SMART1:
> > > > > > > - case ROCKCHIP_VOP2_ESMART1:
> > > > > > > - case ROCKCHIP_VOP2_CLUSTER1:
> > > > > > > - continue;
> > > > > > > - }
> > > > > > > - }
> > > > >
> > > > > Let me say that a "window" in the Rockchip terminology is what a plane
> > > > > is in the DRM terminology, and a video port (vp) corresponds to a crtc.
> > > > >
> > > > > On the RK3566 some windows do not have their own framebuffer, for
> > > > > example the smart1 window always shows what the smart0 window shows.
> > > > > This "feature" makes these windows unusable as planes, so the idea is to
> > > > > simply not use them.
> > > > >
> > > > > If skipping these windows from initialization results in a NULL pointer
> > > > > deref then there's something wrong in the driver logic, but it's not the
> > > > > lines you are removing here that are wrong. Nothing should prevent you
> > > > > from using multiple VPs when the unusable windows on RK3566 are not
> > > > > registered.
> > > >
> > > > I'll tell you what I "think" is causing the null pointer deref, but like
> > > > always I reserve the right to be wrong.
> > > >
> > > > The driver checks for the primary planes and assigns them to active
> > > > video ports. For the rk3566 it's skipping the 3 planes mentioned
> > > > above for the RK3566. As a result if there is more than 1 active video
> > > > port for the rk3566 (or more than 2 for the rk3568 I'm assuming) it
> > > > doesn't set vp->primary_plane = win for that port. Slightly below that
> > > > it iterates through the active video ports again and runs
> > > > drm_crtc_init_with_planes on them. Only in this case one of the ports
> > > > doesn't have a plane, which is where it does the null pointer deref,
> > > > because the plane is null.
> > >
> > > Yes, you're right, that is likely the problem.
> > >
> > > On RK3568 we have four windows that could serve as primary plane, so
> > > there's enough for each of the three VPs. On RK3566 we only have two
> > > windows, so once we use more than two active VPs we are running out of
> > > primary planes.
> >
> > The datasheet states that it supports up to 2 simultaneous displays, so
> > I think a hard limit of 2 VPs is fine for the rk3566.
> >
> > >
> > > I am writing these numbers because I think Esmart0-win0 could be used as
> > > primary plane, but is marked as DRM_PLANE_TYPE_OVERLAY in
> > > rk3568_vop_win_data[]. See the attached patch, if that's the problem it
> > > should resolve your issue.
> >
> > This patch did indeed resolve my issue, and also allowed me to get both
> > HDMI and DSI working simultaneously. It's not perfect (right now I'm
> > displaying a 640x480 image on a 1080p display since my DSI panel runs
> > at 640x480), but it works and that's great.
>
> I just sent this patch for inclusion. Could you provide your formal
> tested-by: tag?
>
> Sascha
Done. Thank you once again for all your help.
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C0a2e9af8accf4b7e720708da9f979700%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637997770679554061%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=k56y1R3snN7uokq64C0Yx0K%2F0CbYktI%2BCeGyX5XHM%2Fk%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-09-26 12:59 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 18:02 [PATCH] drm/rockchip: vop2: Fix Null Pointer Dereference on Multiple VPs Chris Morgan
2022-09-12 18:02 ` Chris Morgan
2022-09-13 6:55 ` Michael Riesch
2022-09-13 6:55 ` Michael Riesch
2022-09-13 9:10 ` Piotr Oniszczuk
2022-09-13 15:36 ` Chris Morgan
2022-09-13 15:36 ` Chris Morgan
2022-09-14 6:49 ` Sascha Hauer
2022-09-14 6:49 ` Sascha Hauer
2022-09-14 13:04 ` Chris Morgan
2022-09-14 13:04 ` Chris Morgan
2022-09-14 13:35 ` Sascha Hauer
2022-09-14 13:35 ` Sascha Hauer
2022-09-15 15:13 ` Chris Morgan
2022-09-15 15:13 ` Chris Morgan
2022-09-26 8:17 ` Sascha Hauer
2022-09-26 8:17 ` Sascha Hauer
2022-09-26 12:58 ` Chris Morgan
2022-09-26 12:58 ` Chris Morgan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.