| Message ID | 1507721021-28174-4-git-send-email-LW@KARO-electronics.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | drm/panel: simple: improve definition of display modes and add more panels | expand |
On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote: > The baseboards for the Ka-Ro electronics series of i.MX modules > use a 24bit LCD interface, no matter what LCD bus width the SoC on the > module provides and what the LCD panel expects. LCDs with 6bit per color > will ignore the 2 LSBs of each color lane, and modules using a SoC > that provides only 6bit per color, drive the display information on the > 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. > > Thus, no matter what combination of LCD and SoC is used, the LCD port > can be used without shuffling bit lanes by always configuring the LCD > output to 24bit mode. Thanks for providing good reasoning as to why this is needed. > > Add a function to handle certain quirks of the LCD interface to the > panel driver to be able to override the bus format specified in a > panel's display_mode. > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> > --- > .../bindings/display/panel/simple-panel.txt | 2 ++ > drivers/gpu/drm/panel/panel-simple.c | 40 +++++++++++++++++++++- > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt > index 1341bbf..e2308c3 100644 > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt > @@ -7,6 +7,8 @@ Optional properties: > - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing > - enable-gpios: GPIO pin to enable or disable the panel > - backlight: phandle of the backlight device attached to the panel > +- bus-format-override: override the bus_format setting of the panel's > + display_mode settings You need to define valid values here. However, we also have this proposal[1]. Please align to a common one. BTW, we don't need another common panel binding file that [1] added. We already have panel-dpi.txt for parallel interface panels. And personally, I'd like to see simple-panel.txt disappear. Rob [1] https://patchwork.ozlabs.org/patch/823104/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote: > The baseboards for the Ka-Ro electronics series of i.MX modules > use a 24bit LCD interface, no matter what LCD bus width the SoC on the > module provides and what the LCD panel expects. LCDs with 6bit per color > will ignore the 2 LSBs of each color lane, and modules using a SoC > that provides only 6bit per color, drive the display information on the > 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. > > Thus, no matter what combination of LCD and SoC is used, the LCD port > can be used without shuffling bit lanes by always configuring the LCD > output to 24bit mode. > > Add a function to handle certain quirks of the LCD interface to the > panel driver to be able to override the bus format specified in a > panel's display_mode. I think the above paragraph clearly indicates that this is the wrong place to workaround this. You say yourself that the LCD interface has quirks that need to be handled, so why do you want to force this handling into the panel driver? The panel remains the same, no matter what interface you connect it to. Thierry
Hi, On Tue, 17 Oct 2017 14:12:40 +0200 Thierry Reding wrote: > On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote: > > The baseboards for the Ka-Ro electronics series of i.MX modules > > use a 24bit LCD interface, no matter what LCD bus width the SoC on the > > module provides and what the LCD panel expects. LCDs with 6bit per color > > will ignore the 2 LSBs of each color lane, and modules using a SoC > > that provides only 6bit per color, drive the display information on the > > 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. > > > > Thus, no matter what combination of LCD and SoC is used, the LCD port > > can be used without shuffling bit lanes by always configuring the LCD > > output to 24bit mode. > > > > Add a function to handle certain quirks of the LCD interface to the > > panel driver to be able to override the bus format specified in a > > panel's display_mode. > > I think the above paragraph clearly indicates that this is the wrong > place to workaround this. You say yourself that the LCD interface has > quirks that need to be handled, so why do you want to force this > handling into the panel driver? > The quirk is in the interfacing of the SoM's LCD output to the LCD panel. Thus it can be handled in either place. > The panel remains the same, no matter what interface you connect it to. > Because that's just ONE place to change, no matter what LCD driver is being used. Lothar Waßmann -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 17, 2017 at 02:44:23PM +0200, Lothar Waßmann wrote: > Hi, > > On Tue, 17 Oct 2017 14:12:40 +0200 Thierry Reding wrote: > > On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote: > > > The baseboards for the Ka-Ro electronics series of i.MX modules > > > use a 24bit LCD interface, no matter what LCD bus width the SoC on the > > > module provides and what the LCD panel expects. LCDs with 6bit per color > > > will ignore the 2 LSBs of each color lane, and modules using a SoC > > > that provides only 6bit per color, drive the display information on the > > > 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. > > > > > > Thus, no matter what combination of LCD and SoC is used, the LCD port > > > can be used without shuffling bit lanes by always configuring the LCD > > > output to 24bit mode. > > > > > > Add a function to handle certain quirks of the LCD interface to the > > > panel driver to be able to override the bus format specified in a > > > panel's display_mode. > > > > I think the above paragraph clearly indicates that this is the wrong > > place to workaround this. You say yourself that the LCD interface has > > quirks that need to be handled, so why do you want to force this > > handling into the panel driver? > > > The quirk is in the interfacing of the SoM's LCD output to the LCD > panel. Thus it can be handled in either place. Yes. What I'm saying is that the panel is, in my opinion, the wrong place to handle an LCD interface (originating from the SoM) quirk. > > The panel remains the same, no matter what interface you connect it to. > > > Because that's just ONE place to change, no matter what LCD driver is > being used. That's a Linux specific implementation detail. If you ever want to use a panel that is not driven by simple-panel you'd have to change it in that driver, too. Thierry
On Tue, Oct 17, 2017 at 7:56 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Tue, Oct 17, 2017 at 02:44:23PM +0200, Lothar Waßmann wrote: >> Hi, >> >> On Tue, 17 Oct 2017 14:12:40 +0200 Thierry Reding wrote: >> > On Wed, Oct 11, 2017 at 01:23:35PM +0200, Lothar Waßmann wrote: >> > > The baseboards for the Ka-Ro electronics series of i.MX modules >> > > use a 24bit LCD interface, no matter what LCD bus width the SoC on the >> > > module provides and what the LCD panel expects. LCDs with 6bit per color >> > > will ignore the 2 LSBs of each color lane, and modules using a SoC >> > > that provides only 6bit per color, drive the display information on the >> > > 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. >> > > >> > > Thus, no matter what combination of LCD and SoC is used, the LCD port >> > > can be used without shuffling bit lanes by always configuring the LCD >> > > output to 24bit mode. >> > > >> > > Add a function to handle certain quirks of the LCD interface to the >> > > panel driver to be able to override the bus format specified in a >> > > panel's display_mode. >> > >> > I think the above paragraph clearly indicates that this is the wrong >> > place to workaround this. You say yourself that the LCD interface has >> > quirks that need to be handled, so why do you want to force this >> > handling into the panel driver? >> > >> The quirk is in the interfacing of the SoM's LCD output to the LCD >> panel. Thus it can be handled in either place. > > Yes. What I'm saying is that the panel is, in my opinion, the wrong > place to handle an LCD interface (originating from the SoM) quirk. There's 2 possible things we could need to describe here. First, is any restrictions the SoM has. For example, the SoM has an 18-bit interface while the SoC has a 24-bit interface. Second, is how a panel with fewer bits than the interface (whether a SoM or direct to SoC) is wired up. If we think about this in terms of a base DT plus overlays, then the former case belongs in the base and the latter belongs in the overlay. If the SoM interface needs to be SoC independent, then it gets more complicated as we don't want to have the SoC's LCD controller node in the overlay adding properties to it and therefore need to have some sort of translation (i.e. a connector binding). But we have no definition of how we would describe OF graph thru a connector. In the end, these are board level properties, so you can make the same argument that this property doesn't belong in the SoC's LCD controller node as you can it doesn't belong in the panel node. Generally, we just default to the node bound to the driver we want to handle the property. In any case, I think this should be properties of the endpoints rather than the endpoint parent nodes as it is a property of the interface, not the controller nor the panel. This is also how camera interfaces were done. An LCD controller node could have 2 interfaces for example. >> > The panel remains the same, no matter what interface you connect it to. >> > >> Because that's just ONE place to change, no matter what LCD driver is >> being used. > > That's a Linux specific implementation detail. If you ever want to use > a panel that is not driven by simple-panel you'd have to change it in > that driver, too. I do agree the implementation belongs in the LCD controller side, but just because the property is in the panel node that doesn't mean it's the panel driver that has to handle the property. As long as the property is parse-able in a standard way, the "parent" can do it. There are cases where the driver tied to the parent node handles properties in the child nodes. SPI timing modes is one example. In this case, I think we'll need to handle this property being in the local endpoint and the remote endpoint and resolving the format needed based on that (as well as what the panel driver says). That's going to be needed with overlays and connectors anyways and then you don't really have to care where the restriction comes from. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt index 1341bbf..e2308c3 100644 --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt @@ -7,6 +7,8 @@ Optional properties: - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing - enable-gpios: GPIO pin to enable or disable the panel - backlight: phandle of the backlight device attached to the panel +- bus-format-override: override the bus_format setting of the panel's + display_mode settings Example: diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index fde9c41..f356a7b 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -87,6 +87,8 @@ struct panel_simple { struct i2c_adapter *ddc; struct gpio_desc *enable_gpio; + + u32 bus_fmt_override; }; #define SP_DISPLAY_MODE(freq, ha, hfp, hs, hbp, va, vfp, vs, vbp, vr, flgs) { \ @@ -165,7 +167,11 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel) connector->display_info.bpc = panel->desc->bpc; connector->display_info.width_mm = panel->desc->size.width; connector->display_info.height_mm = panel->desc->size.height; - if (panel->desc->bus_format) + + if (panel->bus_fmt_override) + drm_display_info_set_bus_formats(&connector->display_info, + &panel->bus_fmt_override, 1); + else if (panel->desc->bus_format) drm_display_info_set_bus_formats(&connector->display_info, &panel->desc->bus_format, 1); connector->display_info.bus_flags = panel->desc->bus_flags; @@ -298,6 +304,34 @@ static int panel_simple_get_timings(struct drm_panel *panel, return p->desc->num_timings; } +static inline int panel_simple_check_quirks(struct device *dev, + struct panel_simple *p) +{ + const char *bus_fmt; + + if (of_property_read_string(dev->of_node, "bus-format-override", + &bus_fmt) == 0) { + if (strcmp(bus_fmt, "rgb24") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB888_1X24; + else if (strcmp(bus_fmt, "rgb666") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB666_1X18; + else if (strcmp(bus_fmt, "rgb565") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB565_1X16; + else if (strcmp(bus_fmt, "spwg-18") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG; + else if (strcmp(bus_fmt, "spwg-24") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG; + else if (strcmp(bus_fmt, "jeida-24") == 0) + p->bus_fmt_override = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; + else + dev_err(dev, + "Unsupported bus-format-override value: '%s'\n", + bus_fmt); + return p->bus_fmt_override ? 0 : -EINVAL; + } + return 0; +} + static const struct drm_panel_funcs panel_simple_funcs = { .disable = panel_simple_disable, .unprepare = panel_simple_unprepare, @@ -353,6 +387,10 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) } } + err = panel_simple_check_quirks(dev, panel); + if (err) + goto free_ddc; + drm_panel_init(&panel->base); panel->base.dev = dev; panel->base.funcs = &panel_simple_funcs;
The baseboards for the Ka-Ro electronics series of i.MX modules use a 24bit LCD interface, no matter what LCD bus width the SoC on the module provides and what the LCD panel expects. LCDs with 6bit per color will ignore the 2 LSBs of each color lane, and modules using a SoC that provides only 6bit per color, drive the display information on the 6 MSBs of each color lane and tie the 2 LSBs of each color lane to GND. Thus, no matter what combination of LCD and SoC is used, the LCD port can be used without shuffling bit lanes by always configuring the LCD output to 24bit mode. Add a function to handle certain quirks of the LCD interface to the panel driver to be able to override the bus format specified in a panel's display_mode. Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> --- .../bindings/display/panel/simple-panel.txt | 2 ++ drivers/gpu/drm/panel/panel-simple.c | 40 +++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-)