| Message ID | 1449675491-9184-2-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Wednesday 09 December 2015 17:38:05 Tomi Valkeinen wrote: > DRM offers no ways to add new displays after the DRM driver has been > loaded. This causes issues on boards that have multiple displays, and > omapdrm is loaded when some of them are loaded but not all. The result > is that omapdrm starts with the displays that were loaded at that time, > ignoring the displays loaded later. > > This patch changes the behavior so that omapdrm ensures all display are > loaded which have an alias in the .dts file. > > The downside is that this prevents omapdrm from loading if, say, the > user has not compiled a kernel module for one of the displays, or > there's some kind of failure in one of the displays. Thus, after this > patch, all the displays have to work, or none does. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index 5c6609cbb6a2..c93c13e2c22d > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -242,11 +242,38 @@ static void omap_disconnect_dssdevs(void) > dssdev->driver->disconnect(dssdev); > } > > +static bool dssdev_with_alias_exists(const char *alias) > +{ > + struct omap_dss_device *dssdev = NULL; > + > + for_each_dss_dev(dssdev) { > + if (strcmp(alias, dssdev->alias) == 0) { > + omap_dss_put_device(dssdev); > + return true; > + } > + } > + > + return false; > +} > + > static int omap_connect_dssdevs(void) > { > int r; > struct omap_dss_device *dssdev = NULL; > bool no_displays = true; > + struct device_node *aliases; > + struct property *pp; > + > + aliases = of_find_node_by_path("/aliases"); > + if (aliases) { > + for_each_property_of_node(aliases, pp) { > + if (strncmp(pp->name, "display", 7) != 0) > + continue; > + > + if (dssdev_with_alias_exists(pp->name) == false) > + return -EPROBE_DEFER; > + } > + } The DSS bindings document the aliases as optional, so this won't work in the general case. The best solution would be to travel the display graph through the ports and use the components framework, but that would be more complex to implement. > > for_each_dss_dev(dssdev) { > r = dssdev->driver->connect(dssdev);
On 13/12/15 22:30, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Wednesday 09 December 2015 17:38:05 Tomi Valkeinen wrote: >> DRM offers no ways to add new displays after the DRM driver has been >> loaded. This causes issues on boards that have multiple displays, and >> omapdrm is loaded when some of them are loaded but not all. The result >> is that omapdrm starts with the displays that were loaded at that time, >> ignoring the displays loaded later. >> >> This patch changes the behavior so that omapdrm ensures all display are >> loaded which have an alias in the .dts file. >> >> The downside is that this prevents omapdrm from loading if, say, the >> user has not compiled a kernel module for one of the displays, or >> there's some kind of failure in one of the displays. Thus, after this >> patch, all the displays have to work, or none does. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5c6609cbb6a2..c93c13e2c22d >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -242,11 +242,38 @@ static void omap_disconnect_dssdevs(void) >> dssdev->driver->disconnect(dssdev); >> } >> >> +static bool dssdev_with_alias_exists(const char *alias) >> +{ >> + struct omap_dss_device *dssdev = NULL; >> + >> + for_each_dss_dev(dssdev) { >> + if (strcmp(alias, dssdev->alias) == 0) { >> + omap_dss_put_device(dssdev); >> + return true; >> + } >> + } >> + >> + return false; >> +} >> + >> static int omap_connect_dssdevs(void) >> { >> int r; >> struct omap_dss_device *dssdev = NULL; >> bool no_displays = true; >> + struct device_node *aliases; >> + struct property *pp; >> + >> + aliases = of_find_node_by_path("/aliases"); >> + if (aliases) { >> + for_each_property_of_node(aliases, pp) { >> + if (strncmp(pp->name, "display", 7) != 0) >> + continue; >> + >> + if (dssdev_with_alias_exists(pp->name) == false) >> + return -EPROBE_DEFER; >> + } >> + } > > The DSS bindings document the aliases as optional, so this won't work in the > general case. The best solution would be to travel the display graph through > the ports and use the components framework, but that would be more complex to > implement. Well, even with this patch aliases are optional. The difference is that if you don't have any aliases, and omapdrm starts, and one of the multiple displays has not been probed yet, that display will be unavailable. If you do have aliases, omapdrm ensures that the displays with an alias are probed. But... You are probably right, travelling the graph is better. Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 5c6609cbb6a2..c93c13e2c22d 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -242,11 +242,38 @@ static void omap_disconnect_dssdevs(void) dssdev->driver->disconnect(dssdev); } +static bool dssdev_with_alias_exists(const char *alias) +{ + struct omap_dss_device *dssdev = NULL; + + for_each_dss_dev(dssdev) { + if (strcmp(alias, dssdev->alias) == 0) { + omap_dss_put_device(dssdev); + return true; + } + } + + return false; +} + static int omap_connect_dssdevs(void) { int r; struct omap_dss_device *dssdev = NULL; bool no_displays = true; + struct device_node *aliases; + struct property *pp; + + aliases = of_find_node_by_path("/aliases"); + if (aliases) { + for_each_property_of_node(aliases, pp) { + if (strncmp(pp->name, "display", 7) != 0) + continue; + + if (dssdev_with_alias_exists(pp->name) == false) + return -EPROBE_DEFER; + } + } for_each_dss_dev(dssdev) { r = dssdev->driver->connect(dssdev);
DRM offers no ways to add new displays after the DRM driver has been loaded. This causes issues on boards that have multiple displays, and omapdrm is loaded when some of them are loaded but not all. The result is that omapdrm starts with the displays that were loaded at that time, ignoring the displays loaded later. This patch changes the behavior so that omapdrm ensures all display are loaded which have an alias in the .dts file. The downside is that this prevents omapdrm from loading if, say, the user has not compiled a kernel module for one of the displays, or there's some kind of failure in one of the displays. Thus, after this patch, all the displays have to work, or none does. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/omap_drv.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)