[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
On Mon, Mar 28, 2016 at 07:00:00PM -0700, Stefan Agner wrote: > Add driver for the TCON (timing controller) module. The TCON module > is a separate module attached after the DCU (display controller > unit). Each DCU instance has its own, directly connected TCON > instance. The DCU's RGB and timing signals are passing through > the TCON module. TCON can provide timing signals for raw TFT panels > or operate in a bypass mode which leaves all signals unaltered. > > The driver currently only supports the bypass mode. > > Signed-off-by: Stefan Agner > --- > .../devicetree/bindings/display/fsl,dcu.txt| 2 + > .../devicetree/bindings/display/fsl,tcon.txt | 18 > drivers/gpu/drm/fsl-dcu/Makefile | 3 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 + > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 1 + > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 11 +++ > drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 108 > + > drivers/gpu/drm/fsl-dcu/fsl_tcon.h | 33 +++ > 8 files changed, 178 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/display/fsl,tcon.txt > create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_tcon.c > create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_tcon.h > > diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt > b/Documentation/devicetree/bindings/display/fsl,dcu.txt > index f299e1e..b19bf12 100644 > --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt > +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt > @@ -14,6 +14,7 @@ Required properties: > Optional properties: > - clocks:Second handle for pixel clock. > - clock-names: Second name "pix" for pixel clock. > +- fsl,tcon: The phandle to the timing controller node. > > Examples: > dcu: dcu at 2ce { > @@ -23,4 +24,5 @@ dcu: dcu at 2ce { > clock-names = "dcu"; > big-endian; > fsl,panel = <>; > + fsl,tcon = <>; > }; > diff --git a/Documentation/devicetree/bindings/display/fsl,tcon.txt > b/Documentation/devicetree/bindings/display/fsl,tcon.txt > new file mode 100644 > index 000..2e1015e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/fsl,tcon.txt > @@ -0,0 +1,18 @@ > +Device Tree bindings for Freescale TCON Driver > + > +Required properties: > +- compatible:Should be one of > + * "fsl,vf610-tcon". > + > +- reg: Address and length of the register set for dcu. s/dcu/tcon/ With that, Acked-by: Rob Herring > +- clocks:From common clock binding: handle to tcon ipg clock. > +- clock-names: From common clock binding: Shall be "ipg". > + > +Examples: > +timing-controller at 4003d000 { > + compatible = "fsl,vf610-tcon"; > + reg = <0x4003d000 0x1000>; > + clocks = < VF610_CLK_TCON0>; > + clock-names = "ipg"; > + status = "okay"; > +};
[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
On Tuesday 29 March 2016 00:11:13, Stefan Agner wrote: > >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt > >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt > >> > >> @@ -14,6 +14,7 @@ Required properties: > >> Optional properties: > >> - clocks: Second handle for pixel clock. > >> - clock-names:Second name "pix" for pixel clock. > >> > >> +- fsl,tcon: The phandle to the timing controller node. > > > > Maybe a comment this is (currently) only for Vybrid, but not LS1021A. > > IMHO, such references to SoCs in a peripheral binding is somewhat > misplaced. If we would start doing this, we would end up in lots of SoC > references, and nobody knows whether they are actually accurate and up > to date... And I think we should add tcon to LS1021a, more on that > below... Well, I don't mind after all. > >> --- /dev/null > >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c > >> [...] > >> +struct fsl_tcon *fsl_tcon_init(struct device *dev) > >> +{ > >> + struct fsl_tcon *tcon; > >> + struct device_node *np; > >> + int ret; > >> + > >> + tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); > >> + if (!tcon) > >> + return NULL; > >> + > >> + np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); > >> + if (!np) { > >> + dev_warn(dev, "Couldn't find the tcon node\n"); > >> + return NULL; > >> + } > > > > Maybe check for device tree node before allocating struct fsl_tcon? Also > > this should not be a warning, as on LS1021A this is to be expected. > > Agreed, definitely the smarter sequence. > > Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches > it was part of the driver, see: > https://lkml.org/lkml/2015/7/13/242 > > Not sure how that driver can work without TCON support on LS1021a, maybe > the bootloader sets the TCON to bypass? > > Since I do not have a LS1021a, I hesitated to just add it to the LS1021s > dt's, but it should be fairly straight forward. Interesting, but in Patch 3 (https://lists.freedesktop.org/archives/dri-devel/2015-July/086381.html) no TCON node is added and using is still optional. Yet, I don't find any TCON hardware in the LS1021A reference manual, e.g. in the memory map. I hard guess is that it doesn't require/use one. Without memory address it would be pretty hard to add one. Alexander
[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
Hi, some comments below. On Monday 28 March 2016 19:00:00, Stefan Agner wrote: > Add driver for the TCON (timing controller) module. The TCON module > is a separate module attached after the DCU (display controller > unit). Each DCU instance has its own, directly connected TCON > instance. The DCU's RGB and timing signals are passing through > the TCON module. TCON can provide timing signals for raw TFT panels > or operate in a bypass mode which leaves all signals unaltered. > > The driver currently only supports the bypass mode. > > Signed-off-by: Stefan Agner > --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt > +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt > @@ -14,6 +14,7 @@ Required properties: > Optional properties: > - clocks:Second handle for pixel clock. > - clock-names: Second name "pix" for pixel clock. > +- fsl,tcon: The phandle to the timing controller node. Maybe a comment this is (currently) only for Vybrid, but not LS1021A. > --- /dev/null > +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c > [...] > +struct fsl_tcon *fsl_tcon_init(struct device *dev) > +{ > + struct fsl_tcon *tcon; > + struct device_node *np; > + int ret; > + > + tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); > + if (!tcon) > + return NULL; > + > + np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); > + if (!np) { > + dev_warn(dev, "Couldn't find the tcon node\n"); > + return NULL; > + } Maybe check for device tree node before allocating struct fsl_tcon? Also this should not be a warning, as on LS1021A this is to be expected. Best regards, Alexander
[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
On 2016-03-29 00:26, Alexander Stein wrote: > On Tuesday 29 March 2016 00:11:13, Stefan Agner wrote: >> >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt >> >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt >> >> >> >> @@ -14,6 +14,7 @@ Required properties: >> >> Optional properties: >> >> - clocks:Second handle for pixel clock. >> >> - clock-names: Second name "pix" for pixel clock. >> >> >> >> +- fsl,tcon: The phandle to the timing controller node. >> > >> > Maybe a comment this is (currently) only for Vybrid, but not LS1021A. >> >> IMHO, such references to SoCs in a peripheral binding is somewhat >> misplaced. If we would start doing this, we would end up in lots of SoC >> references, and nobody knows whether they are actually accurate and up >> to date... And I think we should add tcon to LS1021a, more on that >> below... > > Well, I don't mind after all. > >> >> --- /dev/null >> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c >> >> [...] >> >> +struct fsl_tcon *fsl_tcon_init(struct device *dev) >> >> +{ >> >> + struct fsl_tcon *tcon; >> >> + struct device_node *np; >> >> + int ret; >> >> + >> >> + tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); >> >> + if (!tcon) >> >> + return NULL; >> >> + >> >> + np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); >> >> + if (!np) { >> >> + dev_warn(dev, "Couldn't find the tcon node\n"); >> >> + return NULL; >> >> + } >> > >> > Maybe check for device tree node before allocating struct fsl_tcon? Also >> > this should not be a warning, as on LS1021A this is to be expected. >> >> Agreed, definitely the smarter sequence. >> >> Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches >> it was part of the driver, see: >> https://lkml.org/lkml/2015/7/13/242 >> >> Not sure how that driver can work without TCON support on LS1021a, maybe >> the bootloader sets the TCON to bypass? >> >> Since I do not have a LS1021a, I hesitated to just add it to the LS1021s >> dt's, but it should be fairly straight forward. > > Interesting, but in Patch 3 > (https://lists.freedesktop.org/archives/dri-devel/2015-July/086381.html) > no TCON node is added and using is still > optional. Yet, I don't find any TCON hardware in the LS1021A reference > manual, > e.g. in the memory map. I hard guess is that it doesn't require/use one. > Without memory address it would be pretty hard to add one. Ok, maybe I am completely wrong on that. I thought I have seen it... It could be that Jianwei added TCON support for Vybrid only, in this case I agree there shouldn't be a warning if TCON is missing. Maybe even the other way around, write a info message in case TCON has been found. -- Stefan
[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
On 2016-03-28 23:45, Alexander Stein wrote: > Hi, > > some comments below. > > On Monday 28 March 2016 19:00:00, Stefan Agner wrote: >> Add driver for the TCON (timing controller) module. The TCON module >> is a separate module attached after the DCU (display controller >> unit). Each DCU instance has its own, directly connected TCON >> instance. The DCU's RGB and timing signals are passing through >> the TCON module. TCON can provide timing signals for raw TFT panels >> or operate in a bypass mode which leaves all signals unaltered. >> >> The driver currently only supports the bypass mode. >> >> Signed-off-by: Stefan Agner > >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt >> @@ -14,6 +14,7 @@ Required properties: >> Optional properties: >> - clocks: Second handle for pixel clock. >> - clock-names: Second name "pix" for pixel clock. >> +- fsl,tcon: The phandle to the timing controller node. > > Maybe a comment this is (currently) only for Vybrid, but not LS1021A. > IMHO, such references to SoCs in a peripheral binding is somewhat misplaced. If we would start doing this, we would end up in lots of SoC references, and nobody knows whether they are actually accurate and up to date... And I think we should add tcon to LS1021a, more on that below... >> --- /dev/null >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c >> [...] >> +struct fsl_tcon *fsl_tcon_init(struct device *dev) >> +{ >> +struct fsl_tcon *tcon; >> +struct device_node *np; >> +int ret; >> + >> +tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL); >> +if (!tcon) >> +return NULL; >> + >> +np = of_parse_phandle(dev->of_node, "fsl,tcon", 0); >> +if (!np) { >> +dev_warn(dev, "Couldn't find the tcon node\n"); >> +return NULL; >> +} > > Maybe check for device tree node before allocating struct fsl_tcon? Also this > should not be a warning, as on LS1021A this is to be expected. Agreed, definitely the smarter sequence. Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches it was part of the driver, see: https://lkml.org/lkml/2015/7/13/242 Not sure how that driver can work without TCON support on LS1021a, maybe the bootloader sets the TCON to bypass? Since I do not have a LS1021a, I hesitated to just add it to the LS1021s dt's, but it should be fairly straight forward. -- Stefan
[PATCH v2 6/8] drm/fsl-dcu: add TCON driver
Add driver for the TCON (timing controller) module. The TCON module is a separate module attached after the DCU (display controller unit). Each DCU instance has its own, directly connected TCON instance. The DCU's RGB and timing signals are passing through the TCON module. TCON can provide timing signals for raw TFT panels or operate in a bypass mode which leaves all signals unaltered. The driver currently only supports the bypass mode. Signed-off-by: Stefan Agner --- .../devicetree/bindings/display/fsl,dcu.txt| 2 + .../devicetree/bindings/display/fsl,tcon.txt | 18 drivers/gpu/drm/fsl-dcu/Makefile | 3 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 1 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 11 +++ drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 108 + drivers/gpu/drm/fsl-dcu/fsl_tcon.h | 33 +++ 8 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,tcon.txt create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_tcon.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_tcon.h diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt b/Documentation/devicetree/bindings/display/fsl,dcu.txt index f299e1e..b19bf12 100644 --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt @@ -14,6 +14,7 @@ Required properties: Optional properties: - clocks: Second handle for pixel clock. - clock-names: Second name "pix" for pixel clock. +- fsl,tcon:The phandle to the timing controller node. Examples: dcu: dcu at 2ce { @@ -23,4 +24,5 @@ dcu: dcu at 2ce { clock-names = "dcu"; big-endian; fsl,panel = <>; + fsl,tcon = <>; }; diff --git a/Documentation/devicetree/bindings/display/fsl,tcon.txt b/Documentation/devicetree/bindings/display/fsl,tcon.txt new file mode 100644 index 000..2e1015e --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,tcon.txt @@ -0,0 +1,18 @@ +Device Tree bindings for Freescale TCON Driver + +Required properties: +- compatible: Should be one of + * "fsl,vf610-tcon". + +- reg: Address and length of the register set for dcu. +- clocks: From common clock binding: handle to tcon ipg clock. +- clock-names: From common clock binding: Shall be "ipg". + +Examples: +timing-controller at 4003d000 { + compatible = "fsl,vf610-tcon"; + reg = <0x4003d000 0x1000>; + clocks = < VF610_CLK_TCON0>; + clock-names = "ipg"; + status = "okay"; +}; diff --git a/drivers/gpu/drm/fsl-dcu/Makefile b/drivers/gpu/drm/fsl-dcu/Makefile index 6ea1523..b35a292 100644 --- a/drivers/gpu/drm/fsl-dcu/Makefile +++ b/drivers/gpu/drm/fsl-dcu/Makefile @@ -3,5 +3,6 @@ fsl-dcu-drm-y := fsl_dcu_drm_drv.o \ fsl_dcu_drm_rgb.o \ fsl_dcu_drm_plane.o \ fsl_dcu_drm_crtc.o \ -fsl_dcu_drm_fbdev.o +fsl_dcu_drm_fbdev.o \ +fsl_tcon.o obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu-drm.o diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 093a60b..f62bff2 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -27,6 +27,7 @@ #include "fsl_dcu_drm_crtc.h" #include "fsl_dcu_drm_drv.h" +#include "fsl_tcon.h" static bool fsl_dcu_drm_is_volatile_reg(struct device *dev, unsigned int reg) { @@ -357,6 +358,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) goto unregister_pix_clk; } + fsl_dev->tcon = fsl_tcon_init(dev); + drm = drm_dev_alloc(driver, dev); if (!drm) { ret = -ENOMEM; diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h index f60ec0a..c275f90 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h @@ -184,6 +184,7 @@ struct fsl_dcu_drm_device { int irq; struct clk *clk; struct clk *pix_clk; + struct fsl_tcon *tcon; /*protects hardware register*/ spinlock_t irq_lock; struct drm_device *drm; diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index 8780deb..f586f1e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c @@ -17,6 +17,7 @@ #include #include "fsl_dcu_drm_drv.h" +#include "fsl_tcon.h" static int fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder, @@ -28,10 +29,20 @@ fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder, static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder) { + struct drm_device *dev =