| Message ID | 20231025092711.851168-3-dmitry.baryshkov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 21133266ca12f82b6e59c9711258cca2097c167c |
| Headers | show |
| Series | drm/msm/dp: fix DP subconnector handling | expand |
On 10/25/2023 2:23 AM, Dmitry Baryshkov wrote: > While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: > support setting the DP subconnector type") I had the patch [1] in my > tree. I haven't noticed that it was a dependency for the commit in > question. Mea culpa. > > Since the patch has not landed yet (and even was not reviewed) > and since one of the bridges erroneously uses USB connector type instead > of DP, attach the property directly from the MSM DP driver. > > This fixes the following oops on DP HPD event: > > drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) > dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) > dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) > hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) > kthread (kernel/kthread.c:388) > ret_from_fork (arch/arm64/kernel/entry.S:858) > > [1] https://patchwork.freedesktop.org/patch/555530/ Tested-by: Jessica Zhang <quic_jesszhan@quicinc.com> # SC7280 > > Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c > index 40e7344180e3..e3bdd7dd4cdc 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > @@ -345,6 +345,9 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr > if (IS_ERR(connector)) > return connector; > > + if (!dp_display->is_edp) > + drm_connector_attach_dp_subconnector_property(connector); > + > drm_connector_attach_encoder(connector, encoder); > > return connector; > -- > 2.42.0 >
On Wed, Oct 25, 2023 at 12:23:10PM +0300, Dmitry Baryshkov wrote: > While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: > support setting the DP subconnector type") I had the patch [1] in my > tree. I haven't noticed that it was a dependency for the commit in > question. Mea culpa. This also broke boot on the Lenovo ThinkPad X13s. Would be nice to get this fixed ASAP so that further people don't have to debug this known regression. > Since the patch has not landed yet (and even was not reviewed) > and since one of the bridges erroneously uses USB connector type instead > of DP, attach the property directly from the MSM DP driver. > > This fixes the following oops on DP HPD event: > > drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) > dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) > dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) > hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) > kthread (kernel/kthread.c:388) > ret_from_fork (arch/arm64/kernel/entry.S:858) This only says where the oops happened, it doesn't necessarily in itself indicate an oops at all or that in this case it's a NULL pointer dereference. On the X13s I'm seeing the NULL deref in a different path during boot, and when this happens after a deferred probe (due to the panel lookup mess) it hangs the machine, which makes it a bit of a pain to debug: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000060 ... CPU: 4 PID: 57 Comm: kworker/u16:1 Not tainted 6.7.0-rc1 #4 Hardware name: Qualcomm QRD, BIOS 6.0.220110.BOOT.MXF.1.1-00470-MAKENA-1 01/10/2022 ... Call trace: drm_object_property_set_value+0x0/0x88 [drm] dp_display_process_hpd_high+0xa0/0x14c [msm] dp_hpd_plug_handle.constprop.0.isra.0+0x90/0x110 [msm] dp_bridge_atomic_enable+0x184/0x21c [msm] edp_bridge_atomic_enable+0x60/0x94 [msm] drm_atomic_bridge_chain_enable+0x54/0xc8 [drm] drm_atomic_helper_commit_modeset_enables+0x194/0x26c [drm_kms_helper] msm_atomic_commit_tail+0x204/0x804 [msm] commit_tail+0xa4/0x18c [drm_kms_helper] drm_atomic_helper_commit+0x19c/0x1b0 [drm_kms_helper] drm_atomic_commit+0xa4/0x104 [drm] drm_client_modeset_commit_atomic+0x22c/0x298 [drm] drm_client_modeset_commit_locked+0x60/0x1c0 [drm] drm_client_modeset_commit+0x30/0x58 [drm] __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc [drm_kms_helper] drm_fb_helper_set_par+0x30/0x4c [drm_kms_helper] fbcon_init+0x224/0x49c visual_init+0xb0/0x108 do_bind_con_driver.isra.0+0x19c/0x38c do_take_over_console+0x140/0x1ec do_fbcon_takeover+0x6c/0xe4 fbcon_fb_registered+0x180/0x1f0 register_framebuffer+0x19c/0x228 __drm_fb_helper_initial_config_and_unlock+0x2e8/0x4e8 [drm_kms_helper] drm_fb_helper_initial_config+0x3c/0x4c [drm_kms_helper] msm_fbdev_client_hotplug+0x84/0xcc [msm] drm_client_register+0x5c/0xa0 [drm] msm_fbdev_setup+0x94/0x148 [msm] msm_drm_bind+0x3d0/0x42c [msm] try_to_bring_up_aggregate_device+0x1ec/0x2f4 __component_add+0xa8/0x194 component_add+0x14/0x20 dp_display_probe+0x278/0x41c [msm] > [1] https://patchwork.freedesktop.org/patch/555530/ > > Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Tested-by: Johan Hovold <johan+linaro@kernel.org> Johan
On 21.11.23 19:50, Abhinav Kumar wrote: > On 11/21/2023 9:57 AM, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 15.11.23 19:06, Abhinav Kumar wrote: >>> On 11/15/2023 12:06 AM, Johan Hovold wrote: >>>> On Wed, Oct 25, 2023 at 12:23:10PM +0300, Dmitry Baryshkov wrote: >>>>> While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: >>>>> support setting the DP subconnector type") I had the patch [1] in my >>>>> tree. I haven't noticed that it was a dependency for the commit in >>>>> question. Mea culpa. >>>> >>>> This also broke boot on the Lenovo ThinkPad X13s. >>>> >>>> Would be nice to get this fixed ASAP so that further people don't have >>>> to debug this known regression. >>> >>> I will queue this patch for -fixes rightaway. >> >> Thx. I noticed that this fix is still not in -next. I then investigated >> and I found it was applied on Thursday last week here: >> https://gitlab.freedesktop.org/drm/msm/-/commits/msm-fixes?ref_type=heads >> >> Makes me wonder: when will that patch go to a branch that is included in >> -next? And when will it move on towards mainline? > > This has been included in a pull request for 6.7-rc3 to the DRM tree and > shall make it to -next from there. Ahh, great, thx, I was slowly getting worried. Ciao, Thorsten >>>>> Since the patch has not landed yet (and even was not reviewed) >>>>> and since one of the bridges erroneously uses USB connector type >>>>> instead >>>>> of DP, attach the property directly from the MSM DP driver. >>>>> >>>>> This fixes the following oops on DP HPD event: >>>>> >>>>> drm_object_property_set_value >>>>> (drivers/gpu/drm/drm_mode_object.c:288) >>>>> dp_display_process_hpd_high >>>>> (drivers/gpu/drm/msm/dp/dp_display.c:402) >>>>> dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) >>>>> hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) >>>>> kthread (kernel/kthread.c:388) >>>>> ret_from_fork (arch/arm64/kernel/entry.S:858) >>>> >>>> This only says where the oops happened, it doesn't necessarily in >>>> itself >>>> indicate an oops at all or that in this case it's a NULL pointer >>>> dereference. >>>> >>>> On the X13s I'm seeing the NULL deref in a different path during boot, >>>> and when this happens after a deferred probe (due to the panel lookup >>>> mess) it hangs the machine, which makes it a bit of a pain to debug: >>>> >>>> Unable to handle kernel NULL pointer dereference at virtual >>>> address 0000000000000060 >>>> ... >>>> CPU: 4 PID: 57 Comm: kworker/u16:1 Not tainted 6.7.0-rc1 #4 >>>> Hardware name: Qualcomm QRD, BIOS >>>> 6.0.220110.BOOT.MXF.1.1-00470-MAKENA-1 01/10/2022 >>>> ... >>>> Call trace: >>>> drm_object_property_set_value+0x0/0x88 [drm] >>>> dp_display_process_hpd_high+0xa0/0x14c [msm] >>>> dp_hpd_plug_handle.constprop.0.isra.0+0x90/0x110 [msm] >>>> dp_bridge_atomic_enable+0x184/0x21c [msm] >>>> edp_bridge_atomic_enable+0x60/0x94 [msm] >>>> drm_atomic_bridge_chain_enable+0x54/0xc8 [drm] >>>> drm_atomic_helper_commit_modeset_enables+0x194/0x26c >>>> [drm_kms_helper] >>>> msm_atomic_commit_tail+0x204/0x804 [msm] >>>> commit_tail+0xa4/0x18c [drm_kms_helper] >>>> drm_atomic_helper_commit+0x19c/0x1b0 [drm_kms_helper] >>>> drm_atomic_commit+0xa4/0x104 [drm] >>>> drm_client_modeset_commit_atomic+0x22c/0x298 [drm] >>>> drm_client_modeset_commit_locked+0x60/0x1c0 [drm] >>>> drm_client_modeset_commit+0x30/0x58 [drm] >>>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>>> [drm_kms_helper] >>>> drm_fb_helper_set_par+0x30/0x4c [drm_kms_helper] >>>> fbcon_init+0x224/0x49c >>>> visual_init+0xb0/0x108 >>>> do_bind_con_driver.isra.0+0x19c/0x38c >>>> do_take_over_console+0x140/0x1ec >>>> do_fbcon_takeover+0x6c/0xe4 >>>> fbcon_fb_registered+0x180/0x1f0 >>>> register_framebuffer+0x19c/0x228 >>>> __drm_fb_helper_initial_config_and_unlock+0x2e8/0x4e8 >>>> [drm_kms_helper] >>>> drm_fb_helper_initial_config+0x3c/0x4c [drm_kms_helper] >>>> msm_fbdev_client_hotplug+0x84/0xcc [msm] >>>> drm_client_register+0x5c/0xa0 [drm] >>>> msm_fbdev_setup+0x94/0x148 [msm] >>>> msm_drm_bind+0x3d0/0x42c [msm] >>>> try_to_bring_up_aggregate_device+0x1ec/0x2f4 >>>> __component_add+0xa8/0x194 >>>> component_add+0x14/0x20 >>>> dp_display_probe+0x278/0x41c [msm] >>>> >>>>> [1] https://patchwork.freedesktop.org/patch/555530/ >>>>> >>>>> Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector >>>>> type") >>>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> >>>> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> >>>> Tested-by: Johan Hovold <johan+linaro@kernel.org> >>>> >>> >>> Thanks ! >>> >>>> Johan > >
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index 40e7344180e3..e3bdd7dd4cdc 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -345,6 +345,9 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr if (IS_ERR(connector)) return connector; + if (!dp_display->is_edp) + drm_connector_attach_dp_subconnector_property(connector); + drm_connector_attach_encoder(connector, encoder); return connector;
While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") I had the patch [1] in my tree. I haven't noticed that it was a dependency for the commit in question. Mea culpa. Since the patch has not landed yet (and even was not reviewed) and since one of the bridges erroneously uses USB connector type instead of DP, attach the property directly from the MSM DP driver. This fixes the following oops on DP HPD event: drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) kthread (kernel/kthread.c:388) ret_from_fork (arch/arm64/kernel/entry.S:858) [1] https://patchwork.freedesktop.org/patch/555530/ Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++ 1 file changed, 3 insertions(+)