From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev Date: Tue, 1 Aug 2017 10:00:36 +0800 Message-ID: <597FE0C4.4000808@rock-chips.com> References: <1501494569-6825-1-git-send-email-mark.yao@rock-chips.com> <1501494601-7140-1-git-send-email-mark.yao@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Daniel Vetter , Emil Velikov Cc: LAKML , linux-rockchip , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel List-Id: linux-rockchip.vger.kernel.org On 2017年07月31日 20:28, Daniel Vetter wrote: > On Mon, Jul 31, 2017 at 1:57 PM, Emil Velikov wrote: >> On 31 July 2017 at 10:50, Mark Yao wrote: >>> Since fb_helper is not a pointer on rockchip_drm_private, it's no >>> need to check pointer. >>> >>> Kms hotplug event may race into fbdev helper initial, and fb_helper->dev >>> may be NULL pointer, that would cause the bug: >>> >>> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000 >>> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP >>> [ 0.736648] Modules linked in: >>> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20 >>> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT) >>> [ 0.738020] Workqueue: events cdn_dp_pd_event_work >>> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000 >>> [ 0.739109] PC is at mutex_lock+0x14/0x44 >>> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114 >>> [ 0.756253] [] mutex_lock+0x14/0x44 >>> [ 0.756260] [] drm_fb_helper_hotplug_event+0x30/0x114 >>> [ 0.756271] [] rockchip_drm_output_poll_changed+0x18/0x20 >>> [ 0.756280] [] drm_kms_helper_hotplug_event+0x28/0x34 >>> [ 0.756286] [] cdn_dp_pd_event_work+0x394/0x3c4 >>> [ 0.756295] [] process_one_work+0x218/0x3e0 >>> [ 0.756302] [] worker_thread+0x2e8/0x404 >>> [ 0.756308] [] kthread+0xe8/0xf0 >>> [ 0.756316] [] ret_from_fork+0x10/0x40 >>> >>> Signed-off-by: Mark Yao >>> --- >>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> index 81f9548..e6bd0f4 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> @@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev) >>> struct rockchip_drm_private *private = dev->dev_private; >>> struct drm_fb_helper *fb_helper = &private->fbdev_helper; >>> >>> - if (fb_helper) >>> + if (fb_helper->dev) >>> drm_fb_helper_hotplug_event(fb_helper); >> Food for thought: >> >> Quick grep shows that no other drivers have such a ->dev check. Does >> this mean that either the issue is rockchip specific? >> If not, one could look into resolving the problem directly in drm core. >> >> Or at least update the other users, so they don't stumble upon the problem? > The fbdev helpers support already handling hotplug events before you > have finalized the fbdev setup. Please read the kerneldoc for the > various fbdev functions, they explain what you should be doing. This > hack here should indeed not be needed. > -Daniel Hi Daniel Right, the doc[0] already detail this: It is possible, though perhaps somewhat tricky, to implement race-free hotplug detection using the fbdev helpers. The drm_fb_helper_prepare() helper must be called first to initialize the minimum required to make hotplug detection work.Drivers also need to make sure to properly set up the drm_mode_config.funcs member. After calling drm_kms_helper_poll_init() it is safe to enable interrupts and start processing hotplug events. The problem is drm/rockchip do the wrong initial, call drm_kms_helper_poll_init before fbdev setup. will fix it at next version. [0]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html#fbdev-helper-functions-reference Best regards. -- Ｍark Yao From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Tue, 1 Aug 2017 10:00:36 +0800 Subject: [PATCH 6/6] drm/rockchip: fix race with kms hotplug and fbdev In-Reply-To: References: <1501494569-6825-1-git-send-email-mark.yao@rock-chips.com> <1501494601-7140-1-git-send-email-mark.yao@rock-chips.com> Message-ID: <597FE0C4.4000808@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2017?07?31? 20:28, Daniel Vetter wrote: > On Mon, Jul 31, 2017 at 1:57 PM, Emil Velikov wrote: >> On 31 July 2017 at 10:50, Mark Yao wrote: >>> Since fb_helper is not a pointer on rockchip_drm_private, it's no >>> need to check pointer. >>> >>> Kms hotplug event may race into fbdev helper initial, and fb_helper->dev >>> may be NULL pointer, that would cause the bug: >>> >>> [ 0.735411] [00000200] *pgd=00000000f6ffe003, *pud=00000000f6ffe003, *pmd=0000000000000000 >>> [ 0.736156] Internal error: Oops: 96000005 [#1] PREEMPT SMP >>> [ 0.736648] Modules linked in: >>> [ 0.736930] CPU: 2 PID: 20 Comm: kworker/2:0 Not tainted 4.4.41 #20 >>> [ 0.737480] Hardware name: Rockchip RK3399 Board rev2 (BOX) (DT) >>> [ 0.738020] Workqueue: events cdn_dp_pd_event_work >>> [ 0.738447] task: ffffffc0f21f3100 ti: ffffffc0f2218000 task.ti: ffffffc0f2218000 >>> [ 0.739109] PC is at mutex_lock+0x14/0x44 >>> [ 0.739469] LR is at drm_fb_helper_hotplug_event+0x30/0x114 >>> [ 0.756253] [] mutex_lock+0x14/0x44 >>> [ 0.756260] [] drm_fb_helper_hotplug_event+0x30/0x114 >>> [ 0.756271] [] rockchip_drm_output_poll_changed+0x18/0x20 >>> [ 0.756280] [] drm_kms_helper_hotplug_event+0x28/0x34 >>> [ 0.756286] [] cdn_dp_pd_event_work+0x394/0x3c4 >>> [ 0.756295] [] process_one_work+0x218/0x3e0 >>> [ 0.756302] [] worker_thread+0x2e8/0x404 >>> [ 0.756308] [] kthread+0xe8/0xf0 >>> [ 0.756316] [] ret_from_fork+0x10/0x40 >>> >>> Signed-off-by: Mark Yao >>> --- >>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> index 81f9548..e6bd0f4 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>> @@ -170,7 +170,7 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev) >>> struct rockchip_drm_private *private = dev->dev_private; >>> struct drm_fb_helper *fb_helper = &private->fbdev_helper; >>> >>> - if (fb_helper) >>> + if (fb_helper->dev) >>> drm_fb_helper_hotplug_event(fb_helper); >> Food for thought: >> >> Quick grep shows that no other drivers have such a ->dev check. Does >> this mean that either the issue is rockchip specific? >> If not, one could look into resolving the problem directly in drm core. >> >> Or at least update the other users, so they don't stumble upon the problem? > The fbdev helpers support already handling hotplug events before you > have finalized the fbdev setup. Please read the kerneldoc for the > various fbdev functions, they explain what you should be doing. This > hack here should indeed not be needed. > -Daniel Hi Daniel Right, the doc[0] already detail this: It is possible, though perhaps somewhat tricky, to implement race-free hotplug detection using the fbdev helpers. The drm_fb_helper_prepare() helper must be called first to initialize the minimum required to make hotplug detection work.Drivers also need to make sure to properly set up the drm_mode_config.funcs member. After calling drm_kms_helper_poll_init() it is safe to enable interrupts and start processing hotplug events. The problem is drm/rockchip do the wrong initial, call drm_kms_helper_poll_init before fbdev setup. will fix it at next version. [0]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html#fbdev-helper-functions-reference Best regards. -- ?ark Yao