drm/msm: probe deferral fixes
Subsystems (4)
v2 cover drm/msm: probe deferral fixes
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
The underlying problem is lifetime issues due to careless use of device-managed resources.
Specifically, device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
The following call chain and pseudo code serves as an illustration of the problem:
- platform_probe(pdev1)
- dp_display_probe()
- component_add()
- platform_probe(pdev2) // last component
- dp_display_probe() // d0
- component_add()
- try_to_bring_up_aggregate_device()
- devres_open_group(adev->parent) // d1
- msm_drm_bind()
- msm_drm_init()
- component_bind_all()
- for_each_component()
- component_bind()
- devres_open_group(&pdev->dev) // d2
- dp_display_bind()
- devm_kzalloc(&pdev->dev) // a1, OK
- devres_close_group(&pdev->dev) // d3
- dpu_kms_hw_init()
- for_each_panel()
- msm_dp_modeset_init()
- dp_display_request_irq()
- devm_request_irq(&pdev->dev) // a2, BUG
- if (pdev == pdev2 && condition)
- return -EPROBE_DEFER;
- if (error)
- component_unbind_all()
- for_each_component()
- component_unbind()
- dp_display_unbind()
- devres_release_group(&pdev->dev) // d4, only a1 is freed
- if (error)
- devres_release_group(adev->parent) // d5
The device-managed allocation a2 is buggy as its lifetime is tied to the component platform device and will not be released when the aggregate device bind fails (e.g. due to a probe deferral).
When pdev2 is later probed again, the attempt to allocate the IRQ a second time will fail for pdev1 (which is still bound to its platform driver).
This series fixes the lifetime issues by tying the lifetime of a2 (and similar allocations) to the lifetime of the aggregate device so that a2 is released at d5.
In some cases, such has for the DP IRQ, the above situation can also be avoided by moving the allocation in question to the platform driver probe (d0) or component bind (between d2 and d3). But as doing so is not a general fix, this can be done later as a cleanup/optimisation.
Johan
Changes in v2
- use a custom devres action instead of amending the AUX bus interface (Doug)
- split sanity check fixes and cleanups per bridge type (Dmitry)
- add another Fixes tag for the missing bridge counter reset (Dmitry)
Johan Hovold (10):
drm/msm: fix use-after-free on probe deferral drm/msm/dp: fix memory corruption with too many bridges drm/msm/dsi: fix memory corruption with too many bridges drm/msm/hdmi: fix memory corruption with too many bridges drm/msm/dp: fix IRQ lifetime
drm/msm/dp: fix aux-bus EP lifetime
drm/msm/dp: fix bridge lifetime
drm/msm/hdmi: fix IRQ lifetime
drm/msm/dp: drop modeset sanity checks drm/msm/dsi: drop modeset sanity checks
drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++------- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- drivers/gpu/drm/msm/dsi/dsi.c | 9 +++++---- drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++++- drivers/gpu/drm/msm/msm_drv.c | 1 + 6 files changed, 37 insertions(+), 17 deletions(-)
Signature · 1 line
2.35.1
PATCH v2 1/10 drm/msm: fix use-after-free on probe deferral
The bridge counter was never reset when tearing down the DRM device so that stale pointers to deallocated structures would be accessed on the next tear down (e.g. after a second late bind deferral).
Given enough bridges and a few probe deferrals this could currently also lead to data beyond the bridge array being corrupted.
--- drivers/gpu/drm/msm/msm_drv.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/msm/msm_drv.c +1 -0
@@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)for (i =0 ; i < priv->num_bridges; i++)drm_bridge_remove (priv->bridges[i]); + priv->num_bridges =0 ;pm_runtime_get_sync (dev);msm_irq_uninstall (ddev);
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
Quoted 4 lines · Johan Hovold
The bridge counter was never reset when tearing down the DRM device so that stale pointers to deallocated structures would be accessed on the next tear down (e.g. after a second late bind deferral).
Quoted 8 lines
Given enough bridges and a few probe deferrals this could currently also lead to data beyond the bridge array being corrupted.
Fixes d28ea556267c ("drm/msm: properly add and remove internal bridges")Fixes a3376e3ec81c ("drm/msm: convert to drm_bridge")Cc [email protected] # 3.12Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 16 lines
--- drivers/gpu/drm/msm/msm_drv.c | 1 + 1 file changed, 1 insertion(+)drivers/gpu/drm/msm/msm_drv.c +1
-0@@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)for (i =0 ; i < priv->num_bridges; i++)drm_bridge_remove (priv->bridges[i]); + priv->num_bridges =0 ;pm_runtime_get_sync (dev);msm_irq_uninstall (ddev);
Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
In reply to Johan HovoldQuoted 4 lines · Johan Hovold
The bridge counter was never reset when tearing down the DRM device so that stale pointers to deallocated structures would be accessed on the next tear down (e.g. after a second late bind deferral).
Quoted 8 lines
Given enough bridges and a few probe deferrals this could currently also lead to data beyond the bridge array being corrupted.
Fixes d28ea556267c ("drm/msm: properly add and remove internal bridges")Fixes a3376e3ec81c ("drm/msm: convert to drm_bridge")Cc [email protected] # 3.12Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 16 lines
--- drivers/gpu/drm/msm/msm_drv.c | 1 + 1 file changed, 1 insertion(+)drivers/gpu/drm/msm/msm_drv.c +1
-0@@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)for (i =0 ; i < priv->num_bridges; i++)drm_bridge_remove (priv->bridges[i]); + priv->num_bridges =0 ;pm_runtime_get_sync (dev);msm_irq_uninstall (ddev);
PATCH v2 2/10 drm/msm/dp: fix memory corruption with too many bridges
In reply to Johan HovoldAdd the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
--- drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/gpu/drm/msm/dp/dp_display.c +6 -0
@@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,return -EINVAL ; priv = dev->dev_private; + +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + dp_display->drm_dev = dev; dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
Quoted 7 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")Cc [email protected] # 5.17Signed-off-by Johan Hovold <[email protected]>
Quoted 21 lines
--- drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/gpu/drm/msm/dp/dp_display.c +6
-0@@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,return -EINVAL ; priv = dev->dev_private; + +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + dp_display->drm_dev = dev; dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
Re: [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
In reply to Johan HovoldQuoted 7 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")Cc [email protected] # 5.17Signed-off-by Johan Hovold <[email protected]>
Signature · 2 lines
With best wishes Dmitry
PATCH v2 3/10 drm/msm/dsi: fix memory corruption with too many bridges
In reply to Johan HovoldAdd the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
--- drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/gpu/drm/msm/dsi/dsi.c +6 -0
@@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,return -EINVAL ; priv = dev->dev_private; + +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + msm_dsi->dev = dev; ret =msm_dsi_host_modeset_init (msm_dsi->host, dev);
Signature · 1 line
2.35.1
Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
Quoted 7 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes a689554ba6ed ("drm/msm: Initial add DSI connector support")Cc [email protected] # 4.1Signed-off-by Johan Hovold <[email protected]>
Quoted 21 lines
--- drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/gpu/drm/msm/dsi/dsi.c +6
-0@@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,return -EINVAL ; priv = dev->dev_private; + +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + msm_dsi->dev = dev; ret =msm_dsi_host_modeset_init (msm_dsi->host, dev);
Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
In reply to Johan HovoldQuoted 11 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes a689554ba6ed ("drm/msm: Initial add DSI connector support")Cc [email protected] # 4.1Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++ 1 file changed, 6 insertions(+)
Signature · 2 lines
With best wishes Dmitry
PATCH v2 4/10 drm/msm/hdmi: fix memory corruption with too many bridges
In reply to Johan HovoldAdd the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
--- drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/gpu/drm/msm/hdmi/hdmi.c +5 -0
@@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,struct platform_device *pdev = hdmi->pdev;int ret; +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + hdmi->dev = dev; hdmi->encoder = encoder;
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
Quoted 7 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes a3376e3ec81c ("drm/msm: convert to drm_bridge")Cc [email protected] # 3.12Signed-off-by Johan Hovold <[email protected]>
Quoted 20 lines
--- drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/gpu/drm/msm/hdmi/hdmi.c +5
-0@@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,struct platform_device *pdev = hdmi->pdev;int ret; +if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) { +DRM_DEV_ERROR (dev->dev,"too many bridges\n" ); +return -ENOSPC ; + } + hdmi->dev = dev; hdmi->encoder = encoder;
Re: [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
In reply to Johan HovoldQuoted 10 lines · Johan Hovold
Add the missing sanity check on the bridge counter to avoid corrupting data beyond the fixed-sized bridge array in case there are ever more than eight bridges.
Fixes a3376e3ec81c ("drm/msm: convert to drm_bridge")Cc [email protected] # 3.12Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++ 1 file changed, 5 insertions(+)
Signature · 2 lines
With best wishes Dmitry
PATCH v2 5/10 drm/msm/dp: fix IRQ lifetime
In reply to Johan HovoldDevice-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This is specifically true for the DP IRQ, which will otherwise remain requested so that the next bind attempt fails when requesting the IRQ a second time.
Since commit c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus") this can happen when the aux-bus panel driver has not yet been loaded so that probe is deferred.
Fix this by tying the device-managed lifetime of the DP IRQ to the DRM device so that it is released when bind fails.
--- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/msm/dp/dp_display.c +1 -1
@@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)return -EINVAL ; } - rc =devm_request_irq (&dp->pdev->dev, dp->irq, + rc =devm_request_irq (dp_display->drm_dev->dev, dp->irq, dp_display_irq_handler,IRQF_TRIGGER_HIGH ,"dp_display_isr" , dp);if (rc <0 ) {
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime
Quoted 12 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This is specifically true for the DP IRQ, which will otherwise remain requested so that the next bind attempt fails when requesting the IRQ a second time.
Since commit
c3bf8e21b38a("drm/msm/dp: Add eDP support viaaux_bus") this can happen when the aux-bus panel driver has not yet been loaded so that probe is deferred.
Quoted 7 lines
Fix this by tying the device-managed lifetime of the DP IRQ to the DRM device so that it is released when bind fails.
Fixes c943b4948b58 ("drm/msm/dp: add displayPort driver support")Cc [email protected] # 5.10Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 17 lines
--- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/msm/dp/dp_display.c +1
-1@@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)return -EINVAL ; } - rc =devm_request_irq (&dp->pdev->dev, dp->irq, + rc =devm_request_irq (dp_display->drm_dev->dev, dp->irq, dp_display_irq_handler,IRQF_TRIGGER_HIGH ,"dp_display_isr" , dp);if (rc <0 ) {
PATCH v2 6/10 drm/msm/dp: fix aux-bus EP lifetime
In reply to Johan HovoldDevice-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device when binding is later retried and a second attempt to allocate the resources is made.
For the DP aux-bus, an attempt to populate the bus a second time will simply fail ("DP AUX EP device already populated").
Fix this by tying the lifetime of the EP device to the DRM device rather than DP controller platform device.
--- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/dp/dp_display.c +12 -1
@@ -1535,6 +1535,11 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor) } } +static void of_dp_aux_depopulate_bus_void (void *data) +{ +of_dp_aux_depopulate_bus (data); +} +static int dp_display_get_next_bridge (struct msm_dp *dp) {int rc;
@@ -1559,10 +1564,16 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)* panel driver is probed asynchronously but is the best we * can do without a bigger driver reorganization. */ - rc =devm_of_dp_aux_populate_ep_devices (dp_priv->aux); + rc =of_dp_aux_populate_bus (dp_priv->aux,NULL );of_node_put (aux_bus);if (rc) goto error; + + rc =devm_add_action_or_reset (dp->drm_dev->dev, + of_dp_aux_depopulate_bus_void, + dp_priv->aux); +if (rc) + goto error; }else if (dp->is_edp) {DRM_ERROR ("eDP aux_bus not found\n" );return -ENODEV ;
Signature · 1 line
2.35.1
Re: [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
Hi,
Quoted 15 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device when binding is later retried and a second attempt to allocate the resources is made.
For the DP aux-bus, an attempt to populate the bus a second time will simply fail ("DP AUX EP device already populated").
Fix this by tying the lifetime of the EP device to the DRM device rather than DP controller platform device.
Quoted 6 lines
Fixes c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")Cc [email protected] # 5.19Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
This seems fine to me as a short term fix until we get the DP AUX populating moved to probe.
Re: [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
Quoted 23 lines · Doug Anderson
Hi,
Quoted 14 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device when binding is later retried and a second attempt to allocate the resources is made.
For the DP aux-bus, an attempt to populate the bus a second time will simply fail ("DP AUX EP device already populated").
Fix this by tying the lifetime of the EP device to the DRM device rather than DP controller platform device.
Quoted 6 lines
Fixes c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")Cc [email protected] # 5.19Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
Quoted 4 lines
This seems fine to me as a short term fix until we get the DP AUX populating moved to probe.
Reviewed-by Douglas Anderson <[email protected]>
PATCH v2 7/10 drm/msm/dp: fix bridge lifetime
In reply to Johan HovoldDevice-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device when binding is later retried and a second attempt to allocate the resources is made.
For the DP bridges, previously allocated bridges will leak on probe deferral.
Fix this by amending the DP parser interface and tying the lifetime of the bridge device to the DRM device rather than DP platform device.
--- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/msm/dp/dp_display.c +1 -1
@@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)* For DisplayPort interfaces external bridges are optional, so * silently ignore an error if one is not present (-ENODEV). */ - rc =dp_parser_find_next_bridge (dp_priv->parser); + rc =devm_dp_parser_find_next_bridge (dp->drm_dev->dev, dp_priv->parser);if (!dp->is_edp && rc == -ENODEV )return 0 ;
drivers/gpu/drm/msm/dp/dp_parser.c +3 -3
@@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)return 0 ; } -int dp_parser_find_next_bridge (struct dp_parser *parser) +int devm_dp_parser_find_next_bridge (struct device *dev,struct dp_parser *parser) { -struct device *dev = &parser->pdev->dev; +struct platform_device *pdev = parser->pdev;struct drm_bridge *bridge; - bridge =devm_drm_of_get_bridge (dev, dev->of_node,1 ,0 ); + bridge =devm_drm_of_get_bridge (dev, pdev->dev.of_node,1 ,0 );if (IS_ERR (bridge))return PTR_ERR (bridge);
drivers/gpu/drm/msm/dp/dp_parser.h +3 -2
@@ -138,8 +138,9 @@ struct dp_parser {struct dp_parser *dp_parser_get (struct platform_device *pdev);/** -* dp_parser_find_next_bridge() - find an additional bridge to DP +* devm_dp_parser_find_next_bridge() - find an additional bridge to DP * +* @dev: device to tie bridge lifetime to * @parser: dp_parser data from client * * This function is used to find any additional bridge attached to
@@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);* * Return: 0 if able to get the bridge, otherwise negative errno for failure. */ -int dp_parser_find_next_bridge (struct dp_parser *parser); +int devm_dp_parser_find_next_bridge (struct device *dev,struct dp_parser *parser);#endif
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime
Quoted 11 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device when binding is later retried and a second attempt to allocate the resources is made.
For the DP bridges, previously allocated bridges will leak on probe deferral.
Quoted 7 lines
Fix this by amending the DP parser interface and tying the lifetime of the bridge device to the DRM device rather than DP platform device.
Fixes c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")Cc [email protected] # 5.19Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 62 lines
--- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-)drivers/gpu/drm/msm/dp/dp_display.c +1
-1@@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)* For DisplayPort interfaces external bridges are optional, so * silently ignore an error if one is not present (-ENODEV). */ - rc =dp_parser_find_next_bridge (dp_priv->parser); + rc =devm_dp_parser_find_next_bridge (dp->drm_dev->dev, dp_priv->parser);if (!dp->is_edp && rc == -ENODEV )return 0 ;drivers/gpu/drm/msm/dp/dp_parser.c +3
-3@@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)return 0 ; } -int dp_parser_find_next_bridge (struct dp_parser *parser) +int devm_dp_parser_find_next_bridge (struct device *dev,struct dp_parser *parser) { -struct device *dev = &parser->pdev->dev; +struct platform_device *pdev = parser->pdev;struct drm_bridge *bridge; - bridge =devm_drm_of_get_bridge (dev, dev->of_node,1 ,0 ); + bridge =devm_drm_of_get_bridge (dev, pdev->dev.of_node,1 ,0 );if (IS_ERR (bridge))return PTR_ERR (bridge);drivers/gpu/drm/msm/dp/dp_parser.h +3
-2@@ -138,8 +138,9 @@ struct dp_parser {struct dp_parser *dp_parser_get (struct platform_device *pdev);/** -* dp_parser_find_next_bridge() - find an additional bridge to DP +* devm_dp_parser_find_next_bridge() - find an additional bridge to DP * +* @dev: device to tie bridge lifetime to * @parser: dp_parser data from client * * This function is used to find any additional bridge attached to @@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);* * Return: 0 if able to get the bridge, otherwise negative errno for failure. */ -int dp_parser_find_next_bridge (struct dp_parser *parser); +int devm_dp_parser_find_next_bridge (struct device *dev,struct dp_parser *parser);#endif
PATCH v2 8/10 drm/msm/hdmi: fix IRQ lifetime
In reply to Johan HovoldDevice-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This is specifically true for the HDMI IRQ, which will otherwise remain requested so that the next bind attempt fails when requesting the IRQ a second time.
Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM device so that it is released when bind fails.
--- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/msm/hdmi/hdmi.c +1 -1
@@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, goto fail; } - ret =devm_request_irq (&pdev->dev, hdmi->irq, + ret =devm_request_irq (dev->dev, hdmi->irq, msm_hdmi_irq,IRQF_TRIGGER_HIGH ,"hdmi_isr" , hdmi);if (ret <0 ) {
Signature · 1 line
2.35.1
Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
Quoted 36 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This is specifically true for the HDMI IRQ, which will otherwise remain requested so that the next bind attempt fails when requesting the IRQ a second time.
Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM device so that it is released when bind fails.
Fixes 067fef372c73 ("drm/msm/hdmi: refactor bind/init")Cc [email protected] # 3.19Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Tested-by Kuogee Hsieh <[email protected]>Reviewed-by Kuogee Hsieh <[email protected]>--- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/msm/hdmi/hdmi.c +1
-1@@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, goto fail; } - ret =devm_request_irq (&pdev->dev, hdmi->irq, + ret =devm_request_irq (dev->dev, hdmi->irq, msm_hdmi_irq,IRQF_TRIGGER_HIGH ,"hdmi_isr" , hdmi);if (ret <0 ) {
Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
Quoted 13 lines · Kuogee Hsieh
Quoted 11 lines · Johan Hovold
Device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
This is specifically true for the HDMI IRQ, which will otherwise remain requested so that the next bind attempt fails when requesting the IRQ a second time.
Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM device so that it is released when bind fails.
Quoted 7 lines
Quoted 7 lines
Fixes 067fef372c73 ("drm/msm/hdmi: refactor bind/init")Cc [email protected] # 3.19Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Tested-by Kuogee Hsieh <[email protected]>Reviewed-by Kuogee Hsieh <[email protected]>
I believe you meant:
here (i.e. without the '>' quotes). Otherwise the tooling may not pick these up.
Thanks for reviewing and testing.
Johan
PATCH v2 9/10 drm/msm/dp: drop modeset sanity checks
In reply to Johan HovoldDrop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
--- drivers/gpu/drm/msm/dp/dp_display.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/gpu/drm/msm/dp/dp_display.c +1 -6
@@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)int msm_dp_modeset_init (struct msm_dp *dp_display,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;struct dp_display_private *dp_priv;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!dp_display) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Signature · 1 line
2.35.1
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 25 lines
--- drivers/gpu/drm/msm/dp/dp_display.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)drivers/gpu/drm/msm/dp/dp_display.c +1
-6@@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)int msm_dp_modeset_init (struct msm_dp *dp_display,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;struct dp_display_private *dp_priv;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!dp_display) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
In reply to Johan HovoldQuoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
The change LGTM, hence
I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Quoted 25 lines
--- drivers/gpu/drm/msm/dp/dp_display.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)drivers/gpu/drm/msm/dp/dp_display.c +1
-6@@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)int msm_dp_modeset_init (struct msm_dp *dp_display,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;struct dp_display_private *dp_priv;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!dp_display) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 7 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 8 lines
The change LGTM, hence
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Fixes c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for this otherwise as it's not a bug.
You also have to watch out for Sasha and his autosel scripts which will probably try to backport this to stable if it finds a Fixes tag.
Johan
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 17 lines · Johan Hovold
Quoted 7 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Quoted 8 lines
The change LGTM, hence
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Fixes c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Quoted 7 lines
Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for this otherwise as it's not a bug.
You also have to watch out for Sasha and his autosel scripts which will probably try to backport this to stable if it finds a Fixes tag.
Johan
Discussed with Rob on IRC, we will apply everything except the last two patches of this series in the -fixes and take these two for the next kernel rev push.
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 21 lines · Abhinav Kumar
Quoted 20 lines · Johan Hovold
Quoted 7 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Quoted 8 lines
The change LGTM, hence
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Fixes c943b4948b58 ("drm/msm/dp: add displayPort driver support")Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for this otherwise as it's not a bug.
Quoted 6 lines
Quoted 2 lines
You also have to watch out for Sasha and his autosel scripts which will probably try to backport this to stable if it finds a Fixes tag.
Discussed with Rob on IRC, we will apply everything except the last two patches of this series in the -fixes and take these two for the next kernel rev push.
So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you mean 6.1 and 6.2?
Johan
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 25 lines · Johan Hovold
Quoted 23 lines · Abhinav Kumar
Quoted 22 lines · Johan Hovold
Quoted 7 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Quoted 8 lines
The change LGTM, hence
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Fixes c943b4948b58 ("drm/msm/dp: add displayPort driver support")Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for this otherwise as it's not a bug.
You also have to watch out for Sasha and his autosel scripts which will probably try to backport this to stable if it finds a Fixes tag.
Quoted 8 lines
Quoted 3 lines
Discussed with Rob on IRC, we will apply everything except the last two patches of this series in the -fixes and take these two for the next kernel rev push.
So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you mean 6.1 and 6.2?
Johan
The fixes will go in 6.1 first.
The two follow-on cleanups in 6.2.
Thanks
Abhinav
Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
Quoted 7 lines · Abhinav Kumar
Quoted 6 lines · Johan Hovold
Quoted 3 lines · Abhinav Kumar
Discussed with Rob on IRC, we will apply everything except the last two patches of this series in the -fixes and take these two for the next kernel rev push.
Quoted 6 lines
Quoted 2 lines
So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you mean 6.1 and 6.2?
The fixes will go in 6.1 first.
The two follow-on cleanups in 6.2.
Ok, sounds good. Thanks.
Johan
PATCH v2 10/10 drm/msm/dsi: drop modeset sanity checks
In reply to Johan HovoldDrop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
--- drivers/gpu/drm/msm/dsi/dsi.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/gpu/drm/msm/dsi/dsi.c +1 -6
@@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)int msm_dsi_modeset_init (struct msm_dsi *msm_dsi,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!msm_dsi) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Signature · 1 line
2.35.1
Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Quoted 24 lines
--- drivers/gpu/drm/msm/dsi/dsi.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)drivers/gpu/drm/msm/dsi/dsi.c +1
-6@@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)int msm_dsi_modeset_init (struct msm_dsi *msm_dsi,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!msm_dsi) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
In reply to Johan HovoldQuoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>
The change LGTM, hence
I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
use")
Quoted 24 lines
--- drivers/gpu/drm/msm/dsi/dsi.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)drivers/gpu/drm/msm/dsi/dsi.c +1
-6@@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)int msm_dsi_modeset_init (struct msm_dsi *msm_dsi,struct drm_device *dev,struct drm_encoder *encoder) { -struct msm_drm_private *priv; +struct msm_drm_private *priv = dev->dev_private;int ret; -if (WARN_ON (!encoder) ||WARN_ON (!msm_dsi) ||WARN_ON (!dev)) -return -EINVAL ; - - priv = dev->dev_private; -if (priv->num_bridges ==ARRAY_SIZE (priv->bridges)) {DRM_DEV_ERROR (dev->dev,"too many bridges\n" );return -ENOSPC ;
Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
Quoted 11 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>The change LGTM, hence
Quoted 7 lines
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Fixes 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers beforeuse")
Same here. I wouldn't add a Fixes tag unless required by some DRM process.
Johan
Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
Quoted 17 lines · Johan Hovold
Quoted 16 lines · Abhinav Kumar
Quoted 5 lines · Johan Hovold
Drop the overly defensive modeset sanity checks of function parameters which have already been checked or used by the callers.
Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>The change LGTM, hence
Reviewed-by Abhinav Kumar <[email protected]>I think we can use below fixes tag so that we can pick up this entire series for the fixes cycle.
Quoted 7 lines
Quoted 2 lines
Fixes 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers beforeuse")
Same here. I wouldn't add a Fixes tag unless required by some DRM process.
Johan
Same response as the prev one, we will apply everything except the last two patches of this series in the -fixes and take these two for the next kernel rev push.
Hi Johan,
Quoted 96 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
The underlying problem is lifetime issues due to careless use of device-managed resources.
Specifically, device-managed resources allocated post component bind must be tied to the lifetime of the aggregate DRM device or they will not necessarily be released when binding of the aggregate device is deferred.
The following call chain and pseudo code serves as an illustration of the problem:
-
platform_probe(pdev1)
-dp_display_probe()
-component_add()-
platform_probe(pdev2)// last component
-dp_display_probe()// d0
-component_add()
-try_to_bring_up_aggregate_device()
-devres_open_group(adev->parent)// d1-
msm_drm_bind()
-msm_drm_init()
-component_bind_all()
-for_each_component()
-component_bind()
-devres_open_group(&pdev->dev)// d2
-dp_display_bind()
-devm_kzalloc(&pdev->dev)// a1, OK
-devres_close_group(&pdev->dev)// d3-
dpu_kms_hw_init()
-for_each_panel()
-msm_dp_modeset_init()
-dp_display_request_irq()
-devm_request_irq(&pdev->dev)// a2, BUG
- if (pdev == pdev2 && condition)
- return -EPROBE_DEFER;- if (error)
-component_unbind_all()
-for_each_component()
-component_unbind()
-dp_display_unbind()
-devres_release_group(&pdev->dev)// d4, only a1 is freed- if (error)
-devres_release_group(adev->parent)// d5The device-managed allocation a2 is buggy as its lifetime is tied to the component platform device and will not be released when the aggregate device bind fails (e.g. due to a probe deferral).
When pdev2 is later probed again, the attempt to allocate the IRQ a second time will fail for pdev1 (which is still bound to its platform driver).
This series fixes the lifetime issues by tying the lifetime of a2 (and similar allocations) to the lifetime of the aggregate device so that a2 is released at d5.
In some cases, such has for the DP IRQ, the above situation can also be avoided by moving the allocation in question to the platform driver probe (d0) or component bind (between d2 and d3). But as doing so is not a general fix, this can be done later as a cleanup/optimisation.
Johan
Changes in v2
- use a custom devres action instead of amending the AUX bus interface(Doug)- split sanity check fixes and cleanups per bridge type (Dmitry)
- add another Fixes tag for the missing bridge counter reset (Dmitry)Johan Hovold (10):
drm/msm: fix use-after-free on probe deferral drm/msm/dp: fix memory corruption with too many bridges drm/msm/dsi: fix memory corruption with too many bridges drm/msm/hdmi: fix memory corruption with too many bridges drm/msm/dp: fix IRQ lifetime
drm/msm/dp: fix aux-bus EP lifetime
drm/msm/dp: fix bridge lifetime
drm/msm/hdmi: fix IRQ lifetime
drm/msm/dp: drop modeset sanity checks drm/msm/dsi: drop modeset sanity checks
Quoted 8 lines
drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++------- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- drivers/gpu/drm/msm/dsi/dsi.c | 9 +++++---- drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++++- drivers/gpu/drm/msm/msm_drv.c | 1 + 6 files changed, 37 insertions(+), 17 deletions(-)
I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo Yoga C630), and both of them show the same issue:
[ 7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp [ 7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 [ 7.454406] Mem abort info: [ 7.454423] ESR = 0x0000000096000004 [ 7.454446] EC = 0x25: DABT (current EL), IL = 32 bits [ 7.454475] SET = 0, FnV = 0 [ 7.454494] EA = 0, S1PTW = 0 [ 7.454512] FSC = 0x04: level 0 translation fault [ 7.454539] Data abort info: [ 7.454556] ISV = 0, ISS = 0x00000004 [ 7.454577] CM = 0, WnR = 0 [ 7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000 [ 7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000 [ 7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl [ 7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26 [ 7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021 [ 7.454941] Workqueue: events_unbound deferred_probe_work_func [ 7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 7.455020] pc : dp_display_request_irq+0x50/0xdc [msm] [ 7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm] [ 7.455265] sp : ffff800008c1bb30 [ 7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000 [ 7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a [ 7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410 [ 7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff [ 7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff [ 7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000 [ 7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078 [ 7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880 [ 7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000 [ 7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000 [ 7.455698] Call trace: [ 7.455714] dp_display_request_irq+0x50/0xdc [msm] [ 7.455834] dp_display_probe+0xf8/0x4a4 [msm] [ 7.455950] platform_probe+0x6c/0xc4 [ 7.455976] really_probe+0xbc/0x2d4 [ 7.455999] __driver_probe_device+0x78/0xe0 [ 7.456025] driver_probe_device+0x3c/0x13c [ 7.456051] __device_attach_driver+0xb8/0x120 [ 7.456077] bus_for_each_drv+0x78/0xd0 [ 7.456105] __device_attach+0x9c/0x1a0 [ 7.456129] device_initial_probe+0x18/0x2c [ 7.456154] bus_probe_device+0x9c/0xa4 [ 7.456178] deferred_probe_work_func+0x88/0xc0 [ 7.456204] process_one_work+0x1d4/0x330 [ 7.456231] worker_thread+0x70/0x42c [ 7.456255] kthread+0x10c/0x110 [ 7.456278] ret_from_fork+0x10/0x20 [ 7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400) [ 7.456341] ---[ end trace 0000000000000000 ]---
This is from the sc8180x, sdm850 is the same call stack, just with different addresses.
I do have https://lore.kernel.org/all/[email protected]/ applied here which makes the 10th patch not apply cleanly.
It fails actually, but I applied it manually here.
Quoted 8 lines · Steev Klimaszewski
Hi Johan,
Quoted 4 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
Quoted 7 lines
Quoted 4 lines
In some cases, such has for the DP IRQ, the above situation can also be avoided by moving the allocation in question to the platform driver probe (d0) or component bind (between d2 and d3). But as doing so is not a general fix, this can be done later as a cleanup/optimisation.
I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo Yoga C630), and both of them show the same issue:
[ Copied the below from IRC instead as the formatting in your mail was off. ]
Quoted 52 lines
[ 7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp [ 7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 [ 7.454406] Mem abort info: [ 7.454423] ESR = 0x0000000096000004 [ 7.454446] EC = 0x25: DABT (current EL), IL = 32 bits [ 7.454475] SET = 0, FnV = 0 [ 7.454494] EA = 0, S1PTW = 0 [ 7.454512] FSC = 0x04: level 0 translation fault [ 7.454539] Data abort info: [ 7.454556] ISV = 0, ISS = 0x00000004 [ 7.454577] CM = 0, WnR = 0 [ 7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000 [ 7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000 [ 7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl [ 7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26 [ 7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021 [ 7.454941] Workqueue: events_unbound deferred_probe_work_func [ 7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 7.455020] pc : dp_display_request_irq+0x50/0xdc [msm] [ 7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm] [ 7.455265] sp : ffff800008c1bb30 [ 7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000 [ 7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a [ 7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410 [ 7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff [ 7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff [ 7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000 [ 7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078 [ 7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880 [ 7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000 [ 7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000 [ 7.455698] Call trace: [ 7.455714] dp_display_request_irq+0x50/0xdc [msm] [ 7.455834] dp_display_probe+0xf8/0x4a4 [msm] [ 7.455950] platform_probe+0x6c/0xc4 [ 7.455976] really_probe+0xbc/0x2d4 [ 7.455999] __driver_probe_device+0x78/0xe0 [ 7.456025] driver_probe_device+0x3c/0x13c [ 7.456051] __device_attach_driver+0xb8/0x120 [ 7.456077] bus_for_each_drv+0x78/0xd0 [ 7.456105] __device_attach+0x9c/0x1a0 [ 7.456129] device_initial_probe+0x18/0x2c [ 7.456154] bus_probe_device+0x9c/0xa4 [ 7.456178] deferred_probe_work_func+0x88/0xc0 [ 7.456204] process_one_work+0x1d4/0x330 [ 7.456231] worker_thread+0x70/0x42c [ 7.456255] kthread+0x10c/0x110 [ 7.456278] ret_from_fork+0x10/0x20 [ 7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400) [ 7.456341] ---[ end trace 0000000000000000 ]---
Quoted 6 lines
This is from the sc8180x, sdm850 is the same call stack, just with different addresses.
I do have
https://lore.kernel.org/all/[email protected]/
applied here which makes the 10th patch not apply cleanly.
Yeah, that is expected. You need to drop Dmitry's series first. Once you verified that this series works, you can add it back if you want but you then need to restore the device pointer used when allocating the irq in dp_display_request_irq():
- rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
+ rc = devm_request_irq(&dp->pdev->dev, dp->irq,
Quoted 1 line
It fails actually, but I applied it manually here.
Please drop that series and give this one another spin.
Johan
Steev Klimaszewski
Tested-byHi Johan,
Quoted 88 lines · Johan Hovold
Quoted 8 lines · Steev Klimaszewski
Hi Johan,
Quoted 4 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
Quoted 7 lines
Quoted 4 lines
In some cases, such has for the DP IRQ, the above situation can also be avoided by moving the allocation in question to the platform driver probe (d0) or component bind (between d2 and d3). But as doing so is not a general fix, this can be done later as a cleanup/optimisation.
I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo Yoga C630), and both of them show the same issue:
[ Copied the below from IRC instead as the formatting in your mail was off. ]
Quoted 52 lines
[ 7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp [ 7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 [ 7.454406] Mem abort info: [ 7.454423] ESR = 0x0000000096000004 [ 7.454446] EC = 0x25: DABT (current EL), IL = 32 bits [ 7.454475] SET = 0, FnV = 0 [ 7.454494] EA = 0, S1PTW = 0 [ 7.454512] FSC = 0x04: level 0 translation fault [ 7.454539] Data abort info: [ 7.454556] ISV = 0, ISS = 0x00000004 [ 7.454577] CM = 0, WnR = 0 [ 7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000 [ 7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000 [ 7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl [ 7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26 [ 7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021 [ 7.454941] Workqueue: events_unbound deferred_probe_work_func [ 7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 7.455020] pc : dp_display_request_irq+0x50/0xdc [msm] [ 7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm] [ 7.455265] sp : ffff800008c1bb30 [ 7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000 [ 7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a [ 7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410 [ 7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff [ 7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff [ 7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000 [ 7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078 [ 7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880 [ 7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000 [ 7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000 [ 7.455698] Call trace: [ 7.455714] dp_display_request_irq+0x50/0xdc [msm] [ 7.455834] dp_display_probe+0xf8/0x4a4 [msm] [ 7.455950] platform_probe+0x6c/0xc4 [ 7.455976] really_probe+0xbc/0x2d4 [ 7.455999] __driver_probe_device+0x78/0xe0 [ 7.456025] driver_probe_device+0x3c/0x13c [ 7.456051] __device_attach_driver+0xb8/0x120 [ 7.456077] bus_for_each_drv+0x78/0xd0 [ 7.456105] __device_attach+0x9c/0x1a0 [ 7.456129] device_initial_probe+0x18/0x2c [ 7.456154] bus_probe_device+0x9c/0xa4 [ 7.456178] deferred_probe_work_func+0x88/0xc0 [ 7.456204] process_one_work+0x1d4/0x330 [ 7.456231] worker_thread+0x70/0x42c [ 7.456255] kthread+0x10c/0x110 [ 7.456278] ret_from_fork+0x10/0x20 [ 7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400) [ 7.456341] ---[ end trace 0000000000000000 ]---Quoted 6 lines
This is from the sc8180x, sdm850 is the same call stack, just with different addresses.
I do have
https://lore.kernel.org/all/[email protected]/
applied here which makes the 10th patch not apply cleanly.Yeah, that is expected. You need to drop Dmitry's series first. Once you verified that this series works, you can add it back if you want but you then need to restore the device pointer used when allocating the irq in
dp_display_request_irq():(quoted diff) +1
-1- rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq, + rc = devm_request_irq(&dp->pdev->dev, dp->irq,
Quoted 5 lines
Quoted 1 line
It fails actually, but I applied it manually here.
Please drop that series and give this one another spin.
Johan
I thought as much but wasn't sure. Thanks for the clarification. With Dmitriy's patchset backed out, this series does work as expected.
Quoted 11 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
The underlying problem is lifetime issues due to careless use of device-managed resources.
Any chance of getting this merged for 6.1?
Johan
Quoted 26 lines
Changes in v2
- use a custom devres action instead of amending the AUX bus interface (Doug)
- split sanity check fixes and cleanups per bridge type (Dmitry)
- add another Fixes tag for the missing bridge counter reset (Dmitry)Johan Hovold (10):
drm/msm: fix use-after-free on probe deferral drm/msm/dp: fix memory corruption with too many bridges drm/msm/dsi: fix memory corruption with too many bridges drm/msm/hdmi: fix memory corruption with too many bridges drm/msm/dp: fix IRQ lifetime
drm/msm/dp: fix aux-bus EP lifetime
drm/msm/dp: fix bridge lifetime
drm/msm/hdmi: fix IRQ lifetime
drm/msm/dp: drop modeset sanity checks drm/msm/dsi: drop modeset sanity checksdrivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++------- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- drivers/gpu/drm/msm/dsi/dsi.c | 9 +++++---- drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++++- drivers/gpu/drm/msm/msm_drv.c | 1 + 6 files changed, 37 insertions(+), 17 deletions(-)
Quoted 6 lines · Johan Hovold
Quoted 5 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
Quoted 8 lines
Quoted 6 lines
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
The underlying problem is lifetime issues due to careless use of device-managed resources.
Any chance of getting this merged for 6.1?
Is anyone picking these up as fixes for 6.1-rc as we discussed?
Johan
Quoted 26 lines
Quoted 26 lines
Changes in v2
- use a custom devres action instead of amending the AUX bus interface (Doug)
- split sanity check fixes and cleanups per bridge type (Dmitry)
- add another Fixes tag for the missing bridge counter reset (Dmitry)Johan Hovold (10):
drm/msm: fix use-after-free on probe deferral drm/msm/dp: fix memory corruption with too many bridges drm/msm/dsi: fix memory corruption with too many bridges drm/msm/hdmi: fix memory corruption with too many bridges drm/msm/dp: fix IRQ lifetime
drm/msm/dp: fix aux-bus EP lifetime
drm/msm/dp: fix bridge lifetime
drm/msm/hdmi: fix IRQ lifetime
drm/msm/dp: drop modeset sanity checks drm/msm/dsi: drop modeset sanity checksdrivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++------- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- drivers/gpu/drm/msm/dsi/dsi.c | 9 +++++---- drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++++- drivers/gpu/drm/msm/msm_drv.c | 1 + 6 files changed, 37 insertions(+), 17 deletions(-)
Hi Johan
Quoted 11 lines · Johan Hovold
Quoted 10 lines · Johan Hovold
Quoted 9 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
The underlying problem is lifetime issues due to careless use of device-managed resources.
Any chance of getting this merged for 6.1?
Is anyone picking these up as fixes for 6.1-rc as we discussed?
Johan
All of these except the last two ( as discussed ) have landed in the -fixes tree
https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb
Thanks
Abhinav
Quoted 27 lines
Quoted 27 lines
Quoted 27 lines
Changes in v2
- use a custom devres action instead of amending the AUX bus interface(Doug)- split sanity check fixes and cleanups per bridge type (Dmitry)
- add another Fixes tag for the missing bridge counter reset (Dmitry)Johan Hovold (10):
drm/msm: fix use-after-free on probe deferral drm/msm/dp: fix memory corruption with too many bridges drm/msm/dsi: fix memory corruption with too many bridges drm/msm/hdmi: fix memory corruption with too many bridges drm/msm/dp: fix IRQ lifetime
drm/msm/dp: fix aux-bus EP lifetime
drm/msm/dp: fix bridge lifetime
drm/msm/hdmi: fix IRQ lifetime
drm/msm/dp: drop modeset sanity checks drm/msm/dsi: drop modeset sanity checksdrivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++------- drivers/gpu/drm/msm/dp/dp_parser.c | 6 +++--- drivers/gpu/drm/msm/dp/dp_parser.h | 5 +++-- drivers/gpu/drm/msm/dsi/dsi.c | 9 +++++---- drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++++- drivers/gpu/drm/msm/msm_drv.c | 1 + 6 files changed, 37 insertions(+), 17 deletions(-)
Quoted 19 lines · Abhinav Kumar
Hi Johan
Quoted 16 lines · Johan Hovold
Quoted 14 lines · Johan Hovold
Quoted 11 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
The underlying problem is lifetime issues due to careless use of device-managed resources.
Any chance of getting this merged for 6.1?
Quoted 8 lines
Quoted 3 lines
Is anyone picking these up as fixes for 6.1-rc as we discussed?
Johan
All of these except the last two ( as discussed ) have landed in the -fixes tree
https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb
Ah, perfect, thanks.
When do you expect to send these on so that they end up in linux-next and eventually Linus's tree?
Note that it looks like something happened with the commit messages when you applied these. Specifically, the Fixes tags appears to now have a line break in them and there's also some random whitespace before your SoB:
("drm/msm/dp: Add eDP support via aux_bus")
Patchwork: https://patchwork.freedesktop.org/patch/502667/
It's possible just the gitlab UI that's messed up, but perhaps you can double check before they hit linux-next, which should complain about this otherwise.
Johan
Quoted 28 lines · Johan Hovold
Quoted 26 lines · Abhinav Kumar
Hi Johan
Quoted 11 lines · Johan Hovold
Quoted 10 lines · Johan Hovold
Quoted 9 lines · Johan Hovold
The MSM DRM driver is currently broken in multiple ways with respect to probe deferral. Not only does the driver currently fail to probe again after a late deferral, but due to a related use-after-free bug this also triggers NULL-pointer dereferences.
These bugs are not new but have become critical with the release of 5.19 where probe is deferred in case the aux-bus EP panel driver has not yet been loaded.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
The underlying problem is lifetime issues due to careless use of device-managed resources.
Any chance of getting this merged for 6.1?
Is anyone picking these up as fixes for 6.1-rc as we discussed?
Johan
All of these except the last two ( as discussed ) have landed in the -fixes tree
Quoted 6 lines
Quoted 1 line
https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb
Ah, perfect, thanks.
When do you expect to send these on so that they end up in linux-next and eventually Linus's tree?
I'll send a -fixes PR this week
Quoted 8 lines
Note that it looks like something happened with the commit messages when you applied these. Specifically, the Fixes tags appears to now have a line break in them and there's also some random whitespace before your SoB:
Fixes c3bf8e21("drm/msm/dp: Add eDP support via
aux_bus")
naw, that is just some problem with gitlab's html generation, the actual patch is fine ;-)
BR,
-R
Quoted 16 lines
Cc [email protected] # 5.19Reviewed-by Dmitry Baryshkov <[email protected]>Signed-off-by Johan Hovold <[email protected]>Tested-by Kuogee Hsieh <[email protected]>Reviewed-by Kuogee Hsieh <[email protected]>Patchwork: https://patchwork.freedesktop.org/patch/502667/
Signed-off-by Abhinav Kumar's avatarAbhinav Kumar <[email protected]>It's possible just the gitlab UI that's messed up, but perhaps you can double check before they hit linux-next, which should complain about this otherwise.
Johan