On Mon, Apr 13, 2009 at 10:49 AM, Stephen Neuendorffer <stephen.neuendorf...@xilinx.com> wrote: > > I think the mainline driver might (still) assume the presence of a PLB->DCR > bridge?
Correct. > So there are really 4 cases: > Core has DCR access, accessed directly using DCR. > Core has DCR access, accessed indirectly using DCR. > Core has DCR access, accessed through plb->dcr bridge. > Core has PLB access. sounds right to me. g. > > Steve > >> -----Original Message----- >> From: linuxppc-dev-bounces+stephen.neuendorffer=xilinx....@ozlabs.org >> [mailto:linuxppc-dev- >> bounces+stephen.neuendorffer=xilinx....@ozlabs.org] On Behalf Of John Linn >> Sent: Monday, April 13, 2009 7:13 AM >> To: grant.lik...@secretlab.ca >> Cc: linux-fbdev-de...@lists.sourceforge.net; adap...@gmail.com; Suneel >> Garapati; linuxppc- >> d...@ozlabs.org; akonova...@ru.mvista.com >> Subject: RE: [PATCH] [V2] Xilinx : Framebuffer Driver: Add PLB >> support(non-DCR) >> >> I thought it was based on mainline, but I now see the weirdness you're >> talking about. I'll dig in on >> this and let you know more as I'm confused also. >> >> -- John >> >> > -----Original Message----- >> > From: Grant Likely [mailto:grant.lik...@secretlab.ca] >> > Sent: Sunday, April 12, 2009 12:15 AM >> > To: John Linn >> > Cc: jwbo...@linux.vnet.ibm.com; linux-fbdev-de...@lists.sourceforge.net; >> > linuxppc-dev@ozlabs.org; >> > akonova...@ru.mvista.com; adap...@gmail.com; Suneel Garapati >> > Subject: Re: [PATCH] [V2] Xilinx : Framebuffer Driver: Add PLB support >> > (non-DCR) >> > >> > What tree is this patch prepared against? The version in mainline >> > already does PLB access, and doesn't support DCR at all. It appears >> > that this driver is based on something that does the opposite. >> > >> > g. >> > >> > On Fri, Apr 10, 2009 at 3:17 PM, John Linn <john.l...@xilinx.com> wrote: >> > > From: Suneel <sune...@xilinx.com> >> > > >> > > Added support for the new xps tft controller. >> > > >> > > The new core has PLB interface support in addition to existing >> > > DCR interface. >> > > >> > > The driver has been modified to support this new core which >> > > can be connected on PLB or DCR bus. >> > > >> > > Signed-off-by: Suneel <sune...@xilinx.com> >> > > Signed-off-by: John Linn <john.l...@xilinx.com> >> > > --- >> > > >> > > V2 - Incorporated comments from Josh, Grant and others >> > > >> > > drivers/video/xilinxfb.c | 213 >> > > ++++++++++++++++++++++++++++++++-------------- >> > > 1 files changed, 150 insertions(+), 63 deletions(-) >> > > >> > > diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c >> > > index a82c530..d151237 100644 >> > > --- a/drivers/video/xilinxfb.c >> > > +++ b/drivers/video/xilinxfb.c >> > > @@ -1,13 +1,13 @@ >> > > /* >> > > - * xilinxfb.c >> > > * >> > > - * Xilinx TFT LCD frame buffer driver >> > > + * Xilinx TFT frame buffer driver >> > > * >> > > * Author: MontaVista Software, Inc. >> > > * sou...@mvista.com >> > > * >> > > * 2002-2007 (c) MontaVista Software, Inc. >> > > * 2007 (c) Secret Lab Technologies, Ltd. >> > > + * 2009 (c) Xilinx Inc. >> > > * >> > > * This file is licensed under the terms of the GNU General Public >> > > License >> > > * version 2. This program is licensed "as is" without any warranty of >> > > any >> > > @@ -31,27 +31,31 @@ >> > > #include <linux/fb.h> >> > > #include <linux/init.h> >> > > #include <linux/dma-mapping.h> >> > > -#if defined(CONFIG_OF) >> > > #include <linux/of_device.h> >> > > #include <linux/of_platform.h> >> > > -#endif >> > > -#include <asm/io.h> >> > > +#include <linux/io.h> >> > > #include <linux/xilinxfb.h> >> > > #include <asm/dcr.h> >> > > >> > > #define DRIVER_NAME "xilinxfb" >> > > -#define DRIVER_DESCRIPTION "Xilinx TFT LCD frame buffer driver" >> > > + >> > > >> > > /* >> > > * Xilinx calls it "PLB TFT LCD Controller" though it can also be used >> > > for >> > > - * the VGA port on the Xilinx ML40x board. This is a hardware display >> > > controller >> > > - * for a 640x480 resolution TFT or VGA screen. >> > > + * the VGA port on the Xilinx ML40x board. This is a hardware display >> > > + * controller for a 640x480 resolution TFT or VGA screen. >> > > * >> > > * The interface to the framebuffer is nice and simple. There are two >> > > * control registers. The first tells the LCD interface where in memory >> > > * the frame buffer is (only the 11 most significant bits are used, so >> > > * don't start thinking about scrolling). The second allows the LCD to >> > > * be turned on or off as well as rotated 180 degrees. >> > > + * >> > > + * In case of direct PLB access the second control register will be at >> > > + * an offset of 4 as compared to the DCR access where the offset is 1 >> > > + * i.e. REG_CTRL. So this is taken care in the function >> > > + * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of >> > > + * direct PLB access. >> > > */ >> > > #define NUM_REGS 2 >> > > #define REG_FB_ADDR 0 >> > > @@ -108,10 +112,18 @@ static struct fb_var_screeninfo xilinx_fb_var = { >> > > .activate = FB_ACTIVATE_NOW >> > > }; >> > > >> > > + >> > > +#define PLB_ACCESS_FLAG 0x1 /* 1 = PLB, 0 = DCR */ >> > > + >> > > struct xilinxfb_drvdata { >> > > >> > > struct fb_info info; /* FB driver info record */ >> > > >> > > + phys_addr_t regs_phys; /* phys. address of the control >> > > + registers */ >> > > + void __iomem *regs; /* virt. address of the control >> > > + registers */ >> > > + >> > > dcr_host_t dcr_host; >> > > unsigned int dcr_start; >> > > unsigned int dcr_len; >> > > @@ -120,6 +132,8 @@ struct xilinxfb_drvdata { >> > > dma_addr_t fb_phys; /* phys. address of the frame >> > > buffer */ >> > > int fb_alloced; /* Flag, was the fb memory >> > > alloced? */ >> > > >> > > + u8 flags; /* features of the driver */ >> > > + >> > > u32 reg_ctrl_default; >> > > >> > > u32 pseudo_palette[PALETTE_ENTRIES_NO]; >> > > @@ -130,14 +144,19 @@ struct xilinxfb_drvdata { >> > > container_of(_info, struct xilinxfb_drvdata, info) >> > > >> > > /* >> > > - * The LCD controller has DCR interface to its registers, but all >> > > - * the boards and configurations the driver has been tested with >> > > - * use opb2dcr bridge. So the registers are seen as memory mapped. >> > > - * This macro is to make it simple to add the direct DCR access >> > > - * when it's needed. >> > > + * The XPS TFT Controller can be accessed through PLB or DCR interface. >> > > + * To perform the read/write on the registers we need to check on >> > > + * which bus its connected and call the appropriate write API. >> > > */ >> > > -#define xilinx_fb_out_be32(driverdata, offset, val) \ >> > > - dcr_write(driverdata->dcr_host, offset, val) >> > > +static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 >> > > offset, >> > > + u32 val) >> > > +{ >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) >> > > + out_be32(drvdata->regs + (offset << 2), val); >> > > + else >> > > + dcr_write(drvdata->dcr_host, offset, val); >> > > + >> > > +} >> > > >> > > static int >> > > xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, >> > > unsigned blue, >> > > @@ -175,7 +194,8 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi) >> > > switch (blank_mode) { >> > > case FB_BLANK_UNBLANK: >> > > /* turn on panel */ >> > > - xilinx_fb_out_be32(drvdata, REG_CTRL, >> > > drvdata->reg_ctrl_default); >> > > + xilinx_fb_out_be32(drvdata, REG_CTRL, >> > > + drvdata->reg_ctrl_default); >> > > break; >> > > >> > > case FB_BLANK_NORMAL: >> > > @@ -191,8 +211,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi) >> > > return 0; /* success */ >> > > } >> > > >> > > -static struct fb_ops xilinxfb_ops = >> > > -{ >> > > +static struct fb_ops xilinxfb_ops = { >> > > .owner = THIS_MODULE, >> > > .fb_setcolreg = xilinx_fb_setcolreg, >> > > .fb_blank = xilinx_fb_blank, >> > > @@ -205,25 +224,35 @@ static struct fb_ops xilinxfb_ops = >> > > * Bus independent setup/teardown >> > > */ >> > > >> > > -static int xilinxfb_assign(struct device *dev, dcr_host_t dcr_host, >> > > - unsigned int dcr_start, unsigned int dcr_len, >> > > +static int xilinxfb_assign(struct device *dev, >> > > + struct xilinxfb_drvdata *drvdata, >> > > + unsigned long physaddr, >> > > struct xilinxfb_platform_data *pdata) >> > > { >> > > - struct xilinxfb_drvdata *drvdata; >> > > int rc; >> > > int fbsize = pdata->xvirt * pdata->yvirt * BYTES_PER_PIXEL; >> > > >> > > - /* Allocate the driver data region */ >> > > - drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL); >> > > - if (!drvdata) { >> > > - dev_err(dev, "Couldn't allocate device private >> > > record\n"); >> > > - return -ENOMEM; >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) { >> > > + /* >> > > + * Map the control registers in if the controller >> > > + * is on direct PLB interface. >> > > + */ >> > > + if (!request_mem_region(physaddr, 8, DRIVER_NAME)) { >> > > + dev_err(dev, "Couldn't lock memory region at >> > > 0x%08lX\n", >> > > + physaddr); >> > > + rc = -ENODEV; >> > > + goto err_region; >> > > + } >> > > + >> > > + drvdata->regs_phys = physaddr; >> > > + drvdata->regs = ioremap(physaddr, 8); >> > > + if (!drvdata->regs) { >> > > + dev_err(dev, "Couldn't lock memory region at >> > > 0x%08lX\n", >> > > + physaddr); >> > > + rc = -ENODEV; >> > > + goto err_map; >> > > + } >> > > } >> > > - dev_set_drvdata(dev, drvdata); >> > > - >> > > - drvdata->dcr_start = dcr_start; >> > > - drvdata->dcr_len = dcr_len; >> > > - drvdata->dcr_host = dcr_host; >> > > >> > > /* Allocate the framebuffer memory */ >> > > if (pdata->fb_phys) { >> > > @@ -238,7 +267,10 @@ static int xilinxfb_assign(struct device *dev, >> > > dcr_host_t dcr_host, >> > > if (!drvdata->fb_virt) { >> > > dev_err(dev, "Could not allocate frame buffer memory\n"); >> > > rc = -ENOMEM; >> > > - goto err_region; >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) >> > > + goto err_fbmem; >> > > + else >> > > + goto err_region; >> > > } >> > > >> > > /* Clear (turn to black) the framebuffer */ >> > > @@ -251,7 +283,8 @@ static int xilinxfb_assign(struct device *dev, >> > > dcr_host_t dcr_host, >> > > drvdata->reg_ctrl_default = REG_CTRL_ENABLE; >> > > if (pdata->rotate_screen) >> > > drvdata->reg_ctrl_default |= REG_CTRL_ROTATE; >> > > - xilinx_fb_out_be32(drvdata, REG_CTRL, drvdata->reg_ctrl_default); >> > > + xilinx_fb_out_be32(drvdata, REG_CTRL, >> > > + drvdata->reg_ctrl_default); >> > > >> > > /* Fill struct fb_info */ >> > > drvdata->info.device = dev; >> > > @@ -287,9 +320,14 @@ static int xilinxfb_assign(struct device *dev, >> > > dcr_host_t dcr_host, >> > > goto err_regfb; >> > > } >> > > >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) { >> > > + /* Put a banner in the log (for DEBUG) */ >> > > + dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr, >> > > + drvdata->regs); >> > > + } >> > > /* Put a banner in the log (for DEBUG) */ >> > > dev_dbg(dev, "fb: phys=%p, virt=%p, size=%x\n", >> > > - (void*)drvdata->fb_phys, drvdata->fb_virt, fbsize); >> > > + (void *)drvdata->fb_phys, drvdata->fb_virt, fbsize); >> > > >> > > return 0; /* success */ >> > > >> > > @@ -300,9 +338,20 @@ err_cmap: >> > > if (drvdata->fb_alloced) >> > > dma_free_coherent(dev, PAGE_ALIGN(fbsize), >> > > drvdata->fb_virt, >> > > drvdata->fb_phys); >> > > + else >> > > + iounmap(drvdata->fb_virt); >> > > + >> > > /* Turn off the display */ >> > > xilinx_fb_out_be32(drvdata, REG_CTRL, 0); >> > > >> > > +err_fbmem: >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) >> > > + iounmap(drvdata->regs); >> > > + >> > > +err_map: >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) >> > > + release_mem_region(physaddr, 8); >> > > + >> > > err_region: >> > > kfree(drvdata); >> > > dev_set_drvdata(dev, NULL); >> > > @@ -325,11 +374,18 @@ static int xilinxfb_release(struct device *dev) >> > > if (drvdata->fb_alloced) >> > > dma_free_coherent(dev, >> > > PAGE_ALIGN(drvdata->info.fix.smem_len), >> > > drvdata->fb_virt, drvdata->fb_phys); >> > > + else >> > > + iounmap(drvdata->fb_virt); >> > > >> > > /* Turn off the display */ >> > > xilinx_fb_out_be32(drvdata, REG_CTRL, 0); >> > > >> > > - dcr_unmap(drvdata->dcr_host, drvdata->dcr_len); >> > > + /* Release the resources, as allocated based on interface */ >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) { >> > > + iounmap(drvdata->regs); >> > > + release_mem_region(drvdata->regs_phys, 8); >> > > + } else >> > > + dcr_unmap(drvdata->dcr_host, drvdata->dcr_len); >> > > >> > > kfree(drvdata); >> > > dev_set_drvdata(dev, NULL); >> > > @@ -341,27 +397,54 @@ static int xilinxfb_release(struct device *dev) >> > > * OF bus binding >> > > */ >> > > >> > > -#if defined(CONFIG_OF) >> > > static int __devinit >> > > xilinxfb_of_probe(struct of_device *op, const struct of_device_id >> > > *match) >> > > { >> > > const u32 *prop; >> > > + u32 *p; >> > > + u32 tft_access; >> > > struct xilinxfb_platform_data pdata; >> > > + struct resource res; >> > > int size, rc; >> > > - int start, len; >> > > + int start = 0, len = 0; >> > > dcr_host_t dcr_host; >> > > + struct xilinxfb_drvdata *drvdata; >> > > >> > > /* Copy with the default pdata (not a ptr reference!) */ >> > > pdata = xilinx_fb_default_pdata; >> > > >> > > dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match); >> > > >> > > - start = dcr_resource_start(op->node, 0); >> > > - len = dcr_resource_len(op->node, 0); >> > > - dcr_host = dcr_map(op->node, start, len); >> > > - if (!DCR_MAP_OK(dcr_host)) { >> > > - dev_err(&op->dev, "invalid address\n"); >> > > - return -ENODEV; >> > > + /* >> > > + * To check whether the core is connected directly to DCR or PLB >> > > + * interface and initialize the tft_access accordingly. >> > > + */ >> > > + p = (u32 *)of_get_property(op->node, "xlnx,dcr-splb-slave-if", >> > > NULL); >> > > + >> > > + if (p) >> > > + tft_access = *p; >> > > + else >> > > + tft_access = 0; /* For backward compatibility */ >> > > + >> > > + /* >> > > + * Fill the resource structure if its direct PLB interface >> > > + * otherwise fill the dcr_host structure. >> > > + */ >> > > + if (tft_access) { >> > > + rc = of_address_to_resource(op->node, 0, &res); >> > > + if (rc) { >> > > + dev_err(&op->dev, "invalid address\n"); >> > > + return -ENODEV; >> > > + } >> > > + >> > > + } else { >> > > + start = dcr_resource_start(op->node, 0); >> > > + len = dcr_resource_len(op->node, 0); >> > > + dcr_host = dcr_map(op->node, start, len); >> > > + if (!DCR_MAP_OK(dcr_host)) { >> > > + dev_err(&op->dev, "invalid address\n"); >> > > + return -ENODEV; >> > > + } >> > > } >> > > >> > > prop = of_get_property(op->node, "phys-size", &size); >> > > @@ -385,7 +468,26 @@ xilinxfb_of_probe(struct of_device *op, const >> > > struct of_device_id *match) >> > > if (of_find_property(op->node, "rotate-display", NULL)) >> > > pdata.rotate_screen = 1; >> > > >> > > - return xilinxfb_assign(&op->dev, dcr_host, start, len, &pdata); >> > > + /* Allocate the driver data region */ >> > > + drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL); >> > > + if (!drvdata) { >> > > + dev_err(&op->dev, "Couldn't allocate device private >> > > record\n"); >> > > + return -ENOMEM; >> > > + } >> > > + dev_set_drvdata(&op->dev, drvdata); >> > > + >> > > + if (tft_access) >> > > + drvdata->flags |= PLB_ACCESS_FLAG; >> > > + >> > > + /* Arguments are passed based on the interface */ >> > > + if (drvdata->flags & PLB_ACCESS_FLAG) { >> > > + return xilinxfb_assign(&op->dev, drvdata, res.start, >> > > &pdata); >> > > + } else { >> > > + drvdata->dcr_start = start; >> > > + drvdata->dcr_len = len; >> > > + drvdata->dcr_host = dcr_host; >> > > + return xilinxfb_assign(&op->dev, drvdata, 0, &pdata); >> > > + } >> > > } >> > > >> > > static int __devexit xilinxfb_of_remove(struct of_device *op) >> > > @@ -395,6 +497,7 @@ static int __devexit xilinxfb_of_remove(struct >> > > of_device *op) >> > > >> > > /* Match table for of_platform binding */ >> > > static struct of_device_id xilinxfb_of_match[] __devinitdata = { >> > > + { .compatible = "xlnx,xps-tft-1.00.a", }, >> > > { .compatible = "xlnx,plb-tft-cntlr-ref-1.00.a", }, >> > > { .compatible = "xlnx,plb-dvi-cntlr-ref-1.00.c", }, >> > > {}, >> > > @@ -412,22 +515,6 @@ static struct of_platform_driver xilinxfb_of_driver >> > > = { >> > > }, >> > > }; >> > > >> > > -/* Registration helpers to keep the number of #ifdefs to a minimum */ >> > > -static inline int __init xilinxfb_of_register(void) >> > > -{ >> > > - pr_debug("xilinxfb: calling of_register_platform_driver()\n"); >> > > - return of_register_platform_driver(&xilinxfb_of_driver); >> > > -} >> > > - >> > > -static inline void __exit xilinxfb_of_unregister(void) >> > > -{ >> > > - of_unregister_platform_driver(&xilinxfb_of_driver); >> > > -} >> > > -#else /* CONFIG_OF */ >> > > -/* CONFIG_OF not enabled; do nothing helpers */ >> > > -static inline int __init xilinxfb_of_register(void) { return 0; } >> > > -static inline void __exit xilinxfb_of_unregister(void) { } >> > > -#endif /* CONFIG_OF */ >> > > >> > > /* --------------------------------------------------------------------- >> > > * Module setup and teardown >> > > @@ -436,18 +523,18 @@ static inline void __exit >> > > xilinxfb_of_unregister(void) { } >> > > static int __init >> > > xilinxfb_init(void) >> > > { >> > > - return xilinxfb_of_register(); >> > > + return of_register_platform_driver(&xilinxfb_of_driver); >> > > } >> > > >> > > static void __exit >> > > xilinxfb_cleanup(void) >> > > { >> > > - xilinxfb_of_unregister(); >> > > + of_unregister_platform_driver(&xilinxfb_of_driver); >> > > } >> > > >> > > module_init(xilinxfb_init); >> > > module_exit(xilinxfb_cleanup); >> > > >> > > MODULE_AUTHOR("MontaVista Software, Inc. <sou...@mvista.com>"); >> > > -MODULE_DESCRIPTION(DRIVER_DESCRIPTION); >> > > +MODULE_DESCRIPTION("Xilinx TFT frame buffer driver"); >> > > MODULE_LICENSE("GPL"); >> > > -- >> > > 1.6.2.1 >> > > >> > > >> > > >> > > This email and any attachments are intended for the sole use of the >> > > named recipient(s) and >> > contain(s) confidential information that may be proprietary, privileged or >> > copyrighted under >> > applicable law. If you are not the intended recipient, do not read, copy, >> > or forward this email >> > message or any attachments. Delete this email message and any attachments >> > immediately. >> > > >> > > >> > > >> > >> > >> > >> > -- >> > Grant Likely, B.Sc., P.Eng. >> > Secret Lab Technologies Ltd. >> >> >> This email and any attachments are intended for the sole use of the named >> recipient(s) and contain(s) >> confidential information that may be proprietary, privileged or copyrighted >> under applicable law. If >> you are not the intended recipient, do not read, copy, or forward this email >> message or any >> attachments. Delete this email message and any attachments immediately. >> >> >> _______________________________________________ >> Linuxppc-dev mailing list >> Linuxppc-dev@ozlabs.org >> https://ozlabs.org/mailman/listinfo/linuxppc-dev > > > This email and any attachments are intended for the sole use of the named > recipient(s) and contain(s) confidential information that may be proprietary, > privileged or copyrighted under applicable law. If you are not the intended > recipient, do not read, copy, or forward this email message or any > attachments. Delete this email message and any attachments immediately. > > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
