| Message ID | d6bb7a8900d9a701ae13a3d0fa2c34b01089038f.1519815150.git.jsarha@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/panel: Add device link in drm_panel_attach() | expand |
Jyri Sarha <jsarha@ti.com> writes: > Add device_link from panel device (supplier) to drm device (consumer) > with DL_FLAG_AUTOREMOVE when drm_panel_attach() is called. Currently > the master drm driver is not protected against the attached. The > device_link with DL_FLAG_AUTOREMOVE 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 panel driver it self when > it is removed. Otherwise the both driver are racing to delete the same > link. I think this paragraph wants to be: 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. Other than that, these patches are: Reviewed-by: Eric Anholt <eric@anholt.net> (though you'll probably want to wait a bit for Thierry to look at them too)
On Wed, Feb 28, 2018 at 01:09:30PM +0200, Jyri Sarha wrote: > Add device_link from panel device (supplier) to drm device (consumer) > with DL_FLAG_AUTOREMOVE when drm_panel_attach() is called. Currently > the master drm driver is not protected against the attached. The > device_link with DL_FLAG_AUTOREMOVE 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 panel driver it self when > it is removed. Otherwise the both driver are racing to delete the same > link. AFAICS drm_panel_detach() is always called by the DRM drivers, so it's confusing that you're saying here it should be called by the panel drivers. That would also seem to be asymmetric since drm_panel_attach() is called by the DRM drivers. > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -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; If you ditch the device_link_del() and use DL_FLAG_AUTOREMOVE only, you don't need to save the pointer to struct device_link. Thanks, Lukas
On 28/02/18 20:14, Eric Anholt wrote: > Jyri Sarha <jsarha@ti.com> writes: > >> Add device_link from panel device (supplier) to drm device (consumer) >> with DL_FLAG_AUTOREMOVE when drm_panel_attach() is called. Currently >> the master drm driver is not protected against the attached. The >> device_link with DL_FLAG_AUTOREMOVE 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 panel driver it self when >> it is removed. Otherwise the both driver are racing to delete the same >> link. > > I think this paragraph wants to be: > > 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. > > Other than that, these patches are: > > Reviewed-by: Eric Anholt <eric@anholt.net> > Thanks second paragraph was indeed completely wrong. And the first - especially the second sentence - does not make too much sense either, but it anyway need rephrasing if I drop DL_FLAG_AUTOREMOVE. > (though you'll probably want to wait a bit for Thierry to look at them > too) >
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 308d442..afa8337 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -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> @@ -98,9 +99,18 @@ EXPORT_SYMBOL(drm_panel_remove); */ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector) { + u32 flags = DL_FLAG_AUTOREMOVE; + if (panel->connector) return -EBUSY; + panel->link = device_link_add(connector->dev->dev, panel->dev, flags); + 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; @@ -119,6 +129,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; diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 14ac240..26a1b5f 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -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;
Add device_link from panel device (supplier) to drm device (consumer) with DL_FLAG_AUTOREMOVE when drm_panel_attach() is called. Currently the master drm driver is not protected against the attached. The device_link with DL_FLAG_AUTOREMOVE 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 panel driver it self when it is removed. Otherwise the both driver are racing to delete the same link. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/drm_panel.c | 12 ++++++++++++ include/drm/drm_panel.h | 1 + 2 files changed, 13 insertions(+)