| Message ID | 1386160133-24026-7-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote: > To avoid the need for a "nickname" property for each display, change > the display registration so that the display's alias (i.e. "display0" > etc) will be used for the dssdev->name if the display driver didn't > provide a name. > > This means that when booting with board files, we will have more > descriptive names for displays, like "lcd1", "hdmi". Where are those names used ? Are they reported to userspace, or limited to kernel internal use only ? > With DT we'll only have "display0", etc. But as there are no "nicknames" for > things like serials ports either, I hope we will do fine with this approach. Just a random thought, maybe the aliases node could help here. I'm not sure what rules govern its usage. Adding labels to display DT nodes could be an option too. > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/video/omap2/dss/display.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/video/omap2/dss/display.c > b/drivers/video/omap2/dss/display.c index 669a81fdf58e..a946cf7ed00f 100644 > --- a/drivers/video/omap2/dss/display.c > +++ b/drivers/video/omap2/dss/display.c > @@ -137,6 +137,9 @@ int omapdss_register_display(struct omap_dss_device > *dssdev) snprintf(dssdev->alias, sizeof(dssdev->alias), > "display%d", disp_num_counter++); > > + if (dssdev->name == NULL) > + dssdev->name = dssdev->alias; > + > if (drv && drv->get_resolution == NULL) > drv->get_resolution = omapdss_default_get_resolution; > if (drv && drv->get_recommended_bpp == NULL)
Hi Tomi, On Thursday 12 December 2013 00:13:01 Laurent Pinchart wrote: > On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote: > > To avoid the need for a "nickname" property for each display, change > > the display registration so that the display's alias (i.e. "display0" > > etc) will be used for the dssdev->name if the display driver didn't > > provide a name. > > > > This means that when booting with board files, we will have more > > descriptive names for displays, like "lcd1", "hdmi". > > Where are those names used ? Are they reported to userspace, or limited to > kernel internal use only ? > > > With DT we'll only have "display0", etc. But as there are no "nicknames" > > for things like serials ports either, I hope we will do fine with this > > Just a random thought, maybe the aliases node could help here. I should have read the next patches before replying, sorry :-) > I'm not sure what rules govern its usage. Adding labels to display DT nodes > could be an option too. A label property is still an option. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > > --- > > > > drivers/video/omap2/dss/display.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/video/omap2/dss/display.c > > b/drivers/video/omap2/dss/display.c index 669a81fdf58e..a946cf7ed00f > > 100644 > > --- a/drivers/video/omap2/dss/display.c > > +++ b/drivers/video/omap2/dss/display.c > > @@ -137,6 +137,9 @@ int omapdss_register_display(struct omap_dss_device > > *dssdev) > > snprintf(dssdev->alias, sizeof(dssdev->alias), > > "display%d", disp_num_counter++); > > > > + if (dssdev->name == NULL) > > + dssdev->name = dssdev->alias; > > + > > if (drv && drv->get_resolution == NULL) > > drv->get_resolution = omapdss_default_get_resolution; > > if (drv && drv->get_recommended_bpp == NULL)
On 2013-12-12 01:56, Laurent Pinchart wrote: > Hi Tomi, > > On Thursday 12 December 2013 00:13:01 Laurent Pinchart wrote: >> On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote: >>> To avoid the need for a "nickname" property for each display, change >>> the display registration so that the display's alias (i.e. "display0" >>> etc) will be used for the dssdev->name if the display driver didn't >>> provide a name. >>> >>> This means that when booting with board files, we will have more >>> descriptive names for displays, like "lcd1", "hdmi". >> >> Where are those names used ? Are they reported to userspace, or limited to >> kernel internal use only ? They are visible via omapdss's sysfs, when using omapfb. They are used for debug prints and by the user for selecting the default display and display modes via kernel cmdline, and when he sets the video pipeline routing. So changing them could be considered breaking the API, but... With omapdrm the sysfs files do not exist, and I think omapdrm doesn't use them (except maybe for some debug prints). >>> With DT we'll only have "display0", etc. But as there are no "nicknames" >>> for things like serials ports either, I hope we will do fine with this >> >> Just a random thought, maybe the aliases node could help here. > > I should have read the next patches before replying, sorry :-) > >> I'm not sure what rules govern its usage. Adding labels to display DT nodes >> could be an option too. Using of_alias_get_id() means that the alias is in the form "nameX" where X is a number. > A label property is still an option. Hmm, what do you mean? Label as in: foo : node { }; Isn't that 'foo' label only visible in DT itself, as a shortcut? Tomi
On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote: > > A label property is still an option. > > Hmm, what do you mean? Label as in: > > foo : node { > }; > > Isn't that 'foo' label only visible in DT itself, as a shortcut? Some driver use a "label" property like this: foo : node { label = "lcd"; ... }; See for example Documentation/devicetree/bindings/leds/common.txt Documentation/devicetree/bindings/mtd/partition.txt -- Sebastian
On Thursday 12 December 2013 11:05:28 Sebastian Reichel wrote: > On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote: > > > A label property is still an option. > > > > Hmm, what do you mean? Label as in: > > > > foo : node { > > }; > > > > Isn't that 'foo' label only visible in DT itself, as a shortcut? > > Some driver use a "label" property like this: > > foo : node { > label = "lcd"; > > ... > }; Yes, that's what I meant. > See for example > > Documentation/devicetree/bindings/leds/common.txt > Documentation/devicetree/bindings/mtd/partition.txt
On 2013-12-12 12:05, Sebastian Reichel wrote: > On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote: >>> A label property is still an option. >> >> Hmm, what do you mean? Label as in: >> >> foo : node { >> }; >> >> Isn't that 'foo' label only visible in DT itself, as a shortcut? > > Some driver use a "label" property like this: > > foo : node { > label = "lcd"; > > ... > }; > > See for example > > Documentation/devicetree/bindings/leds/common.txt > Documentation/devicetree/bindings/mtd/partition.txt Ah, I see. That kind of label was actually the first thing I did when starting to work on DSS DT. But I removed it, as it didn't describe the hardware and I didn't see others using anything similar. But I guess one could argue it does describe hardware, not in electrical level but in conceptual level. The question is, do we need labeling for displays? For backward compatibility omapdss would need it, but in general? I'm quite content with having just display0, display1 etc. Using the alias node, those can be fixed and display0 is always the same display. Tomi
Hi Tomi, On Thursday 12 December 2013 16:13:04 Tomi Valkeinen wrote: > On 2013-12-12 12:05, Sebastian Reichel wrote: > > On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote: > >>> A label property is still an option. > >> > >> Hmm, what do you mean? Label as in: > >> > >> foo : node { > >> }; > >> > >> Isn't that 'foo' label only visible in DT itself, as a shortcut? > > > > Some driver use a "label" property like this: > > > > foo : node { > > > > label = "lcd"; > > > > ... > > > > }; > > > > See for example > > > > Documentation/devicetree/bindings/leds/common.txt > > Documentation/devicetree/bindings/mtd/partition.txt > > Ah, I see. That kind of label was actually the first thing I did when > starting to work on DSS DT. But I removed it, as it didn't describe the > hardware and I didn't see others using anything similar. > > But I guess one could argue it does describe hardware, not in electrical > level but in conceptual level. > > The question is, do we need labeling for displays? For backward > compatibility omapdss would need it, but in general? I'm quite content > with having just display0, display1 etc. Using the alias node, those can > be fixed and display0 is always the same display. As you mentioned in your previous e-mail, if the labels are used by omapfb only, I won't strongly push to keep them. I wonder, however, when using DRM/KMS, where do the connector labels that are displayed by xrandr for instance come from ?
On 2013-12-12 16:15, Laurent Pinchart wrote: > As you mentioned in your previous e-mail, if the labels are used by omapfb > only, I won't strongly push to keep them. I wonder, however, when using > DRM/KMS, where do the connector labels that are displayed by xrandr for > instance come from ? drivers/gpu/drm/drm_crtc.c has lists, with names like "DVI-D", "HDMI-A", for connectors and encoders. Maybe from those. Tomi
On Thu, Dec 12, 2013 at 04:19:01PM +0200, Tomi Valkeinen wrote: > On 2013-12-12 16:15, Laurent Pinchart wrote: > > > As you mentioned in your previous e-mail, if the labels are used by omapfb > > only, I won't strongly push to keep them. I wonder, however, when using > > DRM/KMS, where do the connector labels that are displayed by xrandr for > > instance come from ? > > drivers/gpu/drm/drm_crtc.c has lists, with names like "DVI-D", "HDMI-A", > for connectors and encoders. Maybe from those. The xrandr names are generated from the list Tomi mentioned. => drm_connector_enum_list This requires a mapping from omapdss types to DRM types, which is done in drivers/gpu/drm/omapdrm/omap_drv.c:get_connector_type(). Currently only HDMI and DVI are handled properly. -- Sebastian
On 2013-12-12 16:13, Tomi Valkeinen wrote: > On 2013-12-12 12:05, Sebastian Reichel wrote: >> On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote: >>>> A label property is still an option. >>> >>> Hmm, what do you mean? Label as in: >>> >>> foo : node { >>> }; >>> >>> Isn't that 'foo' label only visible in DT itself, as a shortcut? >> >> Some driver use a "label" property like this: >> >> foo : node { >> label = "lcd"; >> >> ... >> }; >> >> See for example >> >> Documentation/devicetree/bindings/leds/common.txt >> Documentation/devicetree/bindings/mtd/partition.txt > > Ah, I see. That kind of label was actually the first thing I did when > starting to work on DSS DT. But I removed it, as it didn't describe the > hardware and I didn't see others using anything similar. > > But I guess one could argue it does describe hardware, not in electrical > level but in conceptual level. > > The question is, do we need labeling for displays? For backward > compatibility omapdss would need it, but in general? I'm quite content > with having just display0, display1 etc. Using the alias node, those can > be fixed and display0 is always the same display. I came to the conclusion that it's better to add the label to keep backward compatibility, especially as it was very easy to add. So we'll have 'name' and 'alias' for each display (as we have already). In the current non-DT boot (which is going away): - 'alias' is created by omapdss dynamically (first display to be registered is display0, etc.) - 'name' comes from platform data In the DT boot: - 'alias' comes from the DT aliases node, or if there are no display aliases, it's created the same way as to non-DT. The code presumes that there either is a DT alias for all displays, or there are none. - 'name' comes from 'label' property In both non-DT and DT cases, if 'name' is NULL (i.e. not set in platform data or no 'label' property), the alias is used as a name. I think this works fine, and was a trivial change. Tomi
diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c index 669a81fdf58e..a946cf7ed00f 100644 --- a/drivers/video/omap2/dss/display.c +++ b/drivers/video/omap2/dss/display.c @@ -137,6 +137,9 @@ int omapdss_register_display(struct omap_dss_device *dssdev) snprintf(dssdev->alias, sizeof(dssdev->alias), "display%d", disp_num_counter++); + if (dssdev->name == NULL) + dssdev->name = dssdev->alias; + if (drv && drv->get_resolution == NULL) drv->get_resolution = omapdss_default_get_resolution; if (drv && drv->get_recommended_bpp == NULL)
To avoid the need for a "nickname" property for each display, change the display registration so that the display's alias (i.e. "display0" etc) will be used for the dssdev->name if the display driver didn't provide a name. This means that when booting with board files, we will have more descriptive names for displays, like "lcd1", "hdmi". With DT we'll only have "display0", etc. But as there are no "nicknames" for things like serials ports either, I hope we will do fine with this approach. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/video/omap2/dss/display.c | 3 +++ 1 file changed, 3 insertions(+)