drm/panel: Add device link in drm_panel_attach()
Subsystems (4)
v3 cover drm/panel: Add device link in drm_panel_attach()
I dropped this work at the end of the previous merge window, since I had more urgent thing to do. It looks like topic has been raised again[1], so I'll respin this now.
The second version of these patches can be found here:
https://lists.freedesktop.org/archives/dri-devel/2018-February/167628.html
Changes since v2:
- Rebased both patches on top of the latest drm-next
- "drm/panel: Remove drm_panel_detach() calls from all panel drives"
- Updated the kernel doc for drm_panel_detach()
- "drm/panel: Add device_link from panel device to drm device"
- Fixed the logically wrong second paragraph in the commit message according to Eric Anholt's suggestion and added his reviewed by
The first patch could be squashed to second, but kept is separate since I think it is correct even without the second patch.
With these patches unbinding a panel driver in use does not cause nasty backtraces and corrupted drm core structures, but instead it cleanly unbinds the drm master device at the same time.
The only down side (currently[1]) is that the drm device does not reprobe if the panel driver is bound again, but everything should work if the drm master driver is bound manually.
Best regards,
Jyri
[1] https://lists.freedesktop.org/archives/dri-devel/2018-April/174083.html
[2] https://lists.freedesktop.org/archives/dri-devel/2018-February/166907.html
Jyri Sarha (2):
drm/panel: Remove drm_panel_detach() calls from all panel drives drm/panel: Add device_link from panel device to drm device
drivers/gpu/drm/drm_panel.c | 14 ++++++++++++++ drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 - drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 1 - drivers/gpu/drm/panel/panel-lvds.c | 1 - drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 1 - drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 - drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 - drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 - drivers/gpu/drm/panel/panel-simple.c | 1 - drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 1 - include/drm/drm_panel.h | 1 + 11 files changed, 15 insertions(+), 9 deletions(-)
Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v3 1/2 drm/panel: Remove drm_panel_detach() calls from all panel drives
Remove all drm_panel_detach() calls from all panel drives update the kernel doc for drm_panel_detach().
Setting the connector and drm to NULL when the drm panel device is going away hardly serves any purpose. Usually the the whole memory structure is freed right after the remove call. However, calling the detach function from the master drm device, and setting the connector pointer to NULL, has the logic of marking the panel again as available for another drm master to attach. The usual situation would be the same drm master device binding again.
--- drivers/gpu/drm/drm_panel.c | 4 ++++ drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 - drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 1 - drivers/gpu/drm/panel/panel-lvds.c | 1 - drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 1 - drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 - drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 - drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 - drivers/gpu/drm/panel/panel-simple.c | 1 - drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 1 - 10 files changed, 4 insertions(+), 9 deletions(-)
drivers/gpu/drm/drm_panel.c +4 -0
@@ -115,6 +115,10 @@ EXPORT_SYMBOL(drm_panel_attach);* Detaches a panel from the connector it is attached to. If a panel is not * attached to any connector this is effectively a no-op. * +* NOTE: This function should not be called by the panel device +* itself. It is only for the drm device that called the +* drm_panel_attach(). +* * Return: 0 on success or a negative error code on failure. */ int drm_panel_detach (struct drm_panel *panel)
drivers/gpu/drm/panel/panel-innolux-p079zca.c +0 -1
@@ -292,7 +292,6 @@ static int innolux_panel_remove(struct mipi_dsi_device *dsi)DRM_DEV_ERROR (&dsi->dev,"failed to detach from DSI host: %d\n" , err); -drm_panel_detach (&innolux->base);innolux_panel_del (innolux);return 0 ;
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c +0 -1
@@ -500,7 +500,6 @@ static int jdi_panel_remove(struct mipi_dsi_device *dsi)dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&jdi->base);jdi_panel_del (jdi);return 0 ;
drivers/gpu/drm/panel/panel-lvds.c +0 -1
@@ -282,7 +282,6 @@ static int panel_lvds_remove(struct platform_device *pdev) {struct panel_lvds *lvds =dev_get_drvdata (&pdev->dev); -drm_panel_detach (&lvds->panel);drm_panel_remove (&lvds->panel);panel_lvds_disable (&lvds->panel);
drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c +0 -1
@@ -299,7 +299,6 @@ static int wuxga_nt_panel_remove(struct mipi_dsi_device *dsi)if (ret <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&wuxga_nt->base);wuxga_nt_panel_del (wuxga_nt);return 0 ;
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c +0 -1
@@ -292,7 +292,6 @@ static int seiko_panel_remove(struct platform_device *pdev) {struct seiko_panel *panel =dev_get_drvdata (&pdev->dev); -drm_panel_detach (&panel->base);drm_panel_remove (&panel->base);seiko_panel_disable (&panel->base);
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c +0 -1
@@ -418,7 +418,6 @@ static int sharp_panel_remove(struct mipi_dsi_device *dsi)if (err <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , err); -drm_panel_detach (&sharp->base);sharp_panel_del (sharp);return 0 ;
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c +0 -1
@@ -327,7 +327,6 @@ static int sharp_nt_panel_remove(struct mipi_dsi_device *dsi)if (ret <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&sharp_nt->base);sharp_nt_panel_del (sharp_nt);return 0 ;
drivers/gpu/drm/panel/panel-simple.c +0 -1
@@ -364,7 +364,6 @@ static int panel_simple_remove(struct device *dev) {struct panel_simple *panel =dev_get_drvdata (dev); -drm_panel_detach (&panel->base);drm_panel_remove (&panel->base);panel_simple_disable (&panel->base);
drivers/gpu/drm/panel/panel-sitronix-st7789v.c +0 -1
@@ -419,7 +419,6 @@ static int st7789v_remove(struct spi_device *spi) {struct st7789v *ctx =spi_get_drvdata (spi); -drm_panel_detach (&ctx->panel);drm_panel_remove (&ctx->panel);if (ctx->backlight)
Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/2] drm/panel: Remove drm_panel_detach() calls from all panel drives
Quoted 11 lines · Jyri Sarha
Remove all
drm_panel_detach()calls from all panel drives update the kernel doc fordrm_panel_detach().Setting the connector and drm to NULL when the drm panel device is going away hardly serves any purpose. Usually the the whole memory structure is freed right after the remove call. However, calling the detach function from the master drm device, and setting the connector pointer to NULL, has the logic of marking the panel again as available for another drm master to attach. The usual situation would be the same drm master device binding again.
Quoted 1 line
Signed-off-by Jyri Sarha <[email protected]>
Yeah this makes tons of sense, no panel driver calls attach either.
Quoted 18 lines
--- drivers/gpu/drm/drm_panel.c | 4 ++++ drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 - drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 1 - drivers/gpu/drm/panel/panel-lvds.c | 1 - drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 1 - drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 - drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 - drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 - drivers/gpu/drm/panel/panel-simple.c | 1 - drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 1 - 10 files changed, 4 insertions(+), 9 deletions(-)drivers/gpu/drm/drm_panel.c +0
-0
drivers/gpu/drm/drm_panel.c EXPORT_SYMBOL(drm_panel_attach); · line 120
Quoted 6 lines
drivers/gpu/drm/drm_panel.c +3
-0@@ -115,6 +115,10 @@ EXPORT_SYMBOL(drm_panel_attach);* Detaches a panel from the connector it is attached to. If a panel is not * attached to any connector this is effectively a no-op. * +* NOTE: This function should not be called by the panel device +* itself. It is only for the drm device that called the +* drm_panel_attach().
Does this result in pretty output when building the docs with
$ make htmldocs
Also might be good to add something like
* When unloading, the driver should detach from the panel by calling
* drm_panel_detach().
to the kerneldoc of drm_panel_attach.
Either way:
Quoted 115 lines
drivers/gpu/drm/drm_panel.c +1
-0+* * Return: 0 on success or a negative error code on failure. */ int drm_panel_detach (struct drm_panel *panel)drivers/gpu/drm/panel/panel-innolux-p079zca.c +0
-1@@ -292,7 +292,6 @@ static int innolux_panel_remove(struct mipi_dsi_device *dsi)DRM_DEV_ERROR (&dsi->dev,"failed to detach from DSI host: %d\n" , err); -drm_panel_detach (&innolux->base);innolux_panel_del (innolux);return 0 ;drivers/gpu/drm/panel/panel-jdi-lt070me05000.c +0
-1@@ -500,7 +500,6 @@ static int jdi_panel_remove(struct mipi_dsi_device *dsi)dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&jdi->base);jdi_panel_del (jdi);return 0 ;drivers/gpu/drm/panel/panel-lvds.c +0
-1@@ -282,7 +282,6 @@ static int panel_lvds_remove(struct platform_device *pdev) {struct panel_lvds *lvds =dev_get_drvdata (&pdev->dev); -drm_panel_detach (&lvds->panel);drm_panel_remove (&lvds->panel);panel_lvds_disable (&lvds->panel);drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c +0
-1@@ -299,7 +299,6 @@ static int wuxga_nt_panel_remove(struct mipi_dsi_device *dsi)if (ret <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&wuxga_nt->base);wuxga_nt_panel_del (wuxga_nt);return 0 ;drivers/gpu/drm/panel/panel-seiko-43wvf1g.c +0
-1@@ -292,7 +292,6 @@ static int seiko_panel_remove(struct platform_device *pdev) {struct seiko_panel *panel =dev_get_drvdata (&pdev->dev); -drm_panel_detach (&panel->base);drm_panel_remove (&panel->base);seiko_panel_disable (&panel->base);drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c +0
-1@@ -418,7 +418,6 @@ static int sharp_panel_remove(struct mipi_dsi_device *dsi)if (err <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , err); -drm_panel_detach (&sharp->base);sharp_panel_del (sharp);return 0 ;drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c +0
-1@@ -327,7 +327,6 @@ static int sharp_nt_panel_remove(struct mipi_dsi_device *dsi)if (ret <0 )dev_err (&dsi->dev,"failed to detach from DSI host: %d\n" , ret); -drm_panel_detach (&sharp_nt->base);sharp_nt_panel_del (sharp_nt);return 0 ;drivers/gpu/drm/panel/panel-simple.c +0
-1@@ -364,7 +364,6 @@ static int panel_simple_remove(struct device *dev) {struct panel_simple *panel =dev_get_drvdata (dev); -drm_panel_detach (&panel->base);drm_panel_remove (&panel->base);panel_simple_disable (&panel->base);drivers/gpu/drm/panel/panel-sitronix-st7789v.c +0
-1@@ -419,7 +419,6 @@ static int st7789v_remove(struct spi_device *spi) {struct st7789v *ctx =spi_get_drvdata (spi); -drm_panel_detach (&ctx->panel);drm_panel_remove (&ctx->panel);if (ctx->backlight)Signature · 1 line
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v3 2/2 drm/panel: Add device_link from panel device to drm device
In reply to Jyri SarhaAdd device_link from panel device (supplier) to drm device (consumer) when drm_panel_attach() is called. This patch should protect the master drm driver if an attached panel driver unbinds while it is in use. The device_link should make sure the drm device is unbound before the panel driver becomes unavailable.
The device_link is removed when drm_panel_detach() is called. The drm_panel_detach() should be called by the consumer DRM driver, not the panel driver, otherwise both drivers are racing to delete the same link.
--- drivers/gpu/drm/drm_panel.c | 10 ++++++++++ include/drm/drm_panel.h | 1 + 2 files changed, 11 insertions(+)
drivers/gpu/drm/drm_panel.c +10 -0
@@ -24,6 +24,7 @@#include <linux/err.h> #include <linux/module.h> +#include <drm/drm_device.h> #include <drm/drm_crtc.h> #include <drm/drm_panel.h>
@@ -101,6 +102,13 @@ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector)if (panel->connector)return -EBUSY ; + panel->link =device_link_add (connector->dev->dev, panel->dev,0 ); +if (!panel->link) { +dev_err (panel->dev,"failed to link panel to %s\n" , +dev_name (connector->dev->dev)); +return -EINVAL ; + } + panel->connector = connector; panel->drm = connector->dev;
@@ -123,6 +131,8 @@ EXPORT_SYMBOL(drm_panel_attach);*/ int drm_panel_detach (struct drm_panel *panel) { +device_link_del (panel->link); + panel->connector =NULL ; panel->drm =NULL ;
include/drm/drm_panel.h +1 -0
@@ -89,6 +89,7 @@ struct drm_panel {struct drm_device *drm;struct drm_connector *connector;struct device *dev; +struct device_link *link;const struct drm_panel_funcs *funcs;
Signature · 6 lines
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel