| Message ID | 20140508233300.GI2198@atomide.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 09/05/14 02:33, Tony Lindgren wrote: > We can pass the GPIO configuration for ls037v7dw01 in a standard > gpios property. > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > --- /dev/null > +++ b/Documentation/devicetree/bindings/panel/sharp,ls037v7dw01.txt > @@ -0,0 +1,56 @@ > +SHARP LS037V7DW01 TFT-LCD panel > + > +Required properties: > +- compatible: should be "sharp,ls037v7dw01" > + > +Optional properties: > +- enable-gpios: a GPIO spec for the optional enable pin > + this pin is the INI pin as specified in the LS037V7DW01.pdf file. > +- reset-gpios: a GPIO spec for the optional reset pin > + this pin is the RESB pin as specified in the LS037V7DW01.pdf file. > +- mode-gpios: a GPIO > + ordered MO, LR, and UD as specified in the LS037V7DW01.pdf file. > + > +This binding is compatible with the simple-panel binding, which is specified > +in simple-panel.txt in this directory. The video port data is required also. See for example Documentation/devicetree/bindings/video/panel-dsi-cm.txt or Documentation/devicetree/bindings/video/hdmi-connector.txt. Also, all the bindings I've made are in Documentation/devicetree/bindings/video/, and I'd rather keep the video bindings OMAP uses in the same place. > +This panel can have zero to five GPIOs to configure > +to change configuration between QVGA and VGA mode > +and the scan direction. As these pins can be also > +configured with external pulls, all the GPIOs are > +considered optional with holes in the array. > + > +Example when connected to a omap2+ based device: > + > + lcd0: display { > + compatible = "sharp,ls037v7dw01"; > + power-supply = <&lcd_3v3>; > + enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>; /* gpio152, lcd INI */ > + reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>; /* gpio155, lcd RESB */ > + mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH /* gpio154, lcd MO */ > + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */ > + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */ > + > + panel-timing { > + clock-frequency = <19200000>; > + hback-porch = <28>; > + hactive = <480>; > + hfront-porch = <1>; > + hsync-len = <2>; > + vback-porch = <1>; > + vactive = <640>; > + vfront-porch = <1>; > + vsync-len = <1>; > + hsync-active = <0>; > + vsync-active = <0>; > + de-active = <1>; > + pixelclk-active = <1>; > + }; I don't think we should define panel-timing here. We know it's sharp,ls037v7dw01, so the driver knows the video timings. Also, if we would extend the driver to support both resolution modes, it needs to support two different timings, so the above doesn't work in that case either. Although if the MO gpio is not controlled by the driver, we should tell the driver whether that gpio is high or low. > +static int sharp_ls_probe_of(struct platform_device *pdev) > +{ > + struct panel_drv_data *ddata = platform_get_drvdata(pdev); > + struct device_node *node = pdev->dev.of_node; > + struct omap_dss_device *in; > + struct display_timing timing; > + struct videomode vm; > + int r; > + > + ddata->vcc = devm_regulator_get(&pdev->dev, "envdd"); > + if (IS_ERR(ddata->vcc)) { > + dev_err(&pdev->dev, "failed to get regulator\n"); > + return PTR_ERR(ddata->vcc); > + } > + > + r = regulator_enable(ddata->vcc); > + if (r != 0) { > + dev_err(&pdev->dev, "failed to enable regulator\n"); > + return r; > + } The regulator should be enabled when the panel is enabled, not at probe time. > +static const struct of_device_id sharp_ls_of_match[] = { > + { .compatible = "sharp,ls037v7dw01", }, At the moment our display drivers are OMAP specific, and for that reason we should prefix the compatible strings with "omapdss,". For example, drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c: { .compatible = "omapdss,panel-dsi-cm", }, But we should still have the right compatible string in the .dts, so we convert the compatible name in arch/arm/mach-omap2/display.c, with 'dss_compat_conv_list' array, to which this panel's name should be added. Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140509 00:24]: > On 09/05/14 02:33, Tony Lindgren wrote: > > +Example when connected to a omap2+ based device: > > + > > + lcd0: display { > > + compatible = "sharp,ls037v7dw01"; > > + power-supply = <&lcd_3v3>; > > + enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>; /* gpio152, lcd INI */ > > + reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>; /* gpio155, lcd RESB */ > > + mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH /* gpio154, lcd MO */ > > + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */ > > + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */ > > + > > + panel-timing { > > + clock-frequency = <19200000>; > > + hback-porch = <28>; > > + hactive = <480>; > > + hfront-porch = <1>; > > + hsync-len = <2>; > > + vback-porch = <1>; > > + vactive = <640>; > > + vfront-porch = <1>; > > + vsync-len = <1>; > > + hsync-active = <0>; > > + vsync-active = <0>; > > + de-active = <1>; > > + pixelclk-active = <1>; > > + }; > > I don't think we should define panel-timing here. We know it's > sharp,ls037v7dw01, so the driver knows the video timings. Also, if we > would extend the driver to support both resolution modes, it needs to > support two different timings, so the above doesn't work in that case > either. OK. It seems we can have at least two different timings for this panel, the VGA timing above and the QVGA timings that LDP uses that are listed in the .dts changes. > Although if the MO gpio is not controlled by the driver, we should tell > the driver whether that gpio is high or low. What do you have in mind for telling that? We should also tell the orientation of the panel: EVM VGA omapfb.rotate=3 LDP QVGA omapfb.rotate=0 Do you have something in mind for that? > At the moment our display drivers are OMAP specific, and for that reason > we should prefix the compatible strings with "omapdss,". For example, > drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c: > > { .compatible = "omapdss,panel-dsi-cm", }, > > But we should still have the right compatible string in the .dts, so we > convert the compatible name in arch/arm/mach-omap2/display.c, with > 'dss_compat_conv_list' array, to which this panel's name should be added. Oh so what do you want to have in the .dts file then? Regards, Tony -- 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 09/05/14 18:55, Tony Lindgren wrote: >> Although if the MO gpio is not controlled by the driver, we should tell >> the driver whether that gpio is high or low. > > What do you have in mind for telling that? We should also tell the > orientation of the panel: > > EVM VGA omapfb.rotate=3 > LDP QVGA omapfb.rotate=0 > > Do you have something in mind for that? Hmm, right. I guess all we can do is have boolean flags in the .dts for MO, LR and UD, which tells if the respective pins are hard-wires high or low. And say in the documentation that you must either have a proper GPIO, or use the flag, but not both. The panel mounting orientation is a different matter. But I think it is also something we should specify in the .dts. However, we don't have any SW support to handle it, and it's a bit unclear to me how it should be handled, so I think that has to be left for later. >> At the moment our display drivers are OMAP specific, and for that reason >> we should prefix the compatible strings with "omapdss,". For example, >> drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c: >> >> { .compatible = "omapdss,panel-dsi-cm", }, >> >> But we should still have the right compatible string in the .dts, so we >> convert the compatible name in arch/arm/mach-omap2/display.c, with >> 'dss_compat_conv_list' array, to which this panel's name should be added. > > Oh so what do you want to have in the .dts file then? The .dts should have the proper names. The idea of this hackery is that in the .dts we can have the proper compatible string. So in the .dts, we have, say: "sony,panel-foobar" Then, at boot time, omap's platform code changes that to: "omapdss,sony,panel-foobar". And our (omap specific) panel-foobar driver use that "omapdss,sony,panel-foobar" string. This way some other platform could do the same, and have their platform specific drivers handle the panel. At some point in the future we hopefully will have common panel drivers, and at that point we can remove that hackery. The .dts files will already be correct. Tomi
Hello Tomi, On Mon, May 12, 2014 at 9:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 09/05/14 18:55, Tony Lindgren wrote: > >>> Although if the MO gpio is not controlled by the driver, we should tell >>> the driver whether that gpio is high or low. >> >> What do you have in mind for telling that? We should also tell the >> orientation of the panel: >> >> EVM VGA omapfb.rotate=3 >> LDP QVGA omapfb.rotate=0 >> >> Do you have something in mind for that? > > Hmm, right. I guess all we can do is have boolean flags in the .dts for > MO, LR and UD, which tells if the respective pins are hard-wires high or > low. And say in the documentation that you must either have a proper > GPIO, or use the flag, but not both. > > The panel mounting orientation is a different matter. But I think it is > also something we should specify in the .dts. However, we don't have any > SW support to handle it, and it's a bit unclear to me how it should be > handled, so I think that has to be left for later. > >>> At the moment our display drivers are OMAP specific, and for that reason >>> we should prefix the compatible strings with "omapdss,". For example, >>> drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c: >>> >>> { .compatible = "omapdss,panel-dsi-cm", }, >>> >>> But we should still have the right compatible string in the .dts, so we >>> convert the compatible name in arch/arm/mach-omap2/display.c, with >>> 'dss_compat_conv_list' array, to which this panel's name should be added. >> >> Oh so what do you want to have in the .dts file then? > > The .dts should have the proper names. The idea of this hackery is that > in the .dts we can have the proper compatible string. So in the .dts, we > have, say: > > "sony,panel-foobar" > > Then, at boot time, omap's platform code changes that to: > > "omapdss,sony,panel-foobar". > > And our (omap specific) panel-foobar driver use that > "omapdss,sony,panel-foobar" string. > > This way some other platform could do the same, and have their platform > specific drivers handle the panel. > > At some point in the future we hopefully will have common panel drivers, > and at that point we can remove that hackery. The .dts files will > already be correct. > Maybe we can remove this hackery by relying on the fact that a compatible string can have a set of values that goes from more specific to more general. So you can have something like: compatible = "sony,panel-foobar", "omapdss,panel-foobar" So right now only "omapdss,panel-foobar" will be matched and later when we have common panel drivers then that driver could handle the "sony,panel-foobar" compatible string. Other platforms could do something similar and have compatible = "sony,panel-foobar", "baz,panel-foobar" That way you won't break DT backward compatible but still not require hacks on arch/arm/mach-omap2/display.c. We do the same for OMAP boards, we now have the following compatible string: compatible = "isee,omap3-igep0020", "ti,omap36xx", "ti,omap3"; There isn't a struct machine_desc that matches "isee,omap3-igep0020" but later if we find that we need some board specific initialization we could add one without breaking existing DTS. In fact it used to be a single machine_desc that matched "ti,omap3" for both omap36xx and omap34xx but later when some DT clocks changes were introduced we needed to split both SoC families. > Tomi > > Best regards, Javier -- 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 12/05/14 12:34, Javier Martinez Canillas wrote: > Maybe we can remove this hackery by relying on the fact that a > compatible string can have a set of values that goes from more > specific to more general. So you can have something like: > > compatible = "sony,panel-foobar", "omapdss,panel-foobar" > > So right now only "omapdss,panel-foobar" will be matched and later > when we have common panel drivers then that driver could handle the > "sony,panel-foobar" compatible string. > > Other platforms could do something similar and have > > compatible = "sony,panel-foobar", "baz,panel-foobar" > > That way you won't break DT backward compatible but still not require > hacks on arch/arm/mach-omap2/display.c. > > We do the same for OMAP boards, we now have the following compatible string: > > compatible = "isee,omap3-igep0020", "ti,omap36xx", "ti,omap3"; > > There isn't a struct machine_desc that matches "isee,omap3-igep0020" > but later if we find that we need some board specific initialization > we could add one without breaking existing DTS. In fact it used to be > a single machine_desc that matched "ti,omap3" for both omap36xx and > omap34xx but later when some DT clocks changes were introduced we > needed to split both SoC families. I think that's a different thing. "isee,omap3-igep0020" is a proper compatible string, if the board is "isee,...". No matter what software you run, that's correct and fine. The omapdss case is different, there the "omapdss" points to a sw thing, it does not describe the hardware. It's only needed as we don't have proper sw drivers for the devices. That said, I think what you describe would work. But I would rather keep the .dts files clean and correct, and keep that hacks hidden inside the kernel. Tomi
Hello Tomi, On Mon, May 12, 2014 at 11:40 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 12/05/14 12:34, Javier Martinez Canillas wrote: > >> Maybe we can remove this hackery by relying on the fact that a >> compatible string can have a set of values that goes from more >> specific to more general. So you can have something like: >> >> compatible = "sony,panel-foobar", "omapdss,panel-foobar" >> >> So right now only "omapdss,panel-foobar" will be matched and later >> when we have common panel drivers then that driver could handle the >> "sony,panel-foobar" compatible string. >> >> Other platforms could do something similar and have >> >> compatible = "sony,panel-foobar", "baz,panel-foobar" >> >> That way you won't break DT backward compatible but still not require >> hacks on arch/arm/mach-omap2/display.c. >> >> We do the same for OMAP boards, we now have the following compatible string: >> >> compatible = "isee,omap3-igep0020", "ti,omap36xx", "ti,omap3"; >> >> There isn't a struct machine_desc that matches "isee,omap3-igep0020" >> but later if we find that we need some board specific initialization >> we could add one without breaking existing DTS. In fact it used to be >> a single machine_desc that matched "ti,omap3" for both omap36xx and >> omap34xx but later when some DT clocks changes were introduced we >> needed to split both SoC families. > > I think that's a different thing. "isee,omap3-igep0020" is a proper > compatible string, if the board is "isee,...". No matter what software > you run, that's correct and fine. > > The omapdss case is different, there the "omapdss" points to a sw thing, > it does not describe the hardware. It's only needed as we don't have > proper sw drivers for the devices. > > That said, I think what you describe would work. But I would rather keep > the .dts files clean and correct, and keep that hacks hidden inside the > kernel. > Thanks for the explanation. Since DT are meant to describe hardware and not software I agree with you that we shouldn't leak an implementation detail to the DeviceTrees. And after all fortunately we don't have a stable API in the kernel like the one that is enforced in the DT so we can fix it later ;-) > Tomi > > Bets regards, Javier -- 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
* Javier Martinez Canillas <javier@dowhile0.org> [140512 03:01]: > Hello Tomi, > > On Mon, May 12, 2014 at 11:40 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > On 12/05/14 12:34, Javier Martinez Canillas wrote: > > > >> Maybe we can remove this hackery by relying on the fact that a > >> compatible string can have a set of values that goes from more > >> specific to more general. So you can have something like: > >> > >> compatible = "sony,panel-foobar", "omapdss,panel-foobar" > >> > >> So right now only "omapdss,panel-foobar" will be matched and later > >> when we have common panel drivers then that driver could handle the > >> "sony,panel-foobar" compatible string. > >> > >> Other platforms could do something similar and have > >> > >> compatible = "sony,panel-foobar", "baz,panel-foobar" > >> > >> That way you won't break DT backward compatible but still not require > >> hacks on arch/arm/mach-omap2/display.c. > >> > >> We do the same for OMAP boards, we now have the following compatible string: > >> > >> compatible = "isee,omap3-igep0020", "ti,omap36xx", "ti,omap3"; > >> > >> There isn't a struct machine_desc that matches "isee,omap3-igep0020" > >> but later if we find that we need some board specific initialization > >> we could add one without breaking existing DTS. In fact it used to be > >> a single machine_desc that matched "ti,omap3" for both omap36xx and > >> omap34xx but later when some DT clocks changes were introduced we > >> needed to split both SoC families. > > > > I think that's a different thing. "isee,omap3-igep0020" is a proper > > compatible string, if the board is "isee,...". No matter what software > > you run, that's correct and fine. > > > > The omapdss case is different, there the "omapdss" points to a sw thing, > > it does not describe the hardware. It's only needed as we don't have > > proper sw drivers for the devices. > > > > That said, I think what you describe would work. But I would rather keep > > the .dts files clean and correct, and keep that hacks hidden inside the > > kernel. > > > > Thanks for the explanation. Since DT are meant to describe hardware > and not software I agree with you that we shouldn't leak an > implementation detail to the DeviceTrees. > > And after all fortunately we don't have a stable API in the kernel > like the one that is enforced in the DT so we can fix it later ;-) This remapping of compatible flags sounds smelly to me, please discuss it first with the device tree people. I think we're better off just using the compatible properties limited to the simple-panel.txt for now and actually describe the real hardware. The fact that the panel is connected to DSS should be possible to find out based on the phandles. Regards, Tony -- 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 12/05/14 17:26, Tony Lindgren wrote: >>> The omapdss case is different, there the "omapdss" points to a sw thing, >>> it does not describe the hardware. It's only needed as we don't have >>> proper sw drivers for the devices. >>> >>> That said, I think what you describe would work. But I would rather keep >>> the .dts files clean and correct, and keep that hacks hidden inside the >>> kernel. >>> >> >> Thanks for the explanation. Since DT are meant to describe hardware >> and not software I agree with you that we shouldn't leak an >> implementation detail to the DeviceTrees. >> >> And after all fortunately we don't have a stable API in the kernel >> like the one that is enforced in the DT so we can fix it later ;-) > > This remapping of compatible flags sounds smelly to me, please discuss > it first with the device tree people. It's already in v3.15. I agree it's smelly, but it smelly only inside the kernel, not visible anywhere, and we can remove it when we have common panel drivers, without any modifications to .dts files. > I think we're better off just using the compatible properties limited > to the simple-panel.txt for now and actually describe the real > hardware. The fact that the panel is connected to DSS should be possible > to find out based on the phandles. I'm not sure what you mean. We do describe only the real hardware. The compatible string in the .dts file is "sharp,ls037v7dw01". Prepending the "omapdss" to the compatible string is required for the device/driver matching to work, because we can't use the "sharp,ls037v7dw01" compatible string in our omap specific panel driver. I'm not sure what it would give us to try to be compatible with simple-panel.txt. The simple-panel bindings won't probably be compatible with the future common display drivers in any case, as the simple-panel binding is just too limited and doesn't describe the hardware fully. Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140512 07:55]: > On 12/05/14 17:26, Tony Lindgren wrote: > > >>> The omapdss case is different, there the "omapdss" points to a sw thing, > >>> it does not describe the hardware. It's only needed as we don't have > >>> proper sw drivers for the devices. > >>> > >>> That said, I think what you describe would work. But I would rather keep > >>> the .dts files clean and correct, and keep that hacks hidden inside the > >>> kernel. > >>> > >> > >> Thanks for the explanation. Since DT are meant to describe hardware > >> and not software I agree with you that we shouldn't leak an > >> implementation detail to the DeviceTrees. > >> > >> And after all fortunately we don't have a stable API in the kernel > >> like the one that is enforced in the DT so we can fix it later ;-) > > > > This remapping of compatible flags sounds smelly to me, please discuss > > it first with the device tree people. > > It's already in v3.15. Oh great.. And you dumped it into arch/arm/mach-omap2 too and I somehow even acked it :( Looks like there's also the custom mux hacks. And custom hwmod hacks. And ongoing soc_is_xxx tinkering. Now let's not add _any_ new crap into arch/arm/mach-omap2/display.c. So please consider this a huge eternal NAK to add any more crap to arch/arm/mach-omap2/display.c from me. No more new soc_is beyond the soc_is_am43xx() you have in linux next. No more adding of_find_compatible_node() beyond ti,omap5-dss you have in linux next. And no more new panel remapping in this file as soon as you have found a better solution. Preferrably by v3.17 merge window. This file just needs to disappear. Yuk. > I agree it's smelly, but it smelly only inside the kernel, not visible > anywhere, and we can remove it when we have common panel drivers, > without any modifications to .dts files. > > > I think we're better off just using the compatible properties limited > > to the simple-panel.txt for now and actually describe the real > > hardware. The fact that the panel is connected to DSS should be possible > > to find out based on the phandles. > > I'm not sure what you mean. We do describe only the real hardware. The > compatible string in the .dts file is "sharp,ls037v7dw01". > > Prepending the "omapdss" to the compatible string is required for the > device/driver matching to work, because we can't use the > "sharp,ls037v7dw01" compatible string in our omap specific panel driver. > > I'm not sure what it would give us to try to be compatible with > simple-panel.txt. The simple-panel bindings won't probably be compatible > with the future common display drivers in any case, as the simple-panel > binding is just too limited and doesn't describe the hardware fully. Hmm what else does a panel need where the existing binding cannot be extended? Regards, Tony -- 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 12/05/14 18:51, Tony Lindgren wrote: >> It's already in v3.15. > > Oh great.. And you dumped it into arch/arm/mach-omap2 too and I somehow > even acked it :( Looks like there's also the custom mux hacks. And > custom hwmod hacks. And ongoing soc_is_xxx tinkering. Now let's not add The omap2_dss_hwmod_data, create_dss_pdev and create_simple_dss_pdev, omap_display_init stuff can be removed when the DT conversion has been done. For the omap4_dsi_mux_pads (or the omap5 dsi muxing that was recently discussed) I still have no solution, but I haven't spent time on it. I have dropped the omap5 dsi muxing from the latest series for that reason. dispc_disable_outputs and omap_dss_reset are needed because omap_device doesn't handle resetting DSS properly, as special steps need to be done for that. omap_dss_reset is called from the hwmod/omap_device code, so I don't think this code can be anywhere else. For the omap_display_get_version() I have no good solution. Making separate .dts versions for all the needed OMAP ES versions seems like a huge hassle to me, but that is one solution. I think that would mean having separate .dtsi files for omapdss for omap3_es1, omap3_es3plus, omap4_es1, omap4_es2, omap4_es3plus, and then having separate omapxxxx.dtsi files for all of those, and then separate board .dts files for the ES versions used on each board. Or is there some sane way to define such things in dts? > _any_ new crap into arch/arm/mach-omap2/display.c. > > So please consider this a huge eternal NAK to add any more crap to > arch/arm/mach-omap2/display.c from me. No more new soc_is beyond > the soc_is_am43xx() you have in linux next. No more adding > of_find_compatible_node() beyond ti,omap5-dss you have in linux next. > > And no more new panel remapping in this file as soon as you have found > a better solution. Preferrably by v3.17 merge window. This file just > needs to disappear. Yuk. Do you object to the compatible string remapping as such, or just that it's in arch/arm/mach-omap2? I guess nothing prevents me from moving it to drivers/, and having some early-ish initcall doing the job. If the remapping as such is horrible, I'm all ears for better ideas. I spent quite a lot of time on it, and that's the best I could come up with. It's rather simple prefixing of the compatible strings for selected devices. The purpose of that is to have proper data in the .dts files (which I think is very important) with the cost of having some rather simple internal hacks in the kernel, which can be removed immediately when we have a common display driver framework. >> I'm not sure what it would give us to try to be compatible with >> simple-panel.txt. The simple-panel bindings won't probably be compatible >> with the future common display drivers in any case, as the simple-panel >> binding is just too limited and doesn't describe the hardware fully. > > Hmm what else does a panel need where the existing binding cannot be > extended? The existing simple-panel binding doesn't describe the connections of the panel, i.e. its video input. I guess it can be extended, but I don't see what the benefit is of trying to create new panel bindings compatible with the simple-panel bindings. As I see, the simple-panel bindings work only with very limited use cases, where the drivers make assumptions. Simple panel bindings cannot be used with omapdss, nor can it be used with the future common display framework. But I'm not really familiar with using extending current bindings, and making new bindings compatible with old ones. Can you explain why it'd be good to have the sharp panel bindings compatible with simple-panel? In what kind of scenario would it be used? Tomi
On Tue, May 13, 2014 at 12:51 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 12/05/14 18:51, Tony Lindgren wrote: > >>> It's already in v3.15. >> >> Oh great.. And you dumped it into arch/arm/mach-omap2 too and I somehow >> even acked it :( Looks like there's also the custom mux hacks. And >> custom hwmod hacks. And ongoing soc_is_xxx tinkering. Now let's not add > > The omap2_dss_hwmod_data, create_dss_pdev and create_simple_dss_pdev, > omap_display_init stuff can be removed when the DT conversion has been done. > > For the omap4_dsi_mux_pads (or the omap5 dsi muxing that was recently > discussed) I still have no solution, but I haven't spent time on it. I > have dropped the omap5 dsi muxing from the latest series for that reason. > > dispc_disable_outputs and omap_dss_reset are needed because omap_device > doesn't handle resetting DSS properly, as special steps need to be done > for that. omap_dss_reset is called from the hwmod/omap_device code, so I > don't think this code can be anywhere else. > > For the omap_display_get_version() I have no good solution. Making > separate .dts versions for all the needed OMAP ES versions seems like a > huge hassle to me, but that is one solution. > > I think that would mean having separate .dtsi files for omapdss for > omap3_es1, omap3_es3plus, omap4_es1, omap4_es2, omap4_es3plus, and then > having separate omapxxxx.dtsi files for all of those, and then separate > board .dts files for the ES versions used on each board. > > Or is there some sane way to define such things in dts? > Unfortunately there isn't. I have a similar problem with the IGEP boards since there are just too many variations of them (e.g: DM3730 SoC vs OMAP3530 SoC, NAND vs OneNAND, 256MB vs 512MB flash memory sizes, with and without wifi chip, etc). Since dts are supposed to describe the hardware, each revision with a slighter variation forces to have a different dts. My solution was to not try to have a dts for every single variation in mainline but only one for the most common revision and that could be used as a reference to modify and ship on each device. Unfortunately that is not possible to you since each DSS IP block is used by many boards. >> _any_ new crap into arch/arm/mach-omap2/display.c. >> >> So please consider this a huge eternal NAK to add any more crap to >> arch/arm/mach-omap2/display.c from me. No more new soc_is beyond >> the soc_is_am43xx() you have in linux next. No more adding >> of_find_compatible_node() beyond ti,omap5-dss you have in linux next. >> >> And no more new panel remapping in this file as soon as you have found >> a better solution. Preferrably by v3.17 merge window. This file just >> needs to disappear. Yuk. > > Do you object to the compatible string remapping as such, or just that > it's in arch/arm/mach-omap2? > > I guess nothing prevents me from moving it to drivers/, and having some > early-ish initcall doing the job. > > If the remapping as such is horrible, I'm all ears for better ideas. I > spent quite a lot of time on it, and that's the best I could come up with. > > It's rather simple prefixing of the compatible strings for selected > devices. The purpose of that is to have proper data in the .dts files > (which I think is very important) with the cost of having some rather > simple internal hacks in the kernel, which can be removed immediately > when we have a common display driver framework. > Even though the compatible trick that I said before could be an alternative, it has the problem that does not describes the hardware as you said. The restriction of the DT being a stable API and get it right from the beginning forces us to make these kind of technical decisions. So, since we can change the kernel later but not the DTS, I agree with you that the remapping is the least bad of our options. >>> I'm not sure what it would give us to try to be compatible with >>> simple-panel.txt. The simple-panel bindings won't probably be compatible >>> with the future common display drivers in any case, as the simple-panel >>> binding is just too limited and doesn't describe the hardware fully. >> >> Hmm what else does a panel need where the existing binding cannot be >> extended? > > The existing simple-panel binding doesn't describe the connections of > the panel, i.e. its video input. I guess it can be extended, but I don't > see what the benefit is of trying to create new panel bindings > compatible with the simple-panel bindings. As I see, the simple-panel > bindings work only with very limited use cases, where the drivers make > assumptions. Simple panel bindings cannot be used with omapdss, nor can > it be used with the future common display framework. > > But I'm not really familiar with using extending current bindings, and > making new bindings compatible with old ones. Can you explain why it'd > be good to have the sharp panel bindings compatible with simple-panel? > In what kind of scenario would it be used? > > Tomi > > Best regards, Javier -- 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
* Javier Martinez Canillas <javier@dowhile0.org> [140513 04:40]: > On Tue, May 13, 2014 at 12:51 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > On 12/05/14 18:51, Tony Lindgren wrote: > > > >>> It's already in v3.15. > >> > >> Oh great.. And you dumped it into arch/arm/mach-omap2 too and I somehow > >> even acked it :( Looks like there's also the custom mux hacks. And > >> custom hwmod hacks. And ongoing soc_is_xxx tinkering. Now let's not add > > > > The omap2_dss_hwmod_data, create_dss_pdev and create_simple_dss_pdev, > > omap_display_init stuff can be removed when the DT conversion has been done. Yes great that helps :) > > For the omap4_dsi_mux_pads (or the omap5 dsi muxing that was recently > > discussed) I still have no solution, but I haven't spent time on it. I > > have dropped the omap5 dsi muxing from the latest series for that reason. OK > > dispc_disable_outputs and omap_dss_reset are needed because omap_device > > doesn't handle resetting DSS properly, as special steps need to be done > > for that. omap_dss_reset is called from the hwmod/omap_device code, so I > > don't think this code can be anywhere else. OK that we should be able to do with drivers/reset eventually. The reset and idle of drivers is also needed for unused devices too, so we can't have drivers do it in those cases. > > For the omap_display_get_version() I have no good solution. Making > > separate .dts versions for all the needed OMAP ES versions seems like a > > huge hassle to me, but that is one solution. > > > > I think that would mean having separate .dtsi files for omapdss for > > omap3_es1, omap3_es3plus, omap4_es1, omap4_es2, omap4_es3plus, and then > > having separate omapxxxx.dtsi files for all of those, and then separate > > board .dts files for the ES versions used on each board. > > > > Or is there some sane way to define such things in dts? > > > > Unfortunately there isn't. > > I have a similar problem with the IGEP boards since there are just too > many variations of them (e.g: DM3730 SoC vs OMAP3530 SoC, NAND vs > OneNAND, 256MB vs 512MB flash memory sizes, with and without wifi > chip, etc). > > Since dts are supposed to describe the hardware, each revision with a > slighter variation forces to have a different dts. My solution was to > not try to have a dts for every single variation in mainline but only > one for the most common revision and that could be used as a reference > to modify and ship on each device. Unfortunately that is not possible > to you since each DSS IP block is used by many boards. Well ideally the revision info for a device would come from device revision registers rather using the SoC revision. In the DSS case when the SoC revision is needed by a device it maybe it can be deciphered from a combination of compatible flag and the clock rate for example? But yeah I agree we still need revision detection in the kernel rather than try to create separate .dtsi files for sub-revisions. > >> _any_ new crap into arch/arm/mach-omap2/display.c. > >> > >> So please consider this a huge eternal NAK to add any more crap to > >> arch/arm/mach-omap2/display.c from me. No more new soc_is beyond > >> the soc_is_am43xx() you have in linux next. No more adding > >> of_find_compatible_node() beyond ti,omap5-dss you have in linux next. > >> > >> And no more new panel remapping in this file as soon as you have found > >> a better solution. Preferrably by v3.17 merge window. This file just > >> needs to disappear. Yuk. > > > > Do you object to the compatible string remapping as such, or just that > > it's in arch/arm/mach-omap2? It's something I'd rather not have under mach-omap2 as that means that I may need to deal with it too to some extent. And I don't think we need to do such remapping, we should be able to use the panel compatible strings as they are just fine. It should be possible to figure out from the device tree properties what controller the panel belongs to. Or for now, use the panel registration to figure out what display controller it belongs to. > > I guess nothing prevents me from moving it to drivers/, and having some > > early-ish initcall doing the job. /me likes this idea if you need it at all. Stuff like this tends to stay and spread, so I'd rather not do the remapping of compatible strings at all. > > If the remapping as such is horrible, I'm all ears for better ideas. I > > spent quite a lot of time on it, and that's the best I could come up with. > > > > It's rather simple prefixing of the compatible strings for selected > > devices. The purpose of that is to have proper data in the .dts files > > (which I think is very important) with the cost of having some rather > > simple internal hacks in the kernel, which can be removed immediately > > when we have a common display driver framework. > > > > Even though the compatible trick that I said before could be an > alternative, it has the problem that does not describes the hardware > as you said. The restriction of the DT being a stable API and get it > right from the beginning forces us to make these kind of technical > decisions. > > So, since we can change the kernel later but not the DTS, I agree with > you that the remapping is the least bad of our options. Yes the binding for the panel should just describe the panel so it can be used with whatever display controller. But we do have quite a few buses probing devices. How about set up the panel probing the same way? For the panels on display controller, just do the usual for_each_child_of_node(pdev->dev.of_node, child) and probe them? It seems the remapping of compatible strings is not needed in this as we're only picking up panels that are children of the display controller. > >>> I'm not sure what it would give us to try to be compatible with > >>> simple-panel.txt. The simple-panel bindings won't probably be compatible > >>> with the future common display drivers in any case, as the simple-panel > >>> binding is just too limited and doesn't describe the hardware fully. > >> > >> Hmm what else does a panel need where the existing binding cannot be > >> extended? > > > > The existing simple-panel binding doesn't describe the connections of > > the panel, i.e. its video input. I guess it can be extended, but I don't > > see what the benefit is of trying to create new panel bindings > > compatible with the simple-panel bindings. As I see, the simple-panel > > bindings work only with very limited use cases, where the drivers make > > assumptions. Simple panel bindings cannot be used with omapdss, nor can > > it be used with the future common display framework. Well it seems at least the reset and enable pin standard from that binding can be kept. > > But I'm not really familiar with using extending current bindings, and > > making new bindings compatible with old ones. Can you explain why it'd > > be good to have the sharp panel bindings compatible with simple-panel? > > In what kind of scenario would it be used? Ideally the panel binding just describes the panel and it should not matter which display controller it is a child of. Regards, Tony -- 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 13/05/14 18:25, Tony Lindgren wrote: > Well ideally the revision info for a device would come from device > revision registers rather using the SoC revision. In the DSS case when > the SoC revision is needed by a device it maybe it can be deciphered > from a combination of compatible flag and the clock rate for example? I've been trying that. The HW guys didn't bother to update the DSS revision registers, so they are useless. And, for example, the OMAP3 ES difference is only about bitfield widths in two registers. I tried writing "too long" value to the register on the earlier ES version, hoping that the extra bits would be kept at zero, but that wasn't the case. So I just don't see a way to detect this from the DSS's point of view. >>> Do you object to the compatible string remapping as such, or just that >>> it's in arch/arm/mach-omap2? > > It's something I'd rather not have under mach-omap2 as that means that > I may need to deal with it too to some extent. And I don't think we > need to do such remapping, we should be able to use the panel compatible > strings as they are just fine. It should be possible to figure out from > the device tree properties what controller the panel belongs to. Or > for now, use the panel registration to figure out what display controller > it belongs to. > >>> I guess nothing prevents me from moving it to drivers/, and having some >>> early-ish initcall doing the job. > > /me likes this idea if you need it at all. Stuff like this tends to stay > and spread, so I'd rather not do the remapping of compatible strings at > all. Yep. I'll look to this. Thinking about it now, it kind of makes more sense to have it in the omapdss's directory. >> So, since we can change the kernel later but not the DTS, I agree with >> you that the remapping is the least bad of our options. > > Yes the binding for the panel should just describe the panel so it can be > used with whatever display controller. But we do have quite a few buses > probing devices. How about set up the panel probing the same way? > For the panels on display controller, just do the usual > for_each_child_of_node(pdev->dev.of_node, child) and probe them? > > It seems the remapping of compatible strings is not needed in this > as we're only picking up panels that are children of the display > controller. The panels (or display encoders) are not (usually) children of the display controller. They are children of their respective control bus. Say, an i2c panel is a child of i2c bus. If there's no control bus, like is the case with the sharp panel, it's a platform device. The video paths of the panels and encoders are connected using the v4l2 style ports/endpoints. We can use those to see what display controller a panel is connected to, but only after the panel driver has already probed. We don't have control for the actual probing, as that happens with whatever the control bus is for the display component. >>>>> I'm not sure what it would give us to try to be compatible with >>>>> simple-panel.txt. The simple-panel bindings won't probably be compatible >>>>> with the future common display drivers in any case, as the simple-panel >>>>> binding is just too limited and doesn't describe the hardware fully. >>>> >>>> Hmm what else does a panel need where the existing binding cannot be >>>> extended? >>> >>> The existing simple-panel binding doesn't describe the connections of >>> the panel, i.e. its video input. I guess it can be extended, but I don't >>> see what the benefit is of trying to create new panel bindings >>> compatible with the simple-panel bindings. As I see, the simple-panel >>> bindings work only with very limited use cases, where the drivers make >>> assumptions. Simple panel bindings cannot be used with omapdss, nor can >>> it be used with the future common display framework. > > Well it seems at least the reset and enable pin standard from that > binding can be kept. Only enable gpio there. But even that's vague. Do you turn on the power before or after setting the enable gpio? How long delay should be between the power and the gpio? Different panels have different rules for the power-up. >>> But I'm not really familiar with using extending current bindings, and >>> making new bindings compatible with old ones. Can you explain why it'd >>> be good to have the sharp panel bindings compatible with simple-panel? >>> In what kind of scenario would it be used? > > Ideally the panel binding just describes the panel and it should not > matter which display controller it is a child of. Yes, but that means the panel bindings need to have enough information so that all display controllers can use it. Simple-panel bindings do not have enough information. The simple-panel bindings do not have information about the video bus input, and it doesn't even have information about the resolution or bitdepth of the panel. So I'm still asking, if we create sharp bindings that use the same properties as the simple-panel bindings, and define that sharp panel is compatible with simple-panel, what kind of scenario in practice would it be used in? Would the scenario be some other OS, that doesn't have a driver for the sharp panel, but has a driver for the simple-panel? That would only work if the sharp panel hardware is setup so that only the enable gpio is needed, so that quite a narrow definition of "compatible". Or is there some other scenario in which it could be used? Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140513 23:20]: > On 13/05/14 18:25, Tony Lindgren wrote: > > > Well ideally the revision info for a device would come from device > > revision registers rather using the SoC revision. In the DSS case when > > the SoC revision is needed by a device it maybe it can be deciphered > > from a combination of compatible flag and the clock rate for example? > > I've been trying that. The HW guys didn't bother to update the DSS > revision registers, so they are useless. And, for example, the OMAP3 ES > difference is only about bitfield widths in two registers. > > I tried writing "too long" value to the register on the earlier ES > version, hoping that the extra bits would be kept at zero, but that > wasn't the case. So I just don't see a way to detect this from the DSS's > point of view. > > >>> Do you object to the compatible string remapping as such, or just that > >>> it's in arch/arm/mach-omap2? > > > > It's something I'd rather not have under mach-omap2 as that means that > > I may need to deal with it too to some extent. And I don't think we > > need to do such remapping, we should be able to use the panel compatible > > strings as they are just fine. It should be possible to figure out from > > the device tree properties what controller the panel belongs to. Or > > for now, use the panel registration to figure out what display controller > > it belongs to. > > > >>> I guess nothing prevents me from moving it to drivers/, and having some > >>> early-ish initcall doing the job. > > > > /me likes this idea if you need it at all. Stuff like this tends to stay > > and spread, so I'd rather not do the remapping of compatible strings at > > all. > > Yep. I'll look to this. Thinking about it now, it kind of makes more > sense to have it in the omapdss's directory. OK thanks. > >> So, since we can change the kernel later but not the DTS, I agree with > >> you that the remapping is the least bad of our options. > > > > Yes the binding for the panel should just describe the panel so it can be > > used with whatever display controller. But we do have quite a few buses > > probing devices. How about set up the panel probing the same way? > > > For the panels on display controller, just do the usual > > for_each_child_of_node(pdev->dev.of_node, child) and probe them? > > > > It seems the remapping of compatible strings is not needed in this > > as we're only picking up panels that are children of the display > > controller. > > The panels (or display encoders) are not (usually) children of the > display controller. They are children of their respective control bus. > Say, an i2c panel is a child of i2c bus. If there's no control bus, like > is the case with the sharp panel, it's a platform device. OK > The video paths of the panels and encoders are connected using the v4l2 > style ports/endpoints. We can use those to see what display controller a > panel is connected to, but only after the panel driver has already > probed. We don't have control for the actual probing, as that happens > with whatever the control bus is for the display component. OK. So with generic panels, you can just let the panels probe with the right compatible flag then and let the ports/endpoints registration to figure out if the panel is usable for the display controller in question. > >>>>> I'm not sure what it would give us to try to be compatible with > >>>>> simple-panel.txt. The simple-panel bindings won't probably be compatible > >>>>> with the future common display drivers in any case, as the simple-panel > >>>>> binding is just too limited and doesn't describe the hardware fully. > >>>> > >>>> Hmm what else does a panel need where the existing binding cannot be > >>>> extended? > >>> > >>> The existing simple-panel binding doesn't describe the connections of > >>> the panel, i.e. its video input. I guess it can be extended, but I don't > >>> see what the benefit is of trying to create new panel bindings > >>> compatible with the simple-panel bindings. As I see, the simple-panel > >>> bindings work only with very limited use cases, where the drivers make > >>> assumptions. Simple panel bindings cannot be used with omapdss, nor can > >>> it be used with the future common display framework. > > > > Well it seems at least the reset and enable pin standard from that > > binding can be kept. > > Only enable gpio there. But even that's vague. Do you turn on the power > before or after setting the enable gpio? How long delay should be > between the power and the gpio? Different panels have different rules > for the power-up. Sure, it's a complex problem. But for the enable gpio.. Maybe the enable GPIO should be treated as a regulator? That would allow specifying first the source regulator startup delay, and then the panel has it's own startup delay. > >>> But I'm not really familiar with using extending current bindings, and > >>> making new bindings compatible with old ones. Can you explain why it'd > >>> be good to have the sharp panel bindings compatible with simple-panel? > >>> In what kind of scenario would it be used? > > > > Ideally the panel binding just describes the panel and it should not > > matter which display controller it is a child of. > > Yes, but that means the panel bindings need to have enough information > so that all display controllers can use it. Simple-panel bindings do not > have enough information. The simple-panel bindings do not have > information about the video bus input, and it doesn't even have > information about the resolution or bitdepth of the panel. Some of that you can hide into the panel driver based on the compatible flag. So why not already do something like this in the .dts files instead of the remapping: compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01"; And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c would only claim to be compatible with "sharp,ls037v7dw01-omap-dss". Then when the common panel framework is available, you can stop parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss" in the .dts files. > So I'm still asking, if we create sharp bindings that use the same > properties as the simple-panel bindings, and define that sharp panel is > compatible with simple-panel, what kind of scenario in practice would it > be used in? Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss" until some other SoC notices it can use the GPIO parts of the panel code at least :) > Would the scenario be some other OS, that doesn't have a driver for the > sharp panel, but has a driver for the simple-panel? That would only work > if the sharp panel hardware is setup so that only the enable gpio is > needed, so that quite a narrow definition of "compatible". That's where we can use the compatible flags and just avoid parsing the generic compatible flag unless some common framework is available. > Or is there some other scenario in which it could be used? I guess other OS or other SoC with a different display controller but the same panel. Regards, Tony -- 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 14/05/14 19:02, Tony Lindgren wrote: >> The video paths of the panels and encoders are connected using the v4l2 >> style ports/endpoints. We can use those to see what display controller a >> panel is connected to, but only after the panel driver has already >> probed. We don't have control for the actual probing, as that happens >> with whatever the control bus is for the display component. > > OK. So with generic panels, you can just let the panels probe with > the right compatible flag then and let the ports/endpoints registration > to figure out if the panel is usable for the display controller in > question. I'm not sure what you mean here. Do you mean with the future common display framework? There's no need to figure out anything there, as supposedly the .dts has been written correctly and the panel and the display controller work together. If you mean with the current kernel, there's still nothing to figure out. We can have only single driver with a particular compatible string. And as our current drivers are omap specific, it makes sense to have their compatible string be something omap-ish. And if the .dts file connects the display controller and the panel, then they must be usable together. >>> Well it seems at least the reset and enable pin standard from that >>> binding can be kept. >> >> Only enable gpio there. But even that's vague. Do you turn on the power >> before or after setting the enable gpio? How long delay should be >> between the power and the gpio? Different panels have different rules >> for the power-up. > > Sure, it's a complex problem. But for the enable gpio.. > > Maybe the enable GPIO should be treated as a regulator? That would allow > specifying first the source regulator startup delay, and then the > panel has it's own startup delay. Well... I don't know. Sounds rather hacky to me. We have the option to have a specific driver for this panel, and that driver can handle all the delays and power-up sequences just right in a clean manner. >>>>> But I'm not really familiar with using extending current bindings, and >>>>> making new bindings compatible with old ones. Can you explain why it'd >>>>> be good to have the sharp panel bindings compatible with simple-panel? >>>>> In what kind of scenario would it be used? >>> >>> Ideally the panel binding just describes the panel and it should not >>> matter which display controller it is a child of. >> >> Yes, but that means the panel bindings need to have enough information >> so that all display controllers can use it. Simple-panel bindings do not >> have enough information. The simple-panel bindings do not have >> information about the video bus input, and it doesn't even have >> information about the resolution or bitdepth of the panel. > > Some of that you can hide into the panel driver based on the compatible > flag. So why not already do something like this in the .dts files > instead of the remapping: > > compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01"; > > And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c > would only claim to be compatible with "sharp,ls037v7dw01-omap-dss". > > Then when the common panel framework is available, you can stop > parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need > to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss" > in the .dts files. Hmm, I don't see how this relates to the simple-panel bindings. But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an alternative for the compatible-string conversion we do now. I guess it's a matter of taste, but I rather hide it inside the kernel, in an internal omapdss file, than pollute the .dts files with those compatible strings. >> So I'm still asking, if we create sharp bindings that use the same >> properties as the simple-panel bindings, and define that sharp panel is >> compatible with simple-panel, what kind of scenario in practice would it >> be used in? > > Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss" > until some other SoC notices it can use the GPIO parts of the panel > code at least :) > >> Would the scenario be some other OS, that doesn't have a driver for the >> sharp panel, but has a driver for the simple-panel? That would only work >> if the sharp panel hardware is setup so that only the enable gpio is >> needed, so that quite a narrow definition of "compatible". > > That's where we can use the compatible flags and just avoid parsing > the generic compatible flag unless some common framework is available. Hmm, sorry, I don't follow. My question was about the simple-panel bindings, not common display framework. You were saying that the sharp bindings should be compatible with simple-panel bindings. I'm still trying to understand what benefit does that give us. As I see it, the sharp panel could be used with the simple-panel bindings only in certain special case, where all the mode pins and the reset are hardwired in the board hardware, and they are hardwired in a certain state (all hardwired low, probably), which matches what the simple-panel driver expects. Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 02:24]: > On 14/05/14 19:02, Tony Lindgren wrote: > > >> The video paths of the panels and encoders are connected using the v4l2 > >> style ports/endpoints. We can use those to see what display controller a > >> panel is connected to, but only after the panel driver has already > >> probed. We don't have control for the actual probing, as that happens > >> with whatever the control bus is for the display component. > > > > OK. So with generic panels, you can just let the panels probe with > > the right compatible flag then and let the ports/endpoints registration > > to figure out if the panel is usable for the display controller in > > question. > > I'm not sure what you mean here. > > Do you mean with the future common display framework? There's no need to > figure out anything there, as supposedly the .dts has been written > correctly and the panel and the display controller work together. OK. Yes I meant for future use. > If you mean with the current kernel, there's still nothing to figure > out. We can have only single driver with a particular compatible string. > And as our current drivers are omap specific, it makes sense to have > their compatible string be something omap-ish. And if the .dts file > connects the display controller and the panel, then they must be usable > together. Yup. > >>> Well it seems at least the reset and enable pin standard from that > >>> binding can be kept. > >> > >> Only enable gpio there. But even that's vague. Do you turn on the power > >> before or after setting the enable gpio? How long delay should be > >> between the power and the gpio? Different panels have different rules > >> for the power-up. > > > > Sure, it's a complex problem. But for the enable gpio.. > > > > Maybe the enable GPIO should be treated as a regulator? That would allow > > specifying first the source regulator startup delay, and then the > > panel has it's own startup delay. > > Well... I don't know. Sounds rather hacky to me. We have the option to > have a specific driver for this panel, and that driver can handle all > the delays and power-up sequences just right in a clean manner. I guess we could have a generic startup-latency property for the GPIOs. > >>>>> But I'm not really familiar with using extending current bindings, and > >>>>> making new bindings compatible with old ones. Can you explain why it'd > >>>>> be good to have the sharp panel bindings compatible with simple-panel? > >>>>> In what kind of scenario would it be used? > >>> > >>> Ideally the panel binding just describes the panel and it should not > >>> matter which display controller it is a child of. > >> > >> Yes, but that means the panel bindings need to have enough information > >> so that all display controllers can use it. Simple-panel bindings do not > >> have enough information. The simple-panel bindings do not have > >> information about the video bus input, and it doesn't even have > >> information about the resolution or bitdepth of the panel. > > > > Some of that you can hide into the panel driver based on the compatible > > flag. So why not already do something like this in the .dts files > > instead of the remapping: > > > > compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01"; > > > > And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c > > would only claim to be compatible with "sharp,ls037v7dw01-omap-dss". > > > > Then when the common panel framework is available, you can stop > > parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need > > to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss" > > in the .dts files. > > Hmm, I don't see how this relates to the simple-panel bindings. > > But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an > alternative for the compatible-string conversion we do now. I guess it's > a matter of taste, but I rather hide it inside the kernel, in an > internal omapdss file, than pollute the .dts files with those compatible > strings. Well it avoid you parsing through all the nodes during booting and leaves out the function to do remapping. And removes the need for maintaining a custom display mapping table. I'd say that's a pretty good list of advantages right there :) > >> So I'm still asking, if we create sharp bindings that use the same > >> properties as the simple-panel bindings, and define that sharp panel is > >> compatible with simple-panel, what kind of scenario in practice would it > >> be used in? > > > > Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss" > > until some other SoC notices it can use the GPIO parts of the panel > > code at least :) > > > >> Would the scenario be some other OS, that doesn't have a driver for the > >> sharp panel, but has a driver for the simple-panel? That would only work > >> if the sharp panel hardware is setup so that only the enable gpio is > >> needed, so that quite a narrow definition of "compatible". > > > > That's where we can use the compatible flags and just avoid parsing > > the generic compatible flag unless some common framework is available. > > Hmm, sorry, I don't follow. My question was about the simple-panel > bindings, not common display framework. > > You were saying that the sharp bindings should be compatible with > simple-panel bindings. I'm still trying to understand what benefit does > that give us. Oh sorry, for that part I don't really have an opinion. If you think the simple-panel binding is not going to be usable in the long run, then I'm fine with whatever binding you display guys come up with and prefer to use. > As I see it, the sharp panel could be used with the simple-panel > bindings only in certain special case, where all the mode pins and the > reset are hardwired in the board hardware, and they are hardwired in a > certain state (all hardwired low, probably), which matches what the > simple-panel driver expects. OK. Maybe take a stab at updating the binding for this as I don't know what you want to do with it? Regards, Tony -- 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 15/05/14 21:21, Tony Lindgren wrote: >> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an >> alternative for the compatible-string conversion we do now. I guess it's >> a matter of taste, but I rather hide it inside the kernel, in an >> internal omapdss file, than pollute the .dts files with those compatible >> strings. > > Well it avoid you parsing through all the nodes during booting > and leaves out the function to do remapping. And removes the need > for maintaining a custom display mapping table. I'd say that's a > pretty good list of advantages right there :) Yep... I don't know. Maybe I'm being too careful about doing wrong things with .dts. I just like it more if any hacks are in kernel code, which I can remove without anyone noticing. Anyway, we already have board.dts files using the non-omapified compatible strings in the mainline, so if I would now add the omapified compatible strings to .dts files, those old board.dts files would break. So I guess the choice has already been made. Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 22:57]: > On 15/05/14 21:21, Tony Lindgren wrote: > > >> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an > >> alternative for the compatible-string conversion we do now. I guess it's > >> a matter of taste, but I rather hide it inside the kernel, in an > >> internal omapdss file, than pollute the .dts files with those compatible > >> strings. > > > > Well it avoid you parsing through all the nodes during booting > > and leaves out the function to do remapping. And removes the need > > for maintaining a custom display mapping table. I'd say that's a > > pretty good list of advantages right there :) > > Yep... I don't know. Maybe I'm being too careful about doing wrong > things with .dts. I just like it more if any hacks are in kernel code, > which I can remove without anyone noticing. > > Anyway, we already have board.dts files using the non-omapified > compatible strings in the mainline, so if I would now add the omapified > compatible strings to .dts files, those old board.dts files would break. > > So I guess the choice has already been made. I really think you should remove this misuse of device tree code ASAP. It's better to fix it up now than carry the hack for next two years and keep on adding to it. Regards, Tony -- 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 Fri, May 16, 2014 at 09:07:17AM -0700, Tony Lindgren wrote: > * Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 22:57]: > > On 15/05/14 21:21, Tony Lindgren wrote: > > >> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an > > >> alternative for the compatible-string conversion we do now. I guess it's > > >> a matter of taste, but I rather hide it inside the kernel, in an > > >> internal omapdss file, than pollute the .dts files with those compatible > > >> strings. > > > > > > Well it avoid you parsing through all the nodes during booting > > > and leaves out the function to do remapping. And removes the need > > > for maintaining a custom display mapping table. I'd say that's a > > > pretty good list of advantages right there :) > > > > Yep... I don't know. Maybe I'm being too careful about doing wrong > > things with .dts. I just like it more if any hacks are in kernel code, > > which I can remove without anyone noticing. > > > > Anyway, we already have board.dts files using the non-omapified > > compatible strings in the mainline, so if I would now add the omapified > > compatible strings to .dts files, those old board.dts files would break. > > > > So I guess the choice has already been made. > > I really think you should remove this misuse of device tree code ASAP. > It's better to fix it up now than carry the hack for next two years > and keep on adding to it. IMHO appending -omap-dss to a random device is an even bigger hack, since its adding lots of bloat to the API. Let's assume there is another OS using DT for ARM, but has no proper API for SPI controllers and it introduces your hack to SPI devices. That would mean each SPI device has -omap-spi appended (or -exynos-spi, -foo-spi, ...). At least I would blame them for creating a huge unmaintainable mess. I think Tomi's workaround is a much better hack, since it keeps the API clean. If the code simply prefixes "omapdss," to all "child"-devices of omapdss it can be left mostly untouched, too. -- Sebastian
* Sebastian Reichel <sre@ring0.de> [140516 10:42]: > On Fri, May 16, 2014 at 09:07:17AM -0700, Tony Lindgren wrote: > > * Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 22:57]: > > > On 15/05/14 21:21, Tony Lindgren wrote: > > > >> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an > > > >> alternative for the compatible-string conversion we do now. I guess it's > > > >> a matter of taste, but I rather hide it inside the kernel, in an > > > >> internal omapdss file, than pollute the .dts files with those compatible > > > >> strings. > > > > > > > > Well it avoid you parsing through all the nodes during booting > > > > and leaves out the function to do remapping. And removes the need > > > > for maintaining a custom display mapping table. I'd say that's a > > > > pretty good list of advantages right there :) > > > > > > Yep... I don't know. Maybe I'm being too careful about doing wrong > > > things with .dts. I just like it more if any hacks are in kernel code, > > > which I can remove without anyone noticing. > > > > > > Anyway, we already have board.dts files using the non-omapified > > > compatible strings in the mainline, so if I would now add the omapified > > > compatible strings to .dts files, those old board.dts files would break. > > > > > > So I guess the choice has already been made. > > > > I really think you should remove this misuse of device tree code ASAP. > > It's better to fix it up now than carry the hack for next two years > > and keep on adding to it. > > IMHO appending -omap-dss to a random device is an even bigger hack, > since its adding lots of bloat to the API. Let's assume there is > another OS using DT for ARM, but has no proper API for SPI > controllers and it introduces your hack to SPI devices. That would > mean each SPI device has -omap-spi appended (or -exynos-spi, > -foo-spi, ...). At least I would blame them for creating a huge > unmaintainable mess. I think you're misunderstanding. I do not want the naming to be Linux specific. The naming should naturally be as hardware specific as possible. In this case something like: compatible = "sharp,ls037v7dw01-dss", "sharp,ls037v7dw01"; Or we should probably use: compatible = "sharp,ls037v7dw01-dpi", "sharp,ls037v7dw01"; As dpi here reflects the hardware it's connected to. The dss is probably a Linux name. Not use what you're after with the SPI example though, but sounds like that's something different. > I think Tomi's workaround is a much better hack, since it keeps the > API clean. If the code simply prefixes "omapdss," to all > "child"-devices of omapdss it can be left mostly untouched, too. AFAIK that remapping not needed at all. All that is already available with the compatible flags. Regards, Tony -- 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
* Tony Lindgren <tony@atomide.com> [140516 11:02]: > * Sebastian Reichel <sre@ring0.de> [140516 10:42]: > > On Fri, May 16, 2014 at 09:07:17AM -0700, Tony Lindgren wrote: > > > * Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 22:57]: > > > > On 15/05/14 21:21, Tony Lindgren wrote: > > > > >> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an > > > > >> alternative for the compatible-string conversion we do now. I guess it's > > > > >> a matter of taste, but I rather hide it inside the kernel, in an > > > > >> internal omapdss file, than pollute the .dts files with those compatible > > > > >> strings. > > > > > > > > > > Well it avoid you parsing through all the nodes during booting > > > > > and leaves out the function to do remapping. And removes the need > > > > > for maintaining a custom display mapping table. I'd say that's a > > > > > pretty good list of advantages right there :) > > > > > > > > Yep... I don't know. Maybe I'm being too careful about doing wrong > > > > things with .dts. I just like it more if any hacks are in kernel code, > > > > which I can remove without anyone noticing. > > > > > > > > Anyway, we already have board.dts files using the non-omapified > > > > compatible strings in the mainline, so if I would now add the omapified > > > > compatible strings to .dts files, those old board.dts files would break. > > > > > > > > So I guess the choice has already been made. > > > > > > I really think you should remove this misuse of device tree code ASAP. > > > It's better to fix it up now than carry the hack for next two years > > > and keep on adding to it. > > > > IMHO appending -omap-dss to a random device is an even bigger hack, > > since its adding lots of bloat to the API. Let's assume there is > > another OS using DT for ARM, but has no proper API for SPI > > controllers and it introduces your hack to SPI devices. That would > > mean each SPI device has -omap-spi appended (or -exynos-spi, > > -foo-spi, ...). At least I would blame them for creating a huge > > unmaintainable mess. > > I think you're misunderstanding. I do not want the naming to > be Linux specific. The naming should naturally be as hardware > specific as possible. In this case something like: > > compatible = "sharp,ls037v7dw01-dss", "sharp,ls037v7dw01"; > > Or we should probably use: > > compatible = "sharp,ls037v7dw01-dpi", "sharp,ls037v7dw01"; > > As dpi here reflects the hardware it's connected to. The dss > is probably a Linux name. > > Not use what you're after with the SPI example though, but sounds > like that's something different. > > > I think Tomi's workaround is a much better hack, since it keeps the > > API clean. If the code simply prefixes "omapdss," to all > > "child"-devices of omapdss it can be left mostly untouched, too. > > AFAIK that remapping not needed at all. All that is already > available with the compatible flags. And alternatively we can also just use the sharp,ls037v7dw01 in the panel driver(s). We can have the panels bail out in driver probe based on of_machine_is_compatible if nothing else is available. Also, currently the remapping code also probably keeps prepending more and more omapdss,omapdss,omapdss,... on each kexec reboot? And it would be quite silly for each display controller to have to do their own remapping for shared panels? If we still still despite all these reasons decide to go with the remapping of the compatible strings, it should really be a Linux generic (or drivers/video generic function), not implemented for each display controller. Cheers, Tony -- 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 16/05/14 21:01, Tony Lindgren wrote: >> IMHO appending -omap-dss to a random device is an even bigger hack, >> since its adding lots of bloat to the API. Let's assume there is >> another OS using DT for ARM, but has no proper API for SPI >> controllers and it introduces your hack to SPI devices. That would >> mean each SPI device has -omap-spi appended (or -exynos-spi, >> -foo-spi, ...). At least I would blame them for creating a huge >> unmaintainable mess. > > I think you're misunderstanding. I do not want the naming to > be Linux specific. The naming should naturally be as hardware > specific as possible. In this case something like: > > compatible = "sharp,ls037v7dw01-dss", "sharp,ls037v7dw01"; > > Or we should probably use: > > compatible = "sharp,ls037v7dw01-dpi", "sharp,ls037v7dw01"; > > As dpi here reflects the hardware it's connected to. The dss > is probably a Linux name. Well, "dss" or "omapdss" is as much a hardware term as "dpi". And "dpi" wouldn't really be a good extension, as what we want is an omapdss driver specific compatible string. So I think "-omapdss" is the best extension if that method is used. But I don't think that's really the point. The point is that the panel's compatible string should be "sharp,ls037v7dw01", nothing else. All the variations of "sharp,ls037v7dw01-dss" are not correct, and are only made for Linux SW reasons. So I would say they are Linux specific SW names, even if the words themselves are also HW terms. > Not use what you're after with the SPI example though, but sounds > like that's something different. I think Sebastien's example is just like the issue here. Tomi
On 17/05/14 00:39, Tony Lindgren wrote: >> AFAIK that remapping not needed at all. All that is already >> available with the compatible flags. > > And alternatively we can also just use the sharp,ls037v7dw01 > in the panel driver(s). We can have the panels bail out in driver > probe based on of_machine_is_compatible if nothing else is available. I don't think that would work. Then we could have two panel drivers, both loaded into the kernel and both having the same driver name and the same compatible-string. I'm not sure if it's even possible to load both of those drivers at the same time, but if it was, and the first one would get probe() called for a device, and the driver would return an error, I'm quite sure the driver framework won't continue trying with the other driver, as the first driver already matched for the device. > Also, currently the remapping code also probably keeps prepending > more and more omapdss,omapdss,omapdss,... on each kexec reboot? And I don't think so. The code looks for, say, "ti,tfp410", and it wouldn't match for "omapdss,ti,tfp410". > it would be quite silly for each display controller to have to do > their own remapping for shared panels? It would, but wouldn't it be just as silly (well, even more silly in my opinion) to have each display controller require driver specific compatible strings for the panels used? Until we have a common display framework, we need _some_ way to handle this problem. Either we mess up the .dts files as you suggest, or we have a driver internal hack. > If we still still despite all these reasons decide to go with > the remapping of the compatible strings, it should really be a > Linux generic (or drivers/video generic function), not implemented > for each display controller. Well, I sure hope nobody else has to implement similar thing. The reason why we have this code, and others do not, is mainly that omapdss is maybe the first driver to implement the display components properly, both in the SW side and in the .dts side, and that I wanted omapdss to behave correctly even if there are other display subsystems/panels compiled into the same kernel. To open that up a bit, traditionally other display driver architectures have not had drivers for each display "entity", like the panels. So you would really just have the display subsystem in the .dts, and some raw data there (panel timings) to get the panel running. Now, with omapdss, we have separate drivers for each display entity. Unfortunately those drivers are all tied to the omapdss driver's API (and cannot thus be used on anything else than OMAP), as there's not yet a driver framework for display entities (DRM is a bit different thing, in my opinion). Everything with that was simple with platform data, as the omap board files created the panel devices, and there were no name clashing problems with other platforms. With DT that changed, as the bindings are common, and thus the compatible strings are common also. But we still have omapdss specific panel drivers. This is the reason we do the compatible-string conversion hack. As soon as we can create platform agnostic panel drivers, we can remove the hack. And, I want to remind, it's an omapdss internal hack (after the patch I sent), not visible to anyone else. The .dts files, the arch/arm files, they are all not touched. So I don't quite understand why you see it as such a bad thing. It's ugly, sure, but what harm does it do (except some maintainer burden, for me)? Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140519 02:49]: > On 17/05/14 00:39, Tony Lindgren wrote: > > >> AFAIK that remapping not needed at all. All that is already > >> available with the compatible flags. > > > > And alternatively we can also just use the sharp,ls037v7dw01 > > in the panel driver(s). We can have the panels bail out in driver > > probe based on of_machine_is_compatible if nothing else is available. > > I don't think that would work. Then we could have two panel drivers, > both loaded into the kernel and both having the same driver name and the > same compatible-string. > > I'm not sure if it's even possible to load both of those drivers at the > same time, but if it was, and the first one would get probe() called for > a device, and the driver would return an error, I'm quite sure the > driver framework won't continue trying with the other driver, as the > first driver already matched for the device. No need to load multiple drivers at this point. That's why I'm saying you can bail out on the undesired display controller probes using of_machine_is_compatible test. It's not that uncommon for drivers to do: $ git grep of_machine_is_compatible drivers/ | wc -l 116 > > Also, currently the remapping code also probably keeps prepending > > more and more omapdss,omapdss,omapdss,... on each kexec reboot? And > > I don't think so. The code looks for, say, "ti,tfp410", and it wouldn't > match for "omapdss,ti,tfp410". OK > > it would be quite silly for each display controller to have to do > > their own remapping for shared panels? > > It would, but wouldn't it be just as silly (well, even more silly in my > opinion) to have each display controller require driver specific > compatible strings for the panels used? Not driver specific, but hardware package specific. That's being done all the time. For example, compatible strings like "nvidia,tegra124", "ti,omap5" describe a package of an ARM core and various devices. But by using of_machine_is_compatible in the display drivers, you can use the right compatible flags from the start if you want to go that way. > Until we have a common display framework, we need _some_ way to handle > this problem. Either we mess up the .dts files as you suggest, or we > have a driver internal hack. It seems that there's no need to mess up the .dts files here by using the of_machine_is_compatible check in the drivers. > > If we still still despite all these reasons decide to go with > > the remapping of the compatible strings, it should really be a > > Linux generic (or drivers/video generic function), not implemented > > for each display controller. > > Well, I sure hope nobody else has to implement similar thing. Suuuure.. I've heard that about 20 times before and guess what happenened? Things never got fixed until some poor maintainer had to start fixing it all over the place about three years later. > The reason why we have this code, and others do not, is mainly that > omapdss is maybe the first driver to implement the display components > properly, both in the SW side and in the .dts side, and that I wanted > omapdss to behave correctly even if there are other display > subsystems/panels compiled into the same kernel. > > To open that up a bit, traditionally other display driver architectures > have not had drivers for each display "entity", like the panels. So you > would really just have the display subsystem in the .dts, and some raw > data there (panel timings) to get the panel running. Right, I believe that is the way to go, no doubt about it. > Now, with omapdss, we have separate drivers for each display entity. > Unfortunately those drivers are all tied to the omapdss driver's API > (and cannot thus be used on anything else than OMAP), as there's not yet > a driver framework for display entities (DRM is a bit different thing, > in my opinion). > > Everything with that was simple with platform data, as the omap board > files created the panel devices, and there were no name clashing > problems with other platforms. > > With DT that changed, as the bindings are common, and thus the > compatible strings are common also. But we still have omapdss specific > panel drivers. This is the reason we do the compatible-string conversion > hack. As soon as we can create platform agnostic panel drivers, we can > remove the hack. > > And, I want to remind, it's an omapdss internal hack (after the patch I > sent), not visible to anyone else. The .dts files, the arch/arm files, > they are all not touched. So I don't quite understand why you see it as > such a bad thing. It's ugly, sure, but what harm does it do (except some > maintainer burden, for me)? Here's a list of things that bothers me: 1. Searching through the dtb from a driver instead of relying on the driver probe mechanism for the components in question. If the parsing of the dtb needs to be done it should be done in a Linux generic way from some framework instead. 2. Having to do this all early before we even have any debug console available. We are trying to move everything to happen later on to avoid all the issues related to initializing things early. 3. Having to maintain a database in kernel of display mappings separately in addition to the .dts files. 4. Having this hack limited to device tree based booting while we are trying to unify the functions for drivers to use to get their resources. 5. Seeing the possibility of this spreading to other drivers. Regards, Tony -- 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
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140519 02:22]: > On 16/05/14 21:01, Tony Lindgren wrote: > > >> IMHO appending -omap-dss to a random device is an even bigger hack, > >> since its adding lots of bloat to the API. Let's assume there is > >> another OS using DT for ARM, but has no proper API for SPI > >> controllers and it introduces your hack to SPI devices. That would > >> mean each SPI device has -omap-spi appended (or -exynos-spi, > >> -foo-spi, ...). At least I would blame them for creating a huge > >> unmaintainable mess. > > > > I think you're misunderstanding. I do not want the naming to > > be Linux specific. The naming should naturally be as hardware > > specific as possible. In this case something like: > > > > compatible = "sharp,ls037v7dw01-dss", "sharp,ls037v7dw01"; > > > > Or we should probably use: > > > > compatible = "sharp,ls037v7dw01-dpi", "sharp,ls037v7dw01"; > > > > As dpi here reflects the hardware it's connected to. The dss > > is probably a Linux name. > > Well, "dss" or "omapdss" is as much a hardware term as "dpi". And "dpi" > wouldn't really be a good extension, as what we want is an omapdss > driver specific compatible string. So I think "-omapdss" is the best > extension if that method is used. > > But I don't think that's really the point. The point is that the panel's > compatible string should be "sharp,ls037v7dw01", nothing else. > > All the variations of "sharp,ls037v7dw01-dss" are not correct, and are > only made for Linux SW reasons. So I would say they are Linux specific > SW names, even if the words themselves are also HW terms. If you really want to use only "sharp,ls037v7dw01" as the compatible string, then you can still do that and bail out from the undesired drivers by using of_machine_is_compatible check. In many cases however we do have multiple compatible strings that describe how the device is wired. See drivers/tty/serial/of_serial.c for example. It has "ns16550" but then it also has additional "nvidia,tegra20-uart", "nxp,lpc3220-uart" and "ibm,qpace-nwp-serial". > > Not use what you're after with the SPI example though, but sounds > > like that's something different. > > I think Sebastien's example is just like the issue here. Hmm is there some existing example in the kernel like that? Regards, Tony -- 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 Monday 19 May 2014 08:57:33 Tony Lindgren wrote: > No need to load multiple drivers at this point. That's why I'm saying > you can bail out on the undesired display controller probes using > of_machine_is_compatible test. It's not that uncommon for drivers to do: > > $ git grep of_machine_is_compatible drivers/ | wc -l > 116 I think this is widely seen as bad style, and most of the examples are testing for Power Mac specific workarounds for stuff that Apple didn't put in DT elsewhere. Arnd -- 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 19/05/14 18:57, Tony Lindgren wrote: >> I'm not sure if it's even possible to load both of those drivers at the >> same time, but if it was, and the first one would get probe() called for >> a device, and the driver would return an error, I'm quite sure the >> driver framework won't continue trying with the other driver, as the >> first driver already matched for the device. > > No need to load multiple drivers at this point. That's why I'm saying > you can bail out on the undesired display controller probes using > of_machine_is_compatible test. It's not that uncommon for drivers to do: Hmm, I don't follow. If both, say, omap and exynos have a panel driver for the same sharp panel, both need a driver for it (presuming exynos has a proper display component driver system, which may not be true). Both drivers would be loaded at boot time for the probe to be even to be possible. Even if the other one bails out from a device probe using of_machine_is_compatible, afaik the driver framework will stop probing at that point, as that driver reports being compatible with the device in question. ... Ah, ok, now I see. I think you mean module init, not probe? For some reason I was just thinking about the probe stage. Maybe that would work. I need to test tomorrow. >>> it would be quite silly for each display controller to have to do >>> their own remapping for shared panels? >> >> It would, but wouldn't it be just as silly (well, even more silly in my >> opinion) to have each display controller require driver specific >> compatible strings for the panels used? > > Not driver specific, but hardware package specific. That's being done > all the time. For example, compatible strings like "nvidia,tegra124", > "ti,omap5" describe a package of an ARM core and various devices. It feels to me rather wrong if I'd specify the compatible string for an external piece of hardware based on the SoC it's connected to. > But by using of_machine_is_compatible in the display drivers, you > can use the right compatible flags from the start if you want to go > that way. Yes, with that we could have right compatible flags in the .dts. >> Well, I sure hope nobody else has to implement similar thing. > > Suuuure.. I've heard that about 20 times before and guess what > happenened? Things never got fixed until some poor maintainer > had to start fixing it all over the place about three years later. The poor maintainer here being me =). And it's still a case of pick-your-poison. We need to hack around the issue, in some form or another. So in any case the poor maintainer has to fix it at some point in the future. But all this is fixed when we have a common display framework. And that's something that is really badly needed in some form or another, the sooner the better. So the "fix" here is not just some internal thing that could be left at that and forgotten. >> To open that up a bit, traditionally other display driver architectures >> have not had drivers for each display "entity", like the panels. So you >> would really just have the display subsystem in the .dts, and some raw >> data there (panel timings) to get the panel running. > > Right, I believe that is the way to go, no doubt about it. No, that's exactly _not_ the way to go =). We need proper drivers for display components, instead of trying to avoid that and embed panel data into the display controller's data. > Here's a list of things that bothers me: > > 1. Searching through the dtb from a driver instead of relying on the > driver probe mechanism for the components in question. If the parsing > of the dtb needs to be done it should be done in a Linux generic way > from some framework instead. The reason I haven't made the conversion code available for others to use is that there are no other users at the moment. And I hope there will be no other user until we get the common display framework. If there is, we'll need to move the code to a generic place. > 2. Having to do this all early before we even have any debug console > available. We are trying to move everything to happen later on to avoid > all the issues related to initializing things early. It worked fine for me when doing it as subsys_init level. Why do you say it'd be early, before debug console? All the drivers with converted compatible strings are normal drivers, so afaik things work fine if we just convert the strings before the module init level. > 3. Having to maintain a database in kernel of display mappings separately > in addition to the .dts files. Yes, that's slightly annoying. Not really a big burden, though, as it's quite rare to get a new encoder or panel driver. > 4. Having this hack limited to device tree based booting while we are > trying to unify the functions for drivers to use to get their > resources. I don't understand this one. With non-DT boot we don't have the issue at all, there's no need for hacks. > 5. Seeing the possibility of this spreading to other drivers. Well, until we have a common display framework, one of the hacky options we've discussed will spread to other drivers if they need to have their own drivers for the same hardware devices. Which is not nice, but not something we can escape. And using the of_machine_is_compatible or having the compatible strings in .dts appended with "dss" or such doesn't change that, it just changes which hack may spread. Tomi
On 19/05/14 19:04, Tony Lindgren wrote: > In many cases however we do have multiple compatible strings that > describe how the device is wired. See drivers/tty/serial/of_serial.c > for example. It has "ns16550" but then it also has additional > "nvidia,tegra20-uart", "nxp,lpc3220-uart" and "ibm,qpace-nwp-serial". All those sound like SoC components. In that case it sounds fine to have the device compatible contain the SoC name. We're talking here about external, detachable devices. >>> Not use what you're after with the SPI example though, but sounds >>> like that's something different. >> >> I think Sebastien's example is just like the issue here. > > Hmm is there some existing example in the kernel like that? No, Sebastien's example was just a hypothetical case. Here, using your way of having SoC specific data in the .dts, we would have "sharp,ls037v7dw01-omap-dss", and in Sebastien's example with a touch sensor we'd have, say, "synaptics,xyz123-omap-spi". Tomi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140519 11:58]: > On 19/05/14 18:57, Tony Lindgren wrote: > > >> I'm not sure if it's even possible to load both of those drivers at the > >> same time, but if it was, and the first one would get probe() called for > >> a device, and the driver would return an error, I'm quite sure the > >> driver framework won't continue trying with the other driver, as the > >> first driver already matched for the device. > > > > No need to load multiple drivers at this point. That's why I'm saying > > you can bail out on the undesired display controller probes using > > of_machine_is_compatible test. It's not that uncommon for drivers to do: > > Hmm, I don't follow. If both, say, omap and exynos have a panel driver > for the same sharp panel, both need a driver for it (presuming exynos > has a proper display component driver system, which may not be true). > > Both drivers would be loaded at boot time for the probe to be even to be > possible. Even if the other one bails out from a device probe using > of_machine_is_compatible, afaik the driver framework will stop probing > at that point, as that driver reports being compatible with the device > in question. > > ... > > Ah, ok, now I see. I think you mean module init, not probe? For some > reason I was just thinking about the probe stage. > > Maybe that would work. I need to test tomorrow. Ah right probe is too late as the match has been made already by then :) > >>> it would be quite silly for each display controller to have to do > >>> their own remapping for shared panels? > >> > >> It would, but wouldn't it be just as silly (well, even more silly in my > >> opinion) to have each display controller require driver specific > >> compatible strings for the panels used? > > > > Not driver specific, but hardware package specific. That's being done > > all the time. For example, compatible strings like "nvidia,tegra124", > > "ti,omap5" describe a package of an ARM core and various devices. > > It feels to me rather wrong if I'd specify the compatible string for an > external piece of hardware based on the SoC it's connected to. > > > But by using of_machine_is_compatible in the display drivers, you > > can use the right compatible flags from the start if you want to go > > that way. > > Yes, with that we could have right compatible flags in the .dts. > > >> Well, I sure hope nobody else has to implement similar thing. > > > > Suuuure.. I've heard that about 20 times before and guess what > > happenened? Things never got fixed until some poor maintainer > > had to start fixing it all over the place about three years later. > > The poor maintainer here being me =). And it's still a case of > pick-your-poison. We need to hack around the issue, in some form or > another. So in any case the poor maintainer has to fix it at some point > in the future. Yeah I agree none of the options so far have been very nice. > But all this is fixed when we have a common display framework. And > that's something that is really badly needed in some form or another, > the sooner the better. > > So the "fix" here is not just some internal thing that could be left at > that and forgotten. Yes.. > >> To open that up a bit, traditionally other display driver architectures > >> have not had drivers for each display "entity", like the panels. So you > >> would really just have the display subsystem in the .dts, and some raw > >> data there (panel timings) to get the panel running. > > > > Right, I believe that is the way to go, no doubt about it. > > No, that's exactly _not_ the way to go =). We need proper drivers for > display components, instead of trying to avoid that and embed panel data > into the display controller's data. Right, sorry I misread it :) > > Here's a list of things that bothers me: > > > > 1. Searching through the dtb from a driver instead of relying on the > > driver probe mechanism for the components in question. If the parsing > > of the dtb needs to be done it should be done in a Linux generic way > > from some framework instead. > > The reason I haven't made the conversion code available for others to > use is that there are no other users at the moment. And I hope there > will be no other user until we get the common display framework. If > there is, we'll need to move the code to a generic place. > > > 2. Having to do this all early before we even have any debug console > > available. We are trying to move everything to happen later on to avoid > > all the issues related to initializing things early. > > It worked fine for me when doing it as subsys_init level. Why do you say > it'd be early, before debug console? The debug console should not be needed for debugging drivers.. Just the serial console. This is because it's hard for users to produce decent error information when the system does not boot if DEBUG_LL and earlyprintk need to be enabled. > All the drivers with converted compatible strings are normal drivers, so > afaik things work fine if we just convert the strings before the module > init level. Yes that seems like the right place to do the workarounds. > > 3. Having to maintain a database in kernel of display mappings separately > > in addition to the .dts files. > > Yes, that's slightly annoying. Not really a big burden, though, as it's > quite rare to get a new encoder or panel driver. > > > 4. Having this hack limited to device tree based booting while we are > > trying to unify the functions for drivers to use to get their > > resources. > > I don't understand this one. With non-DT boot we don't have the issue at > all, there's no need for hacks. Hmm can't we still have multiarch booting happening with the same panel name being used by two different display drivers? > > 5. Seeing the possibility of this spreading to other drivers. > > Well, until we have a common display framework, one of the hacky options > we've discussed will spread to other drivers if they need to have their > own drivers for the same hardware devices. > > Which is not nice, but not something we can escape. And using the > of_machine_is_compatible or having the compatible strings in .dts > appended with "dss" or such doesn't change that, it just changes which > hack may spread. Yes I agree they are all hacks, but your version seems to carry some extra early init baggage with it as I noted above :) Regards, Tony -- 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
Hi, On Mon, May 19, 2014 at 09:57:18PM +0300, Tomi Valkeinen wrote: > > 3. Having to maintain a database in kernel of display mappings separately > > in addition to the .dts files. > > Yes, that's slightly annoying. Not really a big burden, though, as it's > quite rare to get a new encoder or panel driver. I wonder if the "omapdss," prefix could be added to *all* remote endpoints seen by omapdss. Pseudocode: for each port in dss { for each endpoint in port { panel = get_phandle(endpoint, "remote-endpoint"); /* only add prefix if it's not already there (kexec) */ if (panel.compatible != "omapdss,.*") panel.compatible = "omapdss," + panel.compatible; } } -- Sebastian
On Mon, May 19, 2014 at 10:05:15PM +0300, Tomi Valkeinen wrote: > On 19/05/14 19:04, Tony Lindgren wrote: > > > In many cases however we do have multiple compatible strings that > > describe how the device is wired. See drivers/tty/serial/of_serial.c > > for example. It has "ns16550" but then it also has additional > > "nvidia,tegra20-uart", "nxp,lpc3220-uart" and "ibm,qpace-nwp-serial". > > All those sound like SoC components. In that case it sounds fine to have > the device compatible contain the SoC name. We're talking here about > external, detachable devices. > > >>> Not use what you're after with the SPI example though, but sounds > >>> like that's something different. > >> > >> I think Sebastien's example is just like the issue here. > > > > Hmm is there some existing example in the kernel like that? > > No, Sebastien's example was just a hypothetical case. Here, using your > way of having SoC specific data in the .dts, we would have > "sharp,ls037v7dw01-omap-dss", and in Sebastien's example with a touch > sensor we'd have, say, "synaptics,xyz123-omap-spi". Yes, that's what I wanted to say :) My name is spelled Sebastian, though. Sebastien is the French variant as far as i know. -- Sebastian
On 20/05/14 07:57, Sebastian Reichel wrote: > Hi, > > On Mon, May 19, 2014 at 09:57:18PM +0300, Tomi Valkeinen wrote: >>> 3. Having to maintain a database in kernel of display mappings separately >>> in addition to the .dts files. >> >> Yes, that's slightly annoying. Not really a big burden, though, as it's >> quite rare to get a new encoder or panel driver. > > I wonder if the "omapdss," prefix could be added to *all* remote > endpoints seen by omapdss. Pseudocode: > > for each port in dss { > for each endpoint in port { > panel = get_phandle(endpoint, "remote-endpoint"); > > /* only add prefix if it's not already there (kexec) */ > if (panel.compatible != "omapdss,.*") > panel.compatible = "omapdss," + panel.compatible; > } > } Hmm, yes, I think that would work. It would remove the need for the database, but the code would be slightly more complex as we'd need to travel through the graph, not just the nodes that are directly connected to omapdss. I need to try this approach also. Tomi Ps. Sorry for misspelling your name, twice. If I recall right, I even checked if your name is written with an 'a' or an 'e', but somehow still managed to get it wrong.
* Sebastian Reichel <sre@kernel.org> [140519 22:13]: > On Mon, May 19, 2014 at 10:05:15PM +0300, Tomi Valkeinen wrote: > > On 19/05/14 19:04, Tony Lindgren wrote: > > > > > In many cases however we do have multiple compatible strings that > > > describe how the device is wired. See drivers/tty/serial/of_serial.c > > > for example. It has "ns16550" but then it also has additional > > > "nvidia,tegra20-uart", "nxp,lpc3220-uart" and "ibm,qpace-nwp-serial". > > > > All those sound like SoC components. In that case it sounds fine to have > > the device compatible contain the SoC name. We're talking here about > > external, detachable devices. Yes I agree it's also hackish. But playing with the compatible flags avoids adding other custom workarounds. > > >>> Not use what you're after with the SPI example though, but sounds > > >>> like that's something different. > > >> > > >> I think Sebastien's example is just like the issue here. > > > > > > Hmm is there some existing example in the kernel like that? > > > > No, Sebastien's example was just a hypothetical case. Here, using your > > way of having SoC specific data in the .dts, we would have > > "sharp,ls037v7dw01-omap-dss", and in Sebastien's example with a touch > > sensor we'd have, say, "synaptics,xyz123-omap-spi". > > Yes, that's what I wanted to say :) Hmm I think we need to add few things to the hypothetical SPI case to get it even close to the panel example. Like assume that no SPI bus exists, and that potentially multiple different Synaptics drivers are trying to share the same compatible flag for a non-existing generic binding.. I think then it's getting close to the panel example :) Regards, Tony -- 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 Mon, May 19, 2014 at 10:48:52PM -0700, Tony Lindgren wrote: > * Sebastian Reichel <sre@kernel.org> [140519 22:13]: > > On Mon, May 19, 2014 at 10:05:15PM +0300, Tomi Valkeinen wrote: > > > On 19/05/14 19:04, Tony Lindgren wrote: > > > > > > > In many cases however we do have multiple compatible strings that > > > > describe how the device is wired. See drivers/tty/serial/of_serial.c > > > > for example. It has "ns16550" but then it also has additional > > > > "nvidia,tegra20-uart", "nxp,lpc3220-uart" and "ibm,qpace-nwp-serial". > > > > > > All those sound like SoC components. In that case it sounds fine to have > > > the device compatible contain the SoC name. We're talking here about > > > external, detachable devices. > > Yes I agree it's also hackish. But playing with the compatible flags > avoids adding other custom workarounds. > > > > >>> Not use what you're after with the SPI example though, but sounds > > > >>> like that's something different. > > > >> > > > >> I think Sebastien's example is just like the issue here. > > > > > > > > Hmm is there some existing example in the kernel like that? > > > > > > No, Sebastien's example was just a hypothetical case. Here, using your > > > way of having SoC specific data in the .dts, we would have > > > "sharp,ls037v7dw01-omap-dss", and in Sebastien's example with a touch > > > sensor we'd have, say, "synaptics,xyz123-omap-spi". > > > > Yes, that's what I wanted to say :) > > Hmm I think we need to add few things to the hypothetical SPI case > to get it even close to the panel example. Like assume that no SPI bus > exists, [...] The important thing is not, that there is an SPI bus (hardware), but that there is a common SPI client framework (software). That's one of the problem's I see with the modified compatible value. Basically I see the following problem with the appended -omapdss in the compatible string: * You can read the -omapdss as "to be used with omapdss driver". In this case the compatible string is just needed for driver loading, which is frowned upon DT binding maintainers AFAIK. * Alternatively one can read it as "connected to omapdss". In this case we add unneeded redundancy to the DT. This does not belong into a compatible string, since its encoded in the DT hierarchy. And btw. Tomi's rewrite hack basically works exactly like this, just without adding the redundancy explicitly into the DT. Instead of prefixing the comaptible string he could also add a -omapdss sufix. That's basically a proof, that the suffix adds useless redudancy. > [...] and that potentially multiple different Synaptics drivers are > trying to share the same compatible flag for a non-existing generic > binding. If there was no common SPI client framework you would need two different SPI client drivers for two different SPI host controllers. Obviously its a bad idea to have no SPI framework, but that's not important for the example (except that there should obviously be a common panel framework) :) > I think then it's getting close to the panel example :) -- Sebastian
On 19/05/14 22:51, Tony Lindgren wrote: >>> 4. Having this hack limited to device tree based booting while we are >>> trying to unify the functions for drivers to use to get their >>> resources. >> >> I don't understand this one. With non-DT boot we don't have the issue at >> all, there's no need for hacks. > > Hmm can't we still have multiarch booting happening with the same > panel name being used by two different display drivers? Yes, but in that case the driver names are internal to kernel. You can append "omapdss" or such to the driver name, and have that name used in the board file and in the driver. It's not visible outside the kernel, and when there's a common display framework, it can be changed without anyone knowing. With DT we have the old, stable .dts files that need to be supported... >>> 5. Seeing the possibility of this spreading to other drivers. >> >> Well, until we have a common display framework, one of the hacky options >> we've discussed will spread to other drivers if they need to have their >> own drivers for the same hardware devices. >> >> Which is not nice, but not something we can escape. And using the >> of_machine_is_compatible or having the compatible strings in .dts >> appended with "dss" or such doesn't change that, it just changes which >> hack may spread. > > Yes I agree they are all hacks, but your version seems to carry some > extra early init baggage with it as I noted above :) True, but it doesn't feel very big baggage to me. We can bail out immediately if the machine is not omap, so for non-omap's the effect should be negligible. For omap there is some extra stuff being done. I don't really know heavy it is, performance wise, but the operation itself is quite small. I'll try and see how the other options work, which are: - Bailing out from module_init in each display driver. The reason I don't like this (although I haven't tried it) is that all the display drivers need the modification, and because I need to catch the module_init, I cannot use the helpers like module_platform_driver(), so it adds multiple lines to every driver. - Traveling the video graph, starting from omapdss. This one is possibly better performance-wise than my original version, as we only need to search for the omapdss node and can then follow the links. But the code will be more complex. Tomi
Hi, On Wed, May 21, 2014 at 04:05:56PM +0300, Tomi Valkeinen wrote: > I'll try and see how the other options work, which are: > > - Bailing out from module_init in each display driver. The reason I > don't like this (although I haven't tried it) is that all the display > drivers need the modification, and because I need to catch the > module_init, I cannot use the helpers like module_platform_driver(), so > it adds multiple lines to every driver. You could create your own omapdss_driver() define for that. This can be replaced more simply once no longer needed and creates less bloat. I see one more disadvantage for this approach: assumed situation (some future 3.16+x kernel): * The kernel has an common display framework (CDF) * The kernel has an panel driver using CDF * There also exists omapdss driver for the same panel * omapdss does not (yet) adopt CDF * CDF and omapdss are enabled in .config * Kernel is booted from an OMAP system This would result in both drivers being loaded using the same DT compatible string. The same scenario works using the rewriting method, since the common display framework would only see the rewritten compatible string. Of course this scenario only happens when omapdss is not adopted to the common display framework from the start on. > - Traveling the video graph, starting from omapdss. This one is possibly > better performance-wise than my original version, as we only need to > search for the omapdss node and can then follow the links. But the code > will be more complex. I think the main advantage is the missing lookup table/whitelist, which adds redundancy. Not having it has some advantages: * new drivers don't need to touch any existing files (making rebasing etc. easier) * table can get out of sync (though that shouldn't happen anyway, since the binding is supposed to be stable) -- Sebastian
--- /dev/null +++ b/Documentation/devicetree/bindings/panel/sharp,ls037v7dw01.txt @@ -0,0 +1,56 @@ +SHARP LS037V7DW01 TFT-LCD panel + +Required properties: +- compatible: should be "sharp,ls037v7dw01" + +Optional properties: +- enable-gpios: a GPIO spec for the optional enable pin + this pin is the INI pin as specified in the LS037V7DW01.pdf file. +- reset-gpios: a GPIO spec for the optional reset pin + this pin is the RESB pin as specified in the LS037V7DW01.pdf file. +- mode-gpios: a GPIO + ordered MO, LR, and UD as specified in the LS037V7DW01.pdf file. + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. + +This panel can have zero to five GPIOs to configure +to change configuration between QVGA and VGA mode +and the scan direction. As these pins can be also +configured with external pulls, all the GPIOs are +considered optional with holes in the array. + +Example when connected to a omap2+ based device: + + lcd0: display { + compatible = "sharp,ls037v7dw01"; + power-supply = <&lcd_3v3>; + enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>; /* gpio152, lcd INI */ + reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>; /* gpio155, lcd RESB */ + mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH /* gpio154, lcd MO */ + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */ + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */ + + panel-timing { + clock-frequency = <19200000>; + hback-porch = <28>; + hactive = <480>; + hfront-porch = <1>; + hsync-len = <2>; + vback-porch = <1>; + vactive = <640>; + vfront-porch = <1>; + vsync-len = <1>; + hsync-active = <0>; + vsync-active = <0>; + de-active = <1>; + pixelclk-active = <1>; + }; + + port { + lcd_in: endpoint { + remote-endpoint = <&dpi_out>; + }; + }; + }; + --- a/drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c +++ b/drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c @@ -12,15 +12,19 @@ #include <linux/delay.h> #include <linux/gpio.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_gpio.h> #include <linux/platform_device.h> #include <linux/slab.h> - +#include <linux/regulator/consumer.h> +#include <video/of_display_timing.h> #include <video/omapdss.h> #include <video/omap-panel-data.h> struct panel_drv_data { struct omap_dss_device dssdev; struct omap_dss_device *in; + struct regulator *vcc; int data_lines; @@ -95,7 +99,8 @@ static int sharp_ls_enable(struct omap_dss_device *dssdev) if (omapdss_device_is_enabled(dssdev)) return 0; - in->ops.dpi->set_data_lines(in, ddata->data_lines); + if (ddata->data_lines) + in->ops.dpi->set_data_lines(in, ddata->data_lines); in->ops.dpi->set_timings(in, &ddata->videomode); r = in->ops.dpi->enable(in); @@ -240,6 +245,88 @@ static int sharp_ls_probe_pdata(struct platform_device *pdev) if (PTR_ERR(ddata->ini_gpio) == -EPROBE_DEFER) return -EPROBE_DEFER; + ddata->videomode = sharp_ls_timings; + + return 0; +} + +static struct gpio_desc * +sharp_ls_get_gpio_of(struct device *dev, int index, int val, char *desc) +{ + struct gpio_desc *gpio; + + gpio = devm_gpiod_get_index(dev, desc, index); + if (IS_ERR(gpio)) + return gpio; + + gpiod_direction_output(gpio, val); + + return gpio; +} + +static int sharp_ls_probe_of(struct platform_device *pdev) +{ + struct panel_drv_data *ddata = platform_get_drvdata(pdev); + struct device_node *node = pdev->dev.of_node; + struct omap_dss_device *in; + struct display_timing timing; + struct videomode vm; + int r; + + ddata->vcc = devm_regulator_get(&pdev->dev, "envdd"); + if (IS_ERR(ddata->vcc)) { + dev_err(&pdev->dev, "failed to get regulator\n"); + return PTR_ERR(ddata->vcc); + } + + r = regulator_enable(ddata->vcc); + if (r != 0) { + dev_err(&pdev->dev, "failed to enable regulator\n"); + return r; + } + + /* lcd INI */ + ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 3, 0, "enable"); + if (PTR_ERR(ddata->ini_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + /* lcd RESB */ + ddata->resb_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "reset"); + if (PTR_ERR(ddata->resb_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + /* lcd MO */ + ddata->mo_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "mode"); + if (PTR_ERR(ddata->mo_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + /* lcd LR */ + ddata->lr_gpio = sharp_ls_get_gpio_of(&pdev->dev, 1, 1, "mode"); + if (PTR_ERR(ddata->lr_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + /* lcd UD */ + ddata->ud_gpio = sharp_ls_get_gpio_of(&pdev->dev, 2, 1, "mode"); + if (PTR_ERR(ddata->ud_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + r = of_get_display_timing(node, "panel-timing", &timing); + if (r) { + dev_err(&pdev->dev, "failed to get video timing\n"); + return r; + } + + videomode_from_timing(&timing, &vm); + videomode_to_omap_video_timings(&vm, &ddata->videomode); + + in = omapdss_of_find_source_for_first_ep(node); + if (IS_ERR(in)) { + dev_err(&pdev->dev, "failed to find video source\n"); + return PTR_ERR(in); + } + + ddata->in = in; + return 0; } @@ -259,12 +346,14 @@ static int sharp_ls_probe(struct platform_device *pdev) r = sharp_ls_probe_pdata(pdev); if (r) return r; + } else if (pdev->dev.of_node) { + r = sharp_ls_probe_of(pdev); + if (r) + return r; } else { return -ENODEV; } - ddata->videomode = sharp_ls_timings; - dssdev = &ddata->dssdev; dssdev->dev = &pdev->dev; dssdev->driver = &sharp_ls_ops; @@ -302,12 +391,20 @@ static int __exit sharp_ls_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id sharp_ls_of_match[] = { + { .compatible = "sharp,ls037v7dw01", }, + {}, +}; + +MODULE_DEVICE_TABLE(of, sharp_ls_of_match); + static struct platform_driver sharp_ls_driver = { .probe = sharp_ls_probe, .remove = __exit_p(sharp_ls_remove), .driver = { .name = "panel-sharp-ls037v7dw01", .owner = THIS_MODULE, + .of_match_table = sharp_ls_of_match, }, };