| Message ID | 1378488201-21146-1-git-send-email-pawel.moll@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 09/06/2013 07:23 PM, Pawel Moll wrote: > This patch adds basic DT bindings for the PL11x CLCD cells > and make their fbdev driver use them. > > Signed-off-by: Pawel Moll<pawel.moll@arm.com> > --- > .../devicetree/bindings/video/arm,pl11x.txt | 87 +++++++++ > drivers/video/Kconfig | 1 + > drivers/video/amba-clcd.c | 214 +++++++++++++++++++++ > 3 files changed, 302 insertions(+) > create mode 100644 Documentation/devicetree/bindings/video/arm,pl11x.txt > > diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt > new file mode 100644 > index 0000000..178aebb > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt > @@ -0,0 +1,87 @@ > +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 nit: Shouldn't the abbreviation be CLCDC ? > +See also Documentation/devicetree/bindings/arm/primecell.txt > + > +Required properties: > + > +- compatible: must be one of: > + "arm,pl110", "arm,primecell" > + "arm,pl111", "arm,primecell" > +- reg: base address and size of the control registers block > +- interrupts: either a single interrupt specifier representing the > + combined interrupt output (CLCDINTR) or an array of > + four interrupt specifiers for CLCDMBEINTR, > + CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR; in the > + latter case interrupt names must be specified > + (see below) > +- interrupt-names: when four interrupts are specified, their names: > + "mbe", "vcomp", "lnbu", "fuf" > + must be specified in order respective to the > + interrupt specifiers > +- clocks: contains phandle and clock specifier pairs for the entries > + in the clock-names property. See > + Documentation/devicetree/binding/clock/clock-bindings.txt > +- clocks names: should contain "clcdclk" and "apb_pclk" > + > +Optional properties: > + > +- video-ram: phandle to a node describing specialized video memory > + (that is *not* described in the top level "memory" node) > + that must be used as a framebuffer, eg. due to restrictions > + of the system interconnect; the node must contain a > + standard reg property describing the address and the size > + of the memory area It seems the "specialized video memory" is described by some vendor specific DT binding ? Is it documented ? It sounds like you are unnecessarily repeating the memory node details here. Perhaps this binding/driver should use the common reserved memory bindings, see thread http://www.spinics.net/lists/devicetree/msg02880.html > +- max-framebuffer-size: maximum size in bytes of the framebuffer the > + system can handle, eg. in terms of available > + memory bandwidth > + > +In the simplest case of a display panel being connected directly to the > +CLCD, it can be described in the node: > + > +- panel-dimensions: (optional) array of two numbers (width and height) > + describing physical dimension in mm of the panel > +- panel-type: (required) must be "tft" or "stn", defines panel type > +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining > + widths in bits of the R, G and B components > +- panel-tft-rb-swapped: (for "tft" panel type) if present means that > + the R& B components are swapped on the board > +- panel-stn-color: (for "stn" panel type) if present means that > + the panel is a colour STN display, if missing > + is a monochrome display > +- panel-stn-dual: (for "stn" panel type) if present means that there > + are two STN displays connected > +- panel-stn-4bit: (for monochrome "stn" panel) if present means > + that the monochrome display is connected > + via 4 bit-wide interface Are this vendor specific or common properties ? Shouldn't those be prefixed with the vendor name ? Anyway I think we need an RFC to possibly standardize properties that are common across multiple panels and have them listed in a common DT binding. It sounds a bit disappointing that for same class devices there are being introduced custom DT properties for each available device. For instance, the first 3 properties above look like they could apply to various display panels and controllers. > +- display-timings: standard display timings sub-node, see > + Documentation/devicetree/bindings/video/display-timing.txt > + > +Example: > + > + clcd@1f0000 { > + compatible = "arm,pl111", "arm,primecell"; > + reg =<0x1f0000 0x1000>; > + interrupts =<14>; > + clocks =<&v2m_oscclk1>,<&smbclk>; > + clock-names = "clcdclk", "apb_pclk"; > + > + video-ram =<&v2m_vram>; > + max-framebuffer-size =<614400>; /* 640x480 16bpp */ > + > + panel-type = "tft"; > + panel-tft-interface =<8 8 8>; > + display-timings { > + native-mode =<&v2m_clcd_timing0>; > + v2m_clcd_timing0: vga { > + clock-frequency =<25175000>; > + hactive =<640>; > + hback-porch =<40>; > + hfront-porch =<24>; > + hsync-len =<96>; > + vactive =<480>; > + vback-porch =<32>; > + vfront-porch =<11>; > + vsync-len =<2>; > + }; > + }; > + }; > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index 4cf1e1d..375bf63 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -316,6 +316,7 @@ config FB_ARMCLCD > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > + select VIDEOMODE_HELPERS if OF > help > This framebuffer device driver is for the ARM PrimeCell PL110 > Colour LCD controller. ARM PrimeCells provide the building > diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c > index 0a2cce7..145ca5a 100644 > --- a/drivers/video/amba-clcd.c > +++ b/drivers/video/amba-clcd.c > @@ -25,6 +25,11 @@ > #include<linux/amba/clcd.h> > #include<linux/clk.h> > #include<linux/hardirq.h> > +#include<linux/dma-mapping.h> > +#include<linux/of.h> > +#include<linux/of_address.h> > +#include<video/of_display_timing.h> > +#include<video/of_videomode.h> > > #include<asm/sizes.h> > > @@ -542,6 +547,212 @@ static int clcdfb_register(struct clcd_fb *fb) > return ret; > } > > +#ifdef CONFIG_OF > +static int clcdfb_of_get_tft_panel(struct device_node *node, > + struct clcd_panel *panel) > +{ > + int err; > + u32 rgb[3]; > + int r, g, b; Couldn't these be 'unsigned int' ? > + err = of_property_read_u32_array(node, "panel-tft-interface", rgb, 3); > + if (err) > + return err; > + > + r = rgb[0]; > + g = rgb[1]; > + b = rgb[2]; > + > + /* Bypass pixel clock divider, data output on the falling edge */ > + panel->tim2 = TIM2_BCD | TIM2_IPC; > + > + /* TFT display, vert. comp. interrupt at the start of the back porch */ > + panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1); > + > + if (r>= 4&& g>= 4&& b>= 4) > + panel->caps |= CLCD_CAP_444; > + if (r>= 5&& g>= 5&& b>= 5) > + panel->caps |= CLCD_CAP_5551; > + if (r>= 5&& g>= 6&& b>= 5) > + panel->caps |= CLCD_CAP_565; > + if (r>= 8&& g>= 8&& b>= 8) > + panel->caps |= CLCD_CAP_888; > + > + if (of_get_property(node, "panel-tft-rb-swapped", NULL)) > + panel->caps&= ~CLCD_CAP_RGB; > + else > + panel->caps&= ~CLCD_CAP_BGR; > + > + return 0; > +} > + > +static int clcdfb_of_init_display(struct clcd_fb *fb) > +{ > + struct device_node *node = fb->dev->dev.of_node; > + u32 max_size; > + u32 dimensions[2]; > + char *mode_name; > + int len, err; > + const char *panel_type; > + > + fb->panel = devm_kzalloc(&fb->dev->dev, sizeof(*fb->panel), GFP_KERNEL); > + if (!fb->panel) > + return -ENOMEM; > + > + err = of_get_fb_videomode(fb->dev->dev.of_node,&fb->panel->mode, 'node' could be reused here. > + OF_USE_NATIVE_MODE); > + if (err) > + return err; > + > + len = snprintf(NULL, 0, "%ux%u@%u", fb->panel->mode.xres, > + fb->panel->mode.yres, fb->panel->mode.refresh); > + mode_name = devm_kzalloc(&fb->dev->dev, len + 1, GFP_KERNEL); > + snprintf(mode_name, len + 1, "%ux%u@%u", fb->panel->mode.xres, > + fb->panel->mode.yres, fb->panel->mode.refresh); Don't you want to just use kasprintf() here instead and free mode_name manually in the remove() callback ? > + fb->panel->mode.name = mode_name; > + > + err = of_property_read_u32(node, "max-framebuffer-size",&max_size); > + if (!err) > + fb->panel->bpp = max_size / (fb->panel->mode.xres * > + fb->panel->mode.yres) * 8; > + else > + fb->panel->bpp = 32; > + > +#ifdef CONFIG_CPU_BIG_ENDIAN > + fb->panel->cntl |= CNTL_BEBO; > +#endif > + > + if (of_property_read_u32_array(node, "panel-dimensions", > + dimensions, 2) == 0) { > + fb->panel->width = dimensions[0]; > + fb->panel->height = dimensions[1]; > + } else { > + fb->panel->width = -1; > + fb->panel->height = -1; > + } > + > + panel_type = of_get_property(node, "panel-type",&len); > + if (strncmp(panel_type, "tft", len) == 0) > + return clcdfb_of_get_tft_panel(node, fb->panel); > + else > + return -EINVAL; > +} > + > +static int clcdfb_of_vram_setup(struct clcd_fb *fb) > +{ > + const __be32 *prop = of_get_property(fb->dev->dev.of_node, "video-ram", > + NULL); > + struct device_node *node = of_find_node_by_phandle(be32_to_cpup(prop)); This looks like open coding function of_parse_phandle(), why not just: struct device_node *node = of_parse_phandle(fb->dev->dev.of_node, "video-ram", 0); ? > + u64 size; > + int err; > + > + if (!node) > + return -ENODEV; > + > + err = clcdfb_of_init_display(fb); > + if (err) > + return err; > + > + fb->fb.screen_base = of_iomap(node, 0); > + if (!fb->fb.screen_base) > + return -ENOMEM; > + > + fb->fb.fix.smem_start = of_translate_address(node, > + of_get_address(node, 0,&size, NULL)); > + fb->fb.fix.smem_len = size; > + > + return 0; > +} -- Thanks, Sylwester
On Sat, 2013-09-07 at 23:55 +0100, Sylwester Nawrocki wrote: > H --- /dev/null > > +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt > > @@ -0,0 +1,87 @@ > > +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 > > nit: Shouldn't the abbreviation be CLCDC ? The commonly used acronym for this cell is CLCD. For what its worth, I can make the line read: ARM PrimeCell Color LCD (CLCD) Controller PL110/PL111 > > +- video-ram: phandle to a node describing specialized video memory > > + (that is *not* described in the top level "memory" node) > > + that must be used as a framebuffer, eg. due to restrictions > > + of the system interconnect; the node must contain a > > + standard reg property describing the address and the size > > + of the memory area > > It seems the "specialized video memory" is described by some vendor specific > DT binding ? Is it documented ? It sounds like you are unnecessarily > repeating the memory node details here. I appreciate this property being the hardest to swallow, but the problem it is trying to solve is quite simple, really. System can be designed in such a way that CLCD is *not* able to read data from the main memory. In such case there will be a separate block of (usually static) memory "next" to CLCD, which *must* be used for the framebuffer. And I've got two choices here: to simply define an address and size, or to define a phandle to a node with standard reg property. I'll be really grateful for ideas how to describe the situation better. > Perhaps this binding/driver should use the common reserved memory bindings, > see thread http://www.spinics.net/lists/devicetree/msg02880.html No, not really - as I said, this is *not* the main RAM of the system. CMA doesn't even know about its existence. > > +- max-framebuffer-size: maximum size in bytes of the framebuffer the > > + system can handle, eg. in terms of available > > + memory bandwidth > > + > > +In the simplest case of a display panel being connected directly to the > > +CLCD, it can be described in the node: > > + > > +- panel-dimensions: (optional) array of two numbers (width and height) > > + describing physical dimension in mm of the panel > > +- panel-type: (required) must be "tft" or "stn", defines panel type > > +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining > > + widths in bits of the R, G and B components > > +- panel-tft-rb-swapped: (for "tft" panel type) if present means that > > + the R& B components are swapped on the board > > +- panel-stn-color: (for "stn" panel type) if present means that > > + the panel is a colour STN display, if missing > > + is a monochrome display > > +- panel-stn-dual: (for "stn" panel type) if present means that there > > + are two STN displays connected > > +- panel-stn-4bit: (for monochrome "stn" panel) if present means > > + that the monochrome display is connected > > + via 4 bit-wide interface > > Are this vendor specific or common properties ? Shouldn't those be prefixed > with the vendor name ? I have no answer to this question. My guts are telling me - nope. TFT is TFT, STN is STN, nothing to do with "arm,". But I welcome other opinions. > Anyway I think we need an RFC to possibly standardize properties that are > common across multiple panels and have them listed in a common DT binding. > > It sounds a bit disappointing that for same class devices there are being > introduced custom DT properties for each available device. For instance, > the first 3 properties above look like they could apply to various display > panels and controllers. No argument from here. The Common Display Framework was supposed to address this issue. And the first version of this patch used it: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/63417 But you'll notice that it's been almost 10 months since the last version of CDF saw the daylight, and Laurent seems to be too busy with other duties to carry on with this. Additionally the KMS/DRM guys don't look too kindly on it. The bottom line is: there are 3 different platform suffering from lack of DT bindings for CLCD and I've got fed up with waiting. Anyway, I've intentionally kept the panel properties in a separate section and wrote "can be described", to keep an open way for future "display bindings", if and when they appear. > > +#ifdef CONFIG_OF > > +static int clcdfb_of_get_tft_panel(struct device_node *node, > > + struct clcd_panel *panel) > > +{ > > + int err; > > + u32 rgb[3]; > > + int r, g, b; > > Couldn't these be 'unsigned int' ? They could indeed. > > + err = of_property_read_u32_array(node, "panel-tft-interface", rgb, 3); > > + if (err) > > + return err; > > + > > + r = rgb[0]; > > + g = rgb[1]; > > + b = rgb[2]; > > + > > + /* Bypass pixel clock divider, data output on the falling edge */ > > + panel->tim2 = TIM2_BCD | TIM2_IPC; > > + > > + /* TFT display, vert. comp. interrupt at the start of the back porch */ > > + panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1); > > + > > + if (r>= 4&& g>= 4&& b>= 4) > > + panel->caps |= CLCD_CAP_444; > > + if (r>= 5&& g>= 5&& b>= 5) > > + panel->caps |= CLCD_CAP_5551; > > + if (r>= 5&& g>= 6&& b>= 5) > > + panel->caps |= CLCD_CAP_565; > > + if (r>= 8&& g>= 8&& b>= 8) > > + panel->caps |= CLCD_CAP_888; > > + > > + if (of_get_property(node, "panel-tft-rb-swapped", NULL)) > > + panel->caps&= ~CLCD_CAP_RGB; > > + else > > + panel->caps&= ~CLCD_CAP_BGR; > > + > > + return 0; > > +} > > + > > +static int clcdfb_of_init_display(struct clcd_fb *fb) > > +{ > > + struct device_node *node = fb->dev->dev.of_node; > > + u32 max_size; > > + u32 dimensions[2]; > > + char *mode_name; > > + int len, err; > > + const char *panel_type; > > + > > + fb->panel = devm_kzalloc(&fb->dev->dev, sizeof(*fb->panel), GFP_KERNEL); > > + if (!fb->panel) > > + return -ENOMEM; > > + > > + err = of_get_fb_videomode(fb->dev->dev.of_node,&fb->panel->mode, > > 'node' could be reused here. Of course, well spotted. > > + OF_USE_NATIVE_MODE); > > + if (err) > > + return err; > > + > > + len = snprintf(NULL, 0, "%ux%u@%u", fb->panel->mode.xres, > > + fb->panel->mode.yres, fb->panel->mode.refresh); > > + mode_name = devm_kzalloc(&fb->dev->dev, len + 1, GFP_KERNEL); > > + snprintf(mode_name, len + 1, "%ux%u@%u", fb->panel->mode.xres, > > + fb->panel->mode.yres, fb->panel->mode.refresh); > > Don't you want to just use kasprintf() here instead and free mode_name > manually in the remove() callback ? > > +static int clcdfb_of_vram_setup(struct clcd_fb *fb) > > +{ > > + const __be32 *prop = of_get_property(fb->dev->dev.of_node, "video-ram", > > + NULL); > > + struct device_node *node = of_find_node_by_phandle(be32_to_cpup(prop)); > > This looks like open coding function of_parse_phandle(), why not just: > > struct device_node *node = of_parse_phandle(fb->dev->dev.of_node, > "video-ram", 0); > ? I'm not sure why did I write it like this (and it's been a while since this happened), but you're right - it should be of_parse_phandle(). Thanks! Pawel
On 09/09/2013 01:19 PM, Pawel Moll wrote: > On Sat, 2013-09-07 at 23:55 +0100, Sylwester Nawrocki wrote: >> H --- /dev/null >>> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt >>> @@ -0,0 +1,87 @@ >>> +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 >> >> nit: Shouldn't the abbreviation be CLCDC ? > > The commonly used acronym for this cell is CLCD. For what its worth, I > can make the line read: > > ARM PrimeCell Color LCD (CLCD) Controller PL110/PL111 OK, that's probably better. A meaningless detail, I've just seen mostly CLCDC used in the PL100/PL111 TRM and LCDC would presumably be more intuitive for this type of IP block. >>> +- video-ram: phandle to a node describing specialized video memory >>> + (that is *not* described in the top level "memory" node) >>> + that must be used as a framebuffer, eg. due to restrictions >>> + of the system interconnect; the node must contain a >>> + standard reg property describing the address and the size >>> + of the memory area >> >> It seems the "specialized video memory" is described by some vendor specific >> DT binding ? Is it documented ? It sounds like you are unnecessarily >> repeating the memory node details here. > > I appreciate this property being the hardest to swallow, but the problem > it is trying to solve is quite simple, really. System can be designed in > such a way that CLCD is *not* able to read data from the main memory. In > such case there will be a separate block of (usually static) memory > "next" to CLCD, which *must* be used for the framebuffer. And I've got > two choices here: to simply define an address and size, or to define a > phandle to a node with standard reg property. I'll be really grateful > for ideas how to describe the situation better. I though about reusing the binding only, the part defining reserved (carve out) memory regions. >> Perhaps this binding/driver should use the common reserved memory bindings, >> see thread http://www.spinics.net/lists/devicetree/msg02880.html > > No, not really - as I said, this is *not* the main RAM of the system. > CMA doesn't even know about its existence. I'm really not an expert in this area, I'll assume we don't list such separate memory chips in the 'memory' node. But if such memory could be used not only by this single IP block it would probably make sense to have it listed in memory/reserved-memory, so it could be used by other devices of which drivers possibly wouldn't have to contain all the detailed dt parsing/memory handling code. >>> +- max-framebuffer-size: maximum size in bytes of the framebuffer the >>> + system can handle, eg. in terms of available >>> + memory bandwidth >>> + >>> +In the simplest case of a display panel being connected directly to the >>> +CLCD, it can be described in the node: >>> + >>> +- panel-dimensions: (optional) array of two numbers (width and height) >>> + describing physical dimension in mm of the panel >>> +- panel-type: (required) must be "tft" or "stn", defines panel type >>> +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining >>> + widths in bits of the R, G and B components >>> +- panel-tft-rb-swapped: (for "tft" panel type) if present means that >>> + the R& B components are swapped on the board >>> +- panel-stn-color: (for "stn" panel type) if present means that >>> + the panel is a colour STN display, if missing >>> + is a monochrome display >>> +- panel-stn-dual: (for "stn" panel type) if present means that there >>> + are two STN displays connected >>> +- panel-stn-4bit: (for monochrome "stn" panel) if present means >>> + that the monochrome display is connected >>> + via 4 bit-wide interface >> >> Are this vendor specific or common properties ? Shouldn't those be prefixed >> with the vendor name ? > > I have no answer to this question. My guts are telling me - nope. TFT is > TFT, STN is STN, nothing to do with "arm,". But I welcome other > opinions. I thought about documenting such a common properties in a common place. It's not immediately clear from names of these properties that they apply to PL110/ PL111 devices only. If we let such generic names being redefined across DT bindings of different devices it is going to be pretty messy IMHO. Same property in two different dts files would potentially have different meaning. So perhaps instead of panel-dimensions we could define common properties, e.g. - display-phys-width: physical horizontal dimension of a display in millimetres (micrometres ?); - display-phys-height: physical vertical dimension of a display in millimetres (micrometres ?); Instead of 'panel-stn-color' a boolean property 'monochrome-display', the default when this property was missing would be "colour display". I'd like to leave defining such common properties to someone having more experience in the display area. I don't think it would take much time come up with generic names for that couple properties you need. Then CDF implementation would simply use whatever gets agreed. >> Anyway I think we need an RFC to possibly standardize properties that are >> common across multiple panels and have them listed in a common DT binding. >> >> It sounds a bit disappointing that for same class devices there are being >> introduced custom DT properties for each available device. For instance, >> the first 3 properties above look like they could apply to various display >> panels and controllers. > > No argument from here. The Common Display Framework was supposed to > address this issue. And the first version of this patch used it: > > http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/63417 > > But you'll notice that it's been almost 10 months since the last version > of CDF saw the daylight, and Laurent seems to be too busy with other > duties to carry on with this. Additionally the KMS/DRM guys don't look > too kindly on it. The bottom line is: there are 3 different platform > suffering from lack of DT bindings for CLCD and I've got fed up with > waiting. Anyway, I've intentionally kept the panel properties in a > separate section and wrote "can be described", to keep an open way for > future "display bindings", if and when they appear. I might risk people start yelling at me, but perhaps you could explicitly mark this binding as "preliminary", so it is clear that the intention was to switch to whatever common design in future. The recently posted v3 of the CDF uses DT bindings specified at: Documentation/devicetree/bindings/media/video-interfaces.txt, Documentation/devicetree/bindings/video/display-timing.txt. Theoretically, the bindings are independent of any OS implementation, thus I guess if you used the above bindings, perhaps extended with couple more properties, it might have been possible to switch to any common OS API in future, without changing the bindings. That all might be highly theoretical though. :) -- Thanks, Sylwester
On Tue, 2013-09-10 at 22:41 +0100, Sylwester Nawrocki wrote: > On 09/09/2013 01:19 PM, Pawel Moll wrote: > > On Sat, 2013-09-07 at 23:55 +0100, Sylwester Nawrocki wrote: > >> H --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt > >>> @@ -0,0 +1,87 @@ > >>> +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 > >> > >> nit: Shouldn't the abbreviation be CLCDC ? > > > > The commonly used acronym for this cell is CLCD. For what its worth, I > > can make the line read: > > > > ARM PrimeCell Color LCD (CLCD) Controller PL110/PL111 > > OK, that's probably better. A meaningless detail, I've just seen mostly > CLCDC used in the PL100/PL111 TRM and LCDC would presumably be more > intuitive > for this type of IP block. I think I'll completely remove the acronym. No intuition will be necessary then :-) > >>> +- video-ram: phandle to a node describing specialized video memory > >>> + (that is *not* described in the top level "memory" node) > >>> + that must be used as a framebuffer, eg. due to restrictions > >>> + of the system interconnect; the node must contain a > >>> + standard reg property describing the address and the size > >>> + of the memory area > >> > >> It seems the "specialized video memory" is described by some vendor specific > >> DT binding ? Is it documented ? It sounds like you are unnecessarily > >> repeating the memory node details here. > > > > I appreciate this property being the hardest to swallow, but the problem > > it is trying to solve is quite simple, really. System can be designed in > > such a way that CLCD is *not* able to read data from the main memory. In > > such case there will be a separate block of (usually static) memory > > "next" to CLCD, which *must* be used for the framebuffer. And I've got > > two choices here: to simply define an address and size, or to define a > > phandle to a node with standard reg property. I'll be really grateful > > for ideas how to describe the situation better. > > I though about reusing the binding only, the part defining reserved > (carve out) memory regions. Em, what exactly do you mean? Referring to the definition of "reserved memory region" as an example how to use reg = <(address) (size)>? (because, I don't think it can be "linux,contiguous-memory-region", "reserved-memory-region"). Can do, however I'm not sure if it won't cause even more confusion, as I hope that the CMA bindings will be useful here on their own, to allocate "normal" contiguous framebuffers. Keep in mind those two use cases are very very different. > >> Perhaps this binding/driver should use the common reserved memory bindings, > >> see thread http://www.spinics.net/lists/devicetree/msg02880.html > > > > No, not really - as I said, this is *not* the main RAM of the system. > > CMA doesn't even know about its existence. > > I'm really not an expert in this area, I'll assume we don't list such > separate memory chips in the 'memory' node. ePAPR spec, regarding the memory node, states: "The client program may access memory not covered by any memory reservations (see section 8.3) using any storage attributes it chooses." (note that the reservations mentioned are "the other" reservations, /memreserve/ ones ;-) Now, I wouldn't want my "client program" (read: Linux kernel) to use the "video memory" in question for general allocations, if only because of its rather poor performance (resulting from the interconnect design, not the memory chip characteristics), so I treat it more as a memory mapped device which happens to have a lot of word-long registers... > But if such memory could be used not only by this single IP block it would > probably make sense to have it listed in memory/reserved-memory, so it > could be used by other devices of which drivers possibly wouldn't have to > contain all the detailed dt parsing/memory handling code. The "video RAM" on my platform could be used for other purposes. If you don't mind latencies :-) It's just that the CLCD itself can't use anything else. So, if the memory/reserved-memory described areas *not* to be used as "normal RAM", yes - we could use it. I don't think it's the case now, though. Also, Steven mentioned the other option I talked about - just raw address/size pair. See me deliberation there... > >>> +- panel-dimensions: (optional) array of two numbers (width and height) > >>> + describing physical dimension in mm of the panel > >>> +- panel-type: (required) must be "tft" or "stn", defines panel type > >>> +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining > >>> + widths in bits of the R, G and B components > >>> +- panel-tft-rb-swapped: (for "tft" panel type) if present means that > >>> + the R& B components are swapped on the board > >>> +- panel-stn-color: (for "stn" panel type) if present means that > >>> + the panel is a colour STN display, if missing > >>> + is a monochrome display > >>> +- panel-stn-dual: (for "stn" panel type) if present means that there > >>> + are two STN displays connected > >>> +- panel-stn-4bit: (for monochrome "stn" panel) if present means > >>> + that the monochrome display is connected > >>> + via 4 bit-wide interface > >> > >> Are this vendor specific or common properties ? Shouldn't those be prefixed > >> with the vendor name ? > > > > I have no answer to this question. My guts are telling me - nope. TFT is > > TFT, STN is STN, nothing to do with "arm,". But I welcome other > > opinions. > > I thought about documenting such a common properties in a common place. > It's > not immediately clear from names of these properties that they apply to > PL110/ > PL111 devices only. If we let such generic names being redefined across DT > bindings of different devices it is going to be pretty messy IMHO. Same > property in two different dts files would potentially have different > meaning. > > So perhaps instead of panel-dimensions we could define common > properties, e.g. > > - display-phys-width: physical horizontal dimension of a display in > millimetres > (micrometres ?); > - display-phys-height: physical vertical dimension of a display in > millimetres > (micrometres ?); > > Instead of 'panel-stn-color' a boolean property 'monochrome-display', > the default > when this property was missing would be "colour display". > > I'd like to leave defining such common properties to someone having more > experience > in the display area. I don't think it would take much time come up with > generic > names for that couple properties you need. Then CDF implementation > would simply use whatever gets agreed. Ok, I see what you're saying. Yes, this could be done. No, I don't claim to have enough expertise either (micrometers??? :-O ;-) The other thing is that I don't really expect generic CDF bindings to cover such things. They will (hopefully) only describe what's connected with what. And the drivers should know how. Of course they may need the dimensions & alike in the tree (of course having them standardised would help here), but it's not a CDF job to provide those. Anyway, if I decide to split the panel data into a separate sub-node, I'll look into that. Pawe?
On 09/11/2013 03:43 PM, Pawel Moll wrote: > On Tue, 2013-09-10 at 22:41 +0100, Sylwester Nawrocki wrote: >> On 09/09/2013 01:19 PM, Pawel Moll wrote: >>> On Sat, 2013-09-07 at 23:55 +0100, Sylwester Nawrocki wrote: [...] >>>>> +- video-ram: phandle to a node describing specialized video memory >>>>> + (that is *not* described in the top level "memory" node) >>>>> + that must be used as a framebuffer, eg. due to restrictions >>>>> + of the system interconnect; the node must contain a >>>>> + standard reg property describing the address and the size >>>>> + of the memory area >>>> >>>> It seems the "specialized video memory" is described by some vendor specific >>>> DT binding ? Is it documented ? It sounds like you are unnecessarily >>>> repeating the memory node details here. >>> >>> I appreciate this property being the hardest to swallow, but the problem >>> it is trying to solve is quite simple, really. System can be designed in >>> such a way that CLCD is *not* able to read data from the main memory. In >>> such case there will be a separate block of (usually static) memory >>> "next" to CLCD, which *must* be used for the framebuffer. And I've got >>> two choices here: to simply define an address and size, or to define a >>> phandle to a node with standard reg property. I'll be really grateful >>> for ideas how to describe the situation better. >> >> I thought about reusing the binding only, the part defining reserved >> (carve out) memory regions. > > Em, what exactly do you mean? Referring to the definition of "reserved > memory region" as an example how to use reg =<(address) (size)>? > (because, I don't think it can be "linux,contiguous-memory-region", > "reserved-memory-region"). Can do, however I'm not sure if it won't > cause even more confusion, as I hope that the CMA bindings will be > useful here on their own, to allocate "normal" contiguous framebuffers. > Keep in mind those two use cases are very very different. Yes, I wasn't aware then that your "video RAM" is a separate chip attached to a distinct bus. My idea was to reuse memory node structure, including "reserved-memory" compatible value (note there are some fixups pending and those names are subject to change). Not sure about the property in CLCD device node containing phandle to the memory node (currently 'memory-region'). >>>> Perhaps this binding/driver should use the common reserved memory bindings, >>>> see thread http://www.spinics.net/lists/devicetree/msg02880.html >>> >>> No, not really - as I said, this is *not* the main RAM of the system. >>> CMA doesn't even know about its existence. >> >> I'm really not an expert in this area, I'll assume we don't list such >> separate memory chips in the 'memory' node. > > ePAPR spec, regarding the memory node, states: "The client program may > access memory not covered by any memory reservations (see section 8.3) > using any storage attributes it chooses." (note that the reservations > mentioned are "the other" reservations, /memreserve/ ones ;-) > > Now, I wouldn't want my "client program" (read: Linux kernel) to use the > "video memory" in question for general allocations, if only because of > its rather poor performance (resulting from the interconnect design, not > the memory chip characteristics), so I treat it more as a memory mapped > device which happens to have a lot of word-long registers... > >> But if such memory could be used not only by this single IP block it would >> probably make sense to have it listed in memory/reserved-memory, so it >> could be used by other devices of which drivers possibly wouldn't have to >> contain all the detailed dt parsing/memory handling code. > > The "video RAM" on my platform could be used for other purposes. If you > don't mind latencies :-) It's just that the CLCD itself can't use > anything else. It seems then you just need to assign specific memory region to the device. That's one of the main requirements the recent "CMA" memory bindings were supposed to address. > So, if the memory/reserved-memory described areas *not* to be used as > "normal RAM", yes - we could use it. I don't think it's the case now, > though. AFAIU it's the case, you just need to have compatible property set to "reserved-memory-region". Then the kernel will early call memblock_remove() on that region, which will subsequently be declared for use by the device DMA with a call to dma_declare_coherent_memory(), while populating devices from DT [1]. It's likely I'm missing some details though, that would make this unsuitable for your environment. [1] http://www.spinics.net/lists/devicetree/msg02880.html > Also, Steven mentioned the other option I talked about - just raw > address/size pair. See me deliberation there... > >>>>> +- panel-dimensions: (optional) array of two numbers (width and height) >>>>> + describing physical dimension in mm of the panel >>>>> +- panel-type: (required) must be "tft" or "stn", defines panel type >>>>> +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining >>>>> + widths in bits of the R, G and B components >>>>> +- panel-tft-rb-swapped: (for "tft" panel type) if present means that >>>>> + the R& B components are swapped on the board >>>>> +- panel-stn-color: (for "stn" panel type) if present means that >>>>> + the panel is a colour STN display, if missing >>>>> + is a monochrome display >>>>> +- panel-stn-dual: (for "stn" panel type) if present means that there >>>>> + are two STN displays connected >>>>> +- panel-stn-4bit: (for monochrome "stn" panel) if present means >>>>> + that the monochrome display is connected >>>>> + via 4 bit-wide interface >>>> >>>> Are this vendor specific or common properties ? Shouldn't those be prefixed >>>> with the vendor name ? >>> >>> I have no answer to this question. My guts are telling me - nope. TFT is >>> TFT, STN is STN, nothing to do with "arm,". But I welcome other >>> opinions. >> >> I thought about documenting such a common properties in a common place. >> It's >> not immediately clear from names of these properties that they apply to >> PL110/ >> PL111 devices only. If we let such generic names being redefined across DT >> bindings of different devices it is going to be pretty messy IMHO. Same >> property in two different dts files would potentially have different >> meaning. >> >> So perhaps instead of panel-dimensions we could define common >> properties, e.g. >> >> - display-phys-width: physical horizontal dimension of a display in >> millimetres >> (micrometres ?); >> - display-phys-height: physical vertical dimension of a display in >> millimetres >> (micrometres ?); >> >> Instead of 'panel-stn-color' a boolean property 'monochrome-display', >> the default >> when this property was missing would be "colour display". >> >> I'd like to leave defining such common properties to someone having more >> experience >> in the display area. I don't think it would take much time come up with >> generic >> names for that couple properties you need. Then CDF implementation >> would simply use whatever gets agreed. > > Ok, I see what you're saying. Yes, this could be done. No, I don't claim > to have enough expertise either (micrometers??? :-O ;-) The other thing > is that I don't really expect generic CDF bindings to cover such things. > They will (hopefully) only describe what's connected with what. And the > drivers should know how. Of course they may need the dimensions& alike > in the tree (of course having them standardised would help here), but > it's not a CDF job to provide those. Of course it's always easier to define couple of DT properties that would cover part of functionality of some specific device. But then such properties should be prefixed with vendor name AFAIU, since they are not approved as common ones and useful for wider set of devices. From the device tree perspective CDF is just a collection of (display related) devices and a complete set of DT properties will be needed to describe them. Certainly some share of device-specific properties should be expected. Links between (sub)devices can be already described in DT by the binding documented in video-interfaces.txt. -- Thanks, Sylwester
On Wed, 2013-09-11 at 22:14 +0100, Sylwester Nawrocki wrote: > Yes, I wasn't aware then that your "video RAM" is a separate chip attached > to a distinct bus. > My idea was to reuse memory node structure, including "reserved-memory" > compatible value (note there are some fixups pending and those names are > subject to change). Not sure about the property in CLCD device node > containing phandle to the memory node (currently 'memory-region'). As I just told Steven, I'll just make it painfully low-level property defining "address you have to generate to access base of the framebuffer". Of course one can still go the CMA way, if such need arises. > > Ok, I see what you're saying. Yes, this could be done. No, I don't claim > > to have enough expertise either (micrometers??? :-O ;-) The other thing > > is that I don't really expect generic CDF bindings to cover such things. > > They will (hopefully) only describe what's connected with what. And the > > drivers should know how. Of course they may need the dimensions& alike > > in the tree (of course having them standardised would help here), but > > it's not a CDF job to provide those. > > Of course it's always easier to define couple of DT properties that would > cover part of functionality of some specific device. But then such > properties should be prefixed with vendor name AFAIU, since they are > not approved as common ones and useful for wider set of devices. This is a policy that changed many times already... Anyway, I think I've got an idea how to render the problem custom panel properties invalid. If so, I'll send another version of the patch. > From the device tree perspective CDF is just a collection of (display > related) devices and a complete set of DT properties will be needed to > describe them. Certainly some share of device-specific properties should > be expected. Links between (sub)devices can be already described in DT by > the binding documented in video-interfaces.txt. Whether to use the v4l scheme or not still seems to be a bone of contention between the video and the DRM/KMS folk, so I wouldn't draw any conclusions yet. Pawe?
On Thu, 2013-09-12 at 16:23 +0100, Pawel Moll wrote: > Anyway, I think I've got an idea how to render the problem custom panel > properties invalid. If so, I'll send another version of the patch. So, instead of describing the panel I thought I'd go lower level and try to describe the CLCD's signal functions. It gets down to something like this: 8<---- - arm,pl11x,panel-data-pads: array of 24 cells, each of them describing a function of one of the CLD pads, starting from 0 up to 23; each pad can be described by one of the following values: - 0: reserved (not connected) - 0x100-0x107: color upper STN panel data 0 to 7 - 0x110-0x117: color lower STN panel data 0 to 7 - 0x200-0x207: mono upper STN panel data 0 to 7 - 0x210-0x217: mono lower STN panel data 0 to 7 - 0x300-0x307: red component bit 0 to 7 of TFT panel data - 0x310-0x317: green component bit 0 to 7 of TFT panel data - 0x320-0x327: blue component bit 0 to 7 of TFT panel data - 0x330: intensity bit of TFT panel data Example set of values for standard panel interfaces: - PL110 single colour STN panel: <0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL110 dual colour STN panel: <0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>, <0x117 0x116 0x115 0x114 0x113 0x112 0x111 0x110>, <0 0 0 0 0 0 0 0>; - PL110 single mono 4-bit STN panel: <0x203 0x202 0x201 0x200>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL110 dual mono 4-bit STN panel: <0x203 0x202 0x201 0x200>, <0 0 0 0>, <0x213 0x212 0x211 0x210>, <0 0 0 0 0 0 0 0 0 0 0 0>; - PL110 single mono 8-bit STN panel: <0x207 0x206 0x205 0x204 0x203 0x202 0x201 0x200>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL110 dual mono 8-bit STN panel: <0x207 0x206 0x205 0x204 0x203 0x202 0x201 0x200>, <0x217 0x216 0x215 0x214 0x213 0x212 0x211 0x210>, <0 0 0 0 0 0 0 0>; - PL110 TFT (1:)5:5:5 panel: <0x330 0x300 0x301 0x302 0x303 0x304>, <0x330 0x310 0x311 0x312 0x313 0x314>, <0x330 0x320 0x321 0x322 0x323 0x324>, <0 0 0 0 0 0> - PL110 and PL111 TFT RGB 888 panel: <0x300 0x301 0x302 0x303 0x304 0x305 0x306 0x307>, <0x310 0x311 0x312 0x313 0x314 0x315 0x316 0x317>, <0x320 0x321 0x322 0x323 0x324 0x325 0x326 0x327>; - PL111 single colour STN panel: <0x100 0x101 0x102 0x103 0x104 0x105 0x106 0x107>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL111 dual colour STN panel: <0x100 0x101 0x102 0x103 0x104 0x105 0x106 0x107>, <0x110 0x111 0x112 0x113 0x114 0x115 0x116 0x117>, <0 0 0 0 0 0 0 0>; - PL111 single mono 4-bit STN panel: <0x200 0x201 0x202 0x203>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL111 dual mono 4-bit STN panel: <0x200 0x201 0x202 0x203>, <0 0 0 0>, <0x210 0x211 0x212 0x213>, <0 0 0 0 0 0 0 0 0 0 0 0>; - PL111 single mono 8-bit STN panel: <0x200 0x201 0x202 0x203 0x204 0x205 0x206 0x207>, <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>; - PL111 dual mono 8-bit STN panel: <0x200 0x201 0x202 0x203 0x204 0x205 0x206 0x207>, <0x210 0x211 0x212 0x213 0x214 0x215 0x216 0x217>, <0 0 0 0 0 0 0 0>; - PL111 TFT 4:4:4 panel: <0 0 0 0>, <0x300 0x301 0x302 0x303>, <0 0 0 0>, <0x310 0x311 0x312 0x313>, <0 0 0 0>, <0x320 0x321 0x322 0x323>; - PL111 TFT 5:6:5 panel: <0 0 0>, <0x300 0x301 0x302 0x303 0x304>, <0 0>, <0x310 0x311 0x312 0x313 0x314 0x315>, <0 0 0>, <0x320 0x321 0x322 0x323 0x324>; - PL111 TFT (1):5:5:5 panel: <0 0 0>, <0x300 0x301 0x302 0x303 0x304>, <0 0>, <0x330 0x310 0x311 0x312 0x313 0x314>, <0 0 0>, <0x320 0x321 0x322 0x323 0x324>; 8<---- Of course the examples above could be #defined. Such approach would also solve problem with bizarre cases like the Versatile's (not Express ;-) muxer Russell mentioned (where there is a de-facto custom wiring used). And no, I had a look at pinmux bindings and I don't think they fit here at all... Thoughts? Pawe?
On 09/12/2013 12:10 PM, Pawel Moll wrote: > On Thu, 2013-09-12 at 16:23 +0100, Pawel Moll wrote: >> Anyway, I think I've got an idea how to render the problem custom panel >> properties invalid. If so, I'll send another version of the patch. > > So, instead of describing the panel I thought I'd go lower level and try > to describe the CLCD's signal functions. It gets down to something like > this: > > 8<---- > - arm,pl11x,panel-data-pads: array of 24 cells, each of them describing > a function of one of the CLD pads, > starting from 0 up to 23; each pad can > be described by one of the following values: > - 0: reserved (not connected) > - 0x100-0x107: color upper STN panel data 0 to 7 ... > Such approach would also solve problem with bizarre cases like the > Versatile's (not Express ;-) muxer Russell mentioned (where there is a > de-facto custom wiring used). And no, I had a look at pinmux bindings > and I don't think they fit here at all... > > Thoughts? I think pinctrl would work just fine here. However, I suspect there's little need for pinctrl. pinctrl is required when one module may impose requirements on another module's muxable pins. I assume that the configuration for pins on CLCD is only relevant for CLCD itself, so there's no need to export an interface by which something else can configure the pins. Now, if any of the pins could be e.g. "GPIO", then this argument might not apply. Hence, I think pinctrl is only "possible" here, not "required", and the approach above seems fine to me.
On Thu, Sep 12, 2013 at 04:19:45PM -0600, Stephen Warren wrote: > On 09/12/2013 12:10 PM, Pawel Moll wrote: > > Such approach would also solve problem with bizarre cases like the > > Versatile's (not Express ;-) muxer Russell mentioned (where there is a > > de-facto custom wiring used). And no, I had a look at pinmux bindings > > and I don't think they fit here at all... > > > > Thoughts? > > I think pinctrl would work just fine here. However, I suspect there's > little need for pinctrl. pinctrl is required when one module may impose > requirements on another module's muxable pins. I assume that the > configuration for pins on CLCD is only relevant for CLCD itself, so > there's no need to export an interface by which something else can > configure the pins. Well, the hardware is much like this: CLCD ---> Mux ---> output and the mux is controlled by a register, and will manipulate the output from the CLCD controller such that the framebuffer contents can be RGB565, BGR565, XRGB1555, XBGR1555 or RGB888. (Actually, the Versatile mux can only support one of the 1555 formats, but I forget which it is.) I'm not sure that its worth describing all these in DT explicitly, but maybe encoding it as a special "versatile" version of the CLCD primecell instead - especially as the external mux takes care of the colour component order rather than the CLCD controller (and that requires a bit of board specific code to deal with.)
diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt new file mode 100644 index 0000000..178aebb --- /dev/null +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt @@ -0,0 +1,87 @@ +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 + +See also Documentation/devicetree/bindings/arm/primecell.txt + +Required properties: + +- compatible: must be one of: + "arm,pl110", "arm,primecell" + "arm,pl111", "arm,primecell" +- reg: base address and size of the control registers block +- interrupts: either a single interrupt specifier representing the + combined interrupt output (CLCDINTR) or an array of + four interrupt specifiers for CLCDMBEINTR, + CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR; in the + latter case interrupt names must be specified + (see below) +- interrupt-names: when four interrupts are specified, their names: + "mbe", "vcomp", "lnbu", "fuf" + must be specified in order respective to the + interrupt specifiers +- clocks: contains phandle and clock specifier pairs for the entries + in the clock-names property. See + Documentation/devicetree/binding/clock/clock-bindings.txt +- clocks names: should contain "clcdclk" and "apb_pclk" + +Optional properties: + +- video-ram: phandle to a node describing specialized video memory + (that is *not* described in the top level "memory" node) + that must be used as a framebuffer, eg. due to restrictions + of the system interconnect; the node must contain a + standard reg property describing the address and the size + of the memory area +- max-framebuffer-size: maximum size in bytes of the framebuffer the + system can handle, eg. in terms of available + memory bandwidth + +In the simplest case of a display panel being connected directly to the +CLCD, it can be described in the node: + +- panel-dimensions: (optional) array of two numbers (width and height) + describing physical dimension in mm of the panel +- panel-type: (required) must be "tft" or "stn", defines panel type +- panel-tft-interface: (for "tft" panel type) array of 3 numbers defining + widths in bits of the R, G and B components +- panel-tft-rb-swapped: (for "tft" panel type) if present means that + the R & B components are swapped on the board +- panel-stn-color: (for "stn" panel type) if present means that + the panel is a colour STN display, if missing + is a monochrome display +- panel-stn-dual: (for "stn" panel type) if present means that there + are two STN displays connected +- panel-stn-4bit: (for monochrome "stn" panel) if present means + that the monochrome display is connected + via 4 bit-wide interface +- display-timings: standard display timings sub-node, see + Documentation/devicetree/bindings/video/display-timing.txt + +Example: + + clcd@1f0000 { + compatible = "arm,pl111", "arm,primecell"; + reg = <0x1f0000 0x1000>; + interrupts = <14>; + clocks = <&v2m_oscclk1>, <&smbclk>; + clock-names = "clcdclk", "apb_pclk"; + + video-ram = <&v2m_vram>; + max-framebuffer-size = <614400>; /* 640x480 16bpp */ + + panel-type = "tft"; + panel-tft-interface = <8 8 8>; + display-timings { + native-mode = <&v2m_clcd_timing0>; + v2m_clcd_timing0: vga { + clock-frequency = <25175000>; + hactive = <640>; + hback-porch = <40>; + hfront-porch = <24>; + hsync-len = <96>; + vactive = <480>; + vback-porch = <32>; + vfront-porch = <11>; + vsync-len = <2>; + }; + }; + }; diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 4cf1e1d..375bf63 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -316,6 +316,7 @@ config FB_ARMCLCD select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT + select VIDEOMODE_HELPERS if OF help This framebuffer device driver is for the ARM PrimeCell PL110 Colour LCD controller. ARM PrimeCells provide the building diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c index 0a2cce7..145ca5a 100644 --- a/drivers/video/amba-clcd.c +++ b/drivers/video/amba-clcd.c @@ -25,6 +25,11 @@ #include <linux/amba/clcd.h> #include <linux/clk.h> #include <linux/hardirq.h> +#include <linux/dma-mapping.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <video/of_display_timing.h> +#include <video/of_videomode.h> #include <asm/sizes.h> @@ -542,6 +547,212 @@ static int clcdfb_register(struct clcd_fb *fb) return ret; } +#ifdef CONFIG_OF +static int clcdfb_of_get_tft_panel(struct device_node *node, + struct clcd_panel *panel) +{ + int err; + u32 rgb[3]; + int r, g, b; + + err = of_property_read_u32_array(node, "panel-tft-interface", rgb, 3); + if (err) + return err; + + r = rgb[0]; + g = rgb[1]; + b = rgb[2]; + + /* Bypass pixel clock divider, data output on the falling edge */ + panel->tim2 = TIM2_BCD | TIM2_IPC; + + /* TFT display, vert. comp. interrupt at the start of the back porch */ + panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1); + + if (r >= 4 && g >= 4 && b >= 4) + panel->caps |= CLCD_CAP_444; + if (r >= 5 && g >= 5 && b >= 5) + panel->caps |= CLCD_CAP_5551; + if (r >= 5 && g >= 6 && b >= 5) + panel->caps |= CLCD_CAP_565; + if (r >= 8 && g >= 8 && b >= 8) + panel->caps |= CLCD_CAP_888; + + if (of_get_property(node, "panel-tft-rb-swapped", NULL)) + panel->caps &= ~CLCD_CAP_RGB; + else + panel->caps &= ~CLCD_CAP_BGR; + + return 0; +} + +static int clcdfb_of_init_display(struct clcd_fb *fb) +{ + struct device_node *node = fb->dev->dev.of_node; + u32 max_size; + u32 dimensions[2]; + char *mode_name; + int len, err; + const char *panel_type; + + fb->panel = devm_kzalloc(&fb->dev->dev, sizeof(*fb->panel), GFP_KERNEL); + if (!fb->panel) + return -ENOMEM; + + err = of_get_fb_videomode(fb->dev->dev.of_node, &fb->panel->mode, + OF_USE_NATIVE_MODE); + if (err) + return err; + + len = snprintf(NULL, 0, "%ux%u@%u", fb->panel->mode.xres, + fb->panel->mode.yres, fb->panel->mode.refresh); + mode_name = devm_kzalloc(&fb->dev->dev, len + 1, GFP_KERNEL); + snprintf(mode_name, len + 1, "%ux%u@%u", fb->panel->mode.xres, + fb->panel->mode.yres, fb->panel->mode.refresh); + fb->panel->mode.name = mode_name; + + err = of_property_read_u32(node, "max-framebuffer-size", &max_size); + if (!err) + fb->panel->bpp = max_size / (fb->panel->mode.xres * + fb->panel->mode.yres) * 8; + else + fb->panel->bpp = 32; + +#ifdef CONFIG_CPU_BIG_ENDIAN + fb->panel->cntl |= CNTL_BEBO; +#endif + + if (of_property_read_u32_array(node, "panel-dimensions", + dimensions, 2) == 0) { + fb->panel->width = dimensions[0]; + fb->panel->height = dimensions[1]; + } else { + fb->panel->width = -1; + fb->panel->height = -1; + } + + panel_type = of_get_property(node, "panel-type", &len); + if (strncmp(panel_type, "tft", len) == 0) + return clcdfb_of_get_tft_panel(node, fb->panel); + else + return -EINVAL; +} + +static int clcdfb_of_vram_setup(struct clcd_fb *fb) +{ + const __be32 *prop = of_get_property(fb->dev->dev.of_node, "video-ram", + NULL); + struct device_node *node = of_find_node_by_phandle(be32_to_cpup(prop)); + u64 size; + int err; + + if (!node) + return -ENODEV; + + err = clcdfb_of_init_display(fb); + if (err) + return err; + + fb->fb.screen_base = of_iomap(node, 0); + if (!fb->fb.screen_base) + return -ENOMEM; + + fb->fb.fix.smem_start = of_translate_address(node, + of_get_address(node, 0, &size, NULL)); + fb->fb.fix.smem_len = size; + + return 0; +} + +static int clcdfb_of_vram_mmap(struct clcd_fb *fb, struct vm_area_struct *vma) +{ + unsigned long off, user_size, kernel_size; + + off = vma->vm_pgoff << PAGE_SHIFT; + user_size = vma->vm_end - vma->vm_start; + kernel_size = fb->fb.fix.smem_len; + + if (off >= kernel_size || user_size > (kernel_size - off)) + return -ENXIO; + + return remap_pfn_range(vma, vma->vm_start, + __phys_to_pfn(fb->fb.fix.smem_start) + vma->vm_pgoff, + user_size, + pgprot_writecombine(vma->vm_page_prot)); +} + +static void clcdfb_of_vram_remove(struct clcd_fb *fb) +{ + iounmap(fb->fb.screen_base); +} + +static int clcdfb_of_dma_setup(struct clcd_fb *fb) +{ + unsigned long framesize; + dma_addr_t dma; + int err; + + err = clcdfb_of_init_display(fb); + if (err) + return err; + + framesize = fb->panel->mode.xres * fb->panel->mode.yres * + fb->panel->bpp / 8; + fb->fb.screen_base = dma_alloc_writecombine(&fb->dev->dev, framesize, + &dma, GFP_KERNEL); + if (!fb->fb.screen_base) + return -ENOMEM; + + fb->fb.fix.smem_start = dma; + fb->fb.fix.smem_len = framesize; + + return 0; +} + +static int clcdfb_of_dma_mmap(struct clcd_fb *fb, struct vm_area_struct *vma) +{ + return dma_mmap_writecombine(&fb->dev->dev, vma, fb->fb.screen_base, + fb->fb.fix.smem_start, fb->fb.fix.smem_len); +} + +static void clcdfb_of_dma_remove(struct clcd_fb *fb) +{ + dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len, + fb->fb.screen_base, fb->fb.fix.smem_start); +} + +static struct clcd_board *clcdfb_of_get_board(struct amba_device *dev) +{ + struct clcd_board *board = devm_kzalloc(&dev->dev, sizeof(*board), + GFP_KERNEL); + struct device_node *node = dev->dev.of_node; + + if (!board) + return NULL; + + board->name = of_node_full_name(node); + board->caps = CLCD_CAP_ALL; + board->check = clcdfb_check; + board->decode = clcdfb_decode; + if (of_find_property(node, "video-ram", NULL)) { + board->setup = clcdfb_of_vram_setup; + board->mmap = clcdfb_of_vram_mmap; + board->remove = clcdfb_of_vram_remove; + } else { + board->setup = clcdfb_of_dma_setup; + board->mmap = clcdfb_of_dma_mmap; + board->remove = clcdfb_of_dma_remove; + } + + return board; +} +#else +static struct clcd_board *clcdfb_of_get_board(struct amba_dev *dev) +{ + return NULL; +} +#endif + static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) { struct clcd_board *board = dev->dev.platform_data; @@ -549,6 +760,9 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) int ret; if (!board) + board = clcdfb_of_get_board(dev); + + if (!board) return -EINVAL; ret = amba_request_regions(dev, NULL);
This patch adds basic DT bindings for the PL11x CLCD cells and make their fbdev driver use them. Signed-off-by: Pawel Moll <pawel.moll@arm.com> --- .../devicetree/bindings/video/arm,pl11x.txt | 87 +++++++++ drivers/video/Kconfig | 1 + drivers/video/amba-clcd.c | 214 +++++++++++++++++++++ 3 files changed, 302 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/arm,pl11x.txt