* [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-29 23:49 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-29 23:49 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss, yorksun
MPC5121 DIU configuration/setup as initialized by the boot
loader currently will get lost while booting Linux. As a
result displaying the boot splash is not possible through
the boot process.
To prevent this we reserve configured DIU frame buffer
address range while booting and preserve AOI descriptor
and gamma table so that DIU continues displaying through
the whole boot process. On first open from user space
DIU frame buffer driver releases the reserved frame
buffer area and continues to operate as usual.
Signed-off-by: John Rigby <jrigby@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
Changes since previous patch version:
- split out moving fsl-diu-fb.h file to separate patch
- drop unrelated changes
- remove __init annotations in header file
- drop unneeded code
- don't hardcode default busfreq, error out
forcing users to supply a valid tree
- simplify pixelclock calculation
arch/powerpc/platforms/512x/mpc5121_ads.c | 7 +
arch/powerpc/platforms/512x/mpc5121_generic.c | 12 +
arch/powerpc/platforms/512x/mpc512x.h | 2 +
arch/powerpc/platforms/512x/mpc512x_shared.c | 284 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 1 +
drivers/video/fsl-diu-fb.c | 17 ++-
6 files changed, 321 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index ee6ae12..aa4d5a8 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -42,6 +42,7 @@ static void __init mpc5121_ads_setup_arch(void)
for_each_compatible_node(np, "pci", "fsl,mpc5121-pci")
mpc83xx_add_bridge(np);
#endif
+ mpc512x_setup_diu();
}
static void __init mpc5121_ads_init_IRQ(void)
@@ -60,11 +61,17 @@ static int __init mpc5121_ads_probe(void)
return of_flat_dt_is_compatible(root, "fsl,mpc5121ads");
}
+void __init mpc5121_ads_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
define_machine(mpc5121_ads) {
.name = "MPC5121 ADS",
.probe = mpc5121_ads_probe,
.setup_arch = mpc5121_ads_setup_arch,
.init = mpc512x_init,
+ .init_early = mpc5121_ads_init_early,
.init_IRQ = mpc5121_ads_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index a6c0e3a..c5ecb3d 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -48,10 +48,22 @@ static int __init mpc5121_generic_probe(void)
return board[i] != NULL;
}
+void __init mpc512x_generic_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
+void __init mpc512x_generic_setup_arch(void)
+{
+ mpc512x_setup_diu();
+}
+
define_machine(mpc5121_generic) {
.name = "MPC5121 generic",
.probe = mpc5121_generic_probe,
.init = mpc512x_init,
+ .init_early = mpc512x_generic_init_early,
+ .setup_arch = mpc512x_generic_setup_arch,
.init_IRQ = mpc512x_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..1ab6d11 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
extern int __init mpc5121_clk_init(void);
void __init mpc512x_declare_of_platform_devices(void);
extern void mpc512x_restart(char *cmd);
+extern void mpc512x_init_diu(void);
+extern void mpc512x_setup_diu(void);
#endif /* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b7f518a..8e297fa 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -16,7 +16,11 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/of_platform.h>
+#include <linux/fsl-diu-fb.h>
+#include <linux/bootmem.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/cacheflush.h>
#include <asm/machdep.h>
#include <asm/ipic.h>
#include <asm/prom.h>
@@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
;
}
+struct fsl_diu_shared_fb {
+ char gamma[0x300]; /* 32-bit aligned! */
+ struct diu_ad ad0; /* 32-bit aligned! */
+ phys_addr_t fb_phys;
+ size_t fb_len;
+ bool in_use;
+};
+
+unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
+ int monitor_port)
+{
+ unsigned int pix_fmt;
+
+ switch (bits_per_pixel) {
+ case 32:
+ pix_fmt = 0x88883316;
+ break;
+ case 24:
+ pix_fmt = 0x88082219;
+ break;
+ case 16:
+ pix_fmt = 0x65053118;
+ break;
+ default:
+ pix_fmt = 0x00000400;
+ }
+ return pix_fmt;
+}
+
+void mpc512x_set_gamma_table(int monitor_port, char *gamma_table_base)
+{
+}
+
+void mpc512x_set_monitor_port(int monitor_port)
+{
+}
+
+#define CCM_SCFR1 0x0000000c
+#define DIU_DIV_MASK 0x000000ff
+void mpc512x_set_pixel_clock(unsigned int pixclock)
+{
+ unsigned long bestval, bestfreq, speed_ccb, busfreq;
+ unsigned long minpixclock, maxpixclock, pixval;
+ struct device_node *np;
+ void __iomem *ccm;
+ u32 temp;
+ long err;
+ int i;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-clock");
+ if (!np) {
+ pr_err("Can't find clock control module.\n");
+ return;
+ }
+
+ ccm = of_iomap(np, 0);
+ if (!ccm) {
+ pr_err("Can't map clock control module reg.\n");
+ of_node_put(np);
+ return;
+ }
+ of_node_put(np);
+
+ np = of_find_node_by_type(NULL, "cpu");
+ if (np) {
+ unsigned int size;
+ const unsigned int *prop =
+ of_get_property(np, "bus-frequency", &size);
+
+ of_node_put(np);
+ if (prop) {
+ busfreq = *prop;
+ } else {
+ pr_err("Can't get bus-frequency property\n");
+ return;
+ }
+ } else {
+ pr_err("Can't find \"cpu\" node.\n");
+ return;
+ }
+
+ /* Pixel Clock configuration */
+ pr_debug("DIU: Bus Frequency = %lu\n", busfreq);
+ speed_ccb = busfreq * 4;
+
+ /* Calculate the pixel clock with the smallest error */
+ /* calculate the following in steps to avoid overflow */
+ pr_debug("DIU pixclock in ps - %d\n", pixclock);
+ temp = (1000000000 / pixclock) * 1000;
+ pixclock = temp;
+ pr_debug("DIU pixclock freq - %u\n", pixclock);
+
+ temp = (temp * 5) / 100; /* pixclock * 0.05 */
+ pr_debug("deviation = %d\n", temp);
+ minpixclock = pixclock - temp;
+ maxpixclock = pixclock + temp;
+ pr_debug("DIU minpixclock - %lu\n", minpixclock);
+ pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
+ pixval = speed_ccb/pixclock;
+ pr_debug("DIU pixval = %lu\n", pixval);
+
+ err = 100000000;
+ bestval = pixval;
+ pr_debug("DIU bestval = %lu\n", bestval);
+
+ bestfreq = 0;
+ for (i = -1; i <= 1; i++) {
+ temp = speed_ccb / (pixval+i);
+ pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
+ i, pixval, temp);
+ if ((temp < minpixclock) || (temp > maxpixclock))
+ pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
+ minpixclock, maxpixclock);
+ else if (abs(temp - pixclock) < err) {
+ pr_debug("Entered the else if block %d\n", i);
+ err = abs(temp - pixclock);
+ bestval = pixval + i;
+ bestfreq = temp;
+ }
+ }
+
+ pr_debug("DIU chose = %lx\n", bestval);
+ pr_debug("DIU error = %ld\n NomPixClk ", err);
+ pr_debug("DIU: Best Freq = %lx\n", bestfreq);
+ /* Modify DIU_DIV in CCM SCFR1 */
+ temp = in_be32(ccm + CCM_SCFR1);
+ pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
+ temp &= ~DIU_DIV_MASK;
+ temp |= (bestval & DIU_DIV_MASK);
+ out_be32(ccm + CCM_SCFR1, temp);
+ pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
+ iounmap(ccm);
+}
+
+ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
+}
+
+int mpc512x_set_sysfs_monitor_port(int val)
+{
+ return 0;
+}
+
+static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+static inline void mpc512x_free_bootmem(struct page *page)
+{
+ __ClearPageReserved(page);
+ BUG_ON(PageTail(page));
+ BUG_ON(atomic_read(&page->_count) > 1);
+ atomic_set(&page->_count, 1);
+ __free_page(page);
+ totalram_pages++;
+}
+
+void mpc512x_release_bootmem(void)
+{
+ unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
+ unsigned long size = diu_shared_fb.fb_len;
+ unsigned long start, end;
+
+ if (diu_shared_fb.in_use) {
+ start = PFN_UP(addr);
+ end = PFN_DOWN(addr + size);
+
+ for (; start < end; start++)
+ mpc512x_free_bootmem(pfn_to_page(start));
+
+ diu_shared_fb.in_use = false;
+ }
+ diu_ops.release_bootmem = NULL;
+}
+#endif
+
+/*
+ * Check if DIU was pre-initialized. If so, perform steps
+ * needed to continue displaying through the whole boot process.
+ * Move area descriptor and gamma table elsewhere, they are
+ * destroyed by bootmem allocator otherwise. The frame buffer
+ * address range will be reserved in setup_arch() after bootmem
+ * allocator is up.
+ */
+void __init mpc512x_init_diu(void)
+{
+ struct device_node *np;
+ void __iomem *diu_reg;
+ phys_addr_t desc;
+ void __iomem *vaddr;
+ unsigned long mode, pix_fmt, res, bpp;
+ unsigned long dst;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
+ if (!np) {
+ pr_err("No DIU node\n");
+ return;
+ }
+
+ diu_reg = of_iomap(np, 0);
+ of_node_put(np);
+ if (!diu_reg) {
+ pr_err("Can't map DIU\n");
+ return;
+ }
+
+ mode = in_be32(diu_reg + 0x1c);
+ if (mode != 1) {
+ pr_info("%s: DIU OFF\n", __func__);
+ goto out;
+ }
+
+ desc = in_be32(diu_reg);
+ vaddr = ioremap(desc, sizeof(struct diu_ad));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ goto out;
+ }
+ memcpy(&diu_shared_fb.ad0, vaddr, sizeof(struct diu_ad));
+ /* flush fb area descriptor */
+ dst = (unsigned long)&diu_shared_fb.ad0;
+ flush_dcache_range(dst, dst + sizeof(struct diu_ad) - 1);
+
+ res = in_be32(diu_reg + 0x28);
+ pix_fmt = in_le32(vaddr);
+ bpp = ((pix_fmt >> 16) & 0x3) + 1;
+ diu_shared_fb.fb_phys = in_le32(vaddr + 4);
+ diu_shared_fb.fb_len = ((res & 0xfff0000) >> 16) * (res & 0xfff) * bpp;
+ diu_shared_fb.in_use = true;
+ iounmap(vaddr);
+
+ desc = in_be32(diu_reg + 0xc);
+ vaddr = ioremap(desc, sizeof(diu_shared_fb.gamma));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ diu_shared_fb.in_use = false;
+ goto out;
+ }
+ memcpy(&diu_shared_fb.gamma, vaddr, sizeof(diu_shared_fb.gamma));
+ /* flush gamma table */
+ dst = (unsigned long)&diu_shared_fb.gamma;
+ flush_dcache_range(dst, dst + sizeof(diu_shared_fb.gamma) - 1);
+
+ iounmap(vaddr);
+ out_be32(diu_reg + 0xc, virt_to_phys(&diu_shared_fb.gamma));
+ out_be32(diu_reg + 4, 0);
+ out_be32(diu_reg + 8, 0);
+ out_be32(diu_reg, virt_to_phys(&diu_shared_fb.ad0));
+
+out:
+ iounmap(diu_reg);
+}
+
+void __init mpc512x_setup_diu(void)
+{
+ int ret;
+
+ if (diu_shared_fb.in_use) {
+ ret = reserve_bootmem(diu_shared_fb.fb_phys,
+ diu_shared_fb.fb_len,
+ BOOTMEM_EXCLUSIVE);
+ if (ret) {
+ pr_err("%s: reserve bootmem failed\n", __func__);
+ diu_shared_fb.in_use = false;
+ }
+ }
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+ diu_ops.get_pixel_format = mpc512x_get_pixel_format;
+ diu_ops.set_gamma_table = mpc512x_set_gamma_table;
+ diu_ops.set_monitor_port = mpc512x_set_monitor_port;
+ diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
+ diu_ops.show_monitor_port = mpc512x_show_monitor_port;
+ diu_ops.set_sysfs_monitor_port = mpc512x_set_sysfs_monitor_port;
+ diu_ops.release_bootmem = mpc512x_release_bootmem;
+#endif
+}
+
void __init mpc512x_init_IRQ(void)
{
struct device_node *np;
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 42381bb..5360948 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -30,6 +30,7 @@ struct platform_diu_data_ops {
void (*set_pixel_clock) (unsigned int pixclock);
ssize_t (*show_monitor_port) (int monitor_port, char *buf);
int (*set_sysfs_monitor_port) (int val);
+ void (*release_bootmem) (void);
};
extern struct platform_diu_data_ops diu_ops;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 7acdc09..81dec09 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1103,6 +1103,10 @@ static int fsl_diu_open(struct fb_info *info, int user)
struct mfb_info *mfbi = info->par;
int res = 0;
+ /* free boot splash memory on first /dev/fb0 open */
+ if (!mfbi->index && diu_ops.release_bootmem)
+ diu_ops.release_bootmem();
+
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count == 1) {
@@ -1430,6 +1434,7 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
int ret, i, error = 0;
struct resource res;
struct fsl_diu_data *machine_data;
+ int diu_mode;
machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
if (!machine_data)
@@ -1466,7 +1471,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
goto error2;
}
- out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
+ diu_mode = in_be32(&dr.diu_reg->diu_mode);
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
/* Get the IRQ of the DIU */
machine_data->irq = irq_of_parse_and_map(np, 0);
@@ -1514,7 +1521,13 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
machine_data->dummy_ad->offset_xyd = 0;
machine_data->dummy_ad->next_ad = 0;
- out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+ /*
+ * Let DIU display splash screen if it was pre-initialized
+ * by the bootloader, set dummy area descriptor otherwise.
+ */
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+
out_be32(&dr.diu_reg->desc[1], machine_data->dummy_ad->paddr);
out_be32(&dr.diu_reg->desc[2], machine_data->dummy_ad->paddr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-29 23:49 ` Anatolij Gustschin
(?)
@ 2010-04-30 2:05 ` Timur Tabi
-1 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 2:05 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
> +void __init mpc5121_ads_init_early(void)
> +{
> + mpc512x_init_diu();
> +}
> +
> define_machine(mpc5121_ads) {
> .name = "MPC5121 ADS",
> .probe = mpc5121_ads_probe,
> .setup_arch = mpc5121_ads_setup_arch,
> .init = mpc512x_init,
> + .init_early = mpc5121_ads_init_early,
How about just doing this?
.init_early = mpc512x_init_diu,
> +void __init mpc512x_generic_init_early(void)
> +{
> + mpc512x_init_diu();
> +}
> +
> +void __init mpc512x_generic_setup_arch(void)
> +{
> + mpc512x_setup_diu();
> +}
> +
> define_machine(mpc5121_generic) {
> .name = "MPC5121 generic",
> .probe = mpc5121_generic_probe,
> .init = mpc512x_init,
> + .init_early = mpc512x_generic_init_early,
> + .setup_arch = mpc512x_generic_setup_arch,
And a similar change here.
> .init_IRQ = mpc512x_init_IRQ,
> .get_irq = ipic_get_irq,
> .calibrate_decr = generic_calibrate_decr,
> diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
> index b2daca0..1ab6d11 100644
> --- a/arch/powerpc/platforms/512x/mpc512x.h
> +++ b/arch/powerpc/platforms/512x/mpc512x.h
> @@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
> extern int __init mpc5121_clk_init(void);
> void __init mpc512x_declare_of_platform_devices(void);
> extern void mpc512x_restart(char *cmd);
> +extern void mpc512x_init_diu(void);
> +extern void mpc512x_setup_diu(void);
> #endif /* __MPC512X_H__ */
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index b7f518a..8e297fa 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -16,7 +16,11 @@
> #include <linux/io.h>
> #include <linux/irq.h>
> #include <linux/of_platform.h>
> +#include <linux/fsl-diu-fb.h>
> +#include <linux/bootmem.h>
> +#include <sysdev/fsl_soc.h>
>
> +#include <asm/cacheflush.h>
> #include <asm/machdep.h>
> #include <asm/ipic.h>
> #include <asm/prom.h>
> @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> ;
> }
>
> +struct fsl_diu_shared_fb {
> + char gamma[0x300]; /* 32-bit aligned! */
char or u8?
> + struct diu_ad ad0; /* 32-bit aligned! */
> + phys_addr_t fb_phys;
> + size_t fb_len;
> + bool in_use;
> +};
Where did "bool" come from? Use "int" instead.
> +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> + int monitor_port)
> +{
> + unsigned int pix_fmt;
> +
> + switch (bits_per_pixel) {
> + case 32:
> + pix_fmt = 0x88883316;
> + break;
> + case 24:
> + pix_fmt = 0x88082219;
> + break;
> + case 16:
> + pix_fmt = 0x65053118;
> + break;
> + default:
> + pix_fmt = 0x00000400;
> + }
> + return pix_fmt;
> +}
This is simpler:
switch (bits_per_pixel) {
case 32:
return 0x88883316;
case 24:
return 0x88082219;
case 16:
return = 0x65053118;
}
return 0x00000400;
}
> + ccm = of_iomap(np, 0);
> + if (!ccm) {
> + pr_err("Can't map clock control module reg.\n");
> + of_node_put(np);
> + return;
> + }
> + of_node_put(np);
This is simpler:
ccm = of_iomap(np, 0);
of_node_put(np);
if (!ccm) {
pr_err("Can't map clock control module reg.\n");
return;
}
> + np = of_find_node_by_type(NULL, "cpu");
> + if (np) {
> + unsigned int size;
> + const unsigned int *prop > + of_get_property(np, "bus-frequency", &size);
Since you don't use 'size', you can skip it:
const unsigned int *prop of_get_property(np, "bus-frequency", NULL);
> + } else {
> + pr_err("Can't find \"cpu\" node.\n");
'cpu' is simpler than \"cpu\"
> + /* Calculate the pixel clock with the smallest error */
> + /* calculate the following in steps to avoid overflow */
> + pr_debug("DIU pixclock in ps - %d\n", pixclock);
> + temp = (1000000000 / pixclock) * 1000;
I'm pretty sure the compiler will optimize this to:
temp = (1000000000000UL / pixclock);
so you may as well do it that way.
> + pixclock = temp;
> + pr_debug("DIU pixclock freq - %u\n", pixclock);
> +
> + temp = (temp * 5) / 100; /* pixclock * 0.05 */
The compiler will optimize this to:
temp /= 20;
> + pr_debug("deviation = %d\n", temp);
> + minpixclock = pixclock - temp;
> + maxpixclock = pixclock + temp;
> + pr_debug("DIU minpixclock - %lu\n", minpixclock);
> + pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> + pixval = speed_ccb/pixclock;
> + pr_debug("DIU pixval = %lu\n", pixval);
> +
> + err = 100000000;
Why do you assign err to this arbitrary value?
> + bestval = pixval;
> + pr_debug("DIU bestval = %lu\n", bestval);
> +
> + bestfreq = 0;
> + for (i = -1; i <= 1; i++) {
> + temp = speed_ccb / (pixval+i);
> + pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
> + i, pixval, temp);
> + if ((temp < minpixclock) || (temp > maxpixclock))
> + pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
> + minpixclock, maxpixclock);
> + else if (abs(temp - pixclock) < err) {
> + pr_debug("Entered the else if block %d\n", i);
> + err = abs(temp - pixclock);
> + bestval = pixval + i;
> + bestfreq = temp;
> + }
> + }
> +
> + pr_debug("DIU chose = %lx\n", bestval);
> + pr_debug("DIU error = %ld\n NomPixClk ", err);
> + pr_debug("DIU: Best Freq = %lx\n", bestfreq);
> + /* Modify DIU_DIV in CCM SCFR1 */
> + temp = in_be32(ccm + CCM_SCFR1);
Don't use offsets like + CCM_SCFR1. Create a structure and use that instead.
> + pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
> + temp &= ~DIU_DIV_MASK;
> + temp |= (bestval & DIU_DIV_MASK);
> + out_be32(ccm + CCM_SCFR1, temp);
> + pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
> + iounmap(ccm);
> +}
> +
> +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
There's no point in using snprintf since you're printing a string
literal. You can use sprintf.
> +}
> +
> +int mpc512x_set_sysfs_monitor_port(int val)
> +{
> + return 0;
> +}
> +
> +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
> +
> +#if defined(CONFIG_FB_FSL_DIU) || \
> + defined(CONFIG_FB_FSL_DIU_MODULE)
> +static inline void mpc512x_free_bootmem(struct page *page)
> +{
> + __ClearPageReserved(page);
> + BUG_ON(PageTail(page));
> + BUG_ON(atomic_read(&page->_count) > 1);
> + atomic_set(&page->_count, 1);
> + __free_page(page);
> + totalram_pages++;
> +}
> +
> +void mpc512x_release_bootmem(void)
> +{
> + unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
> + unsigned long size = diu_shared_fb.fb_len;
> + unsigned long start, end;
> +
> + if (diu_shared_fb.in_use) {
> + start = PFN_UP(addr);
> + end = PFN_DOWN(addr + size);
> +
> + for (; start < end; start++)
> + mpc512x_free_bootmem(pfn_to_page(start));
> +
> + diu_shared_fb.in_use = false;
> + }
> + diu_ops.release_bootmem = NULL;
> +}
> +#endif
Do you really need to use reserve_bootmem? Have you tried kmalloc or
alloc_pages_exact()?
> +
> +/*
> + * Check if DIU was pre-initialized. If so, perform steps
> + * needed to continue displaying through the whole boot process.
> + * Move area descriptor and gamma table elsewhere, they are
> + * destroyed by bootmem allocator otherwise. The frame buffer
> + * address range will be reserved in setup_arch() after bootmem
> + * allocator is up.
> + */
> +void __init mpc512x_init_diu(void)
> +{
> + struct device_node *np;
> + void __iomem *diu_reg;
> + phys_addr_t desc;
> + void __iomem *vaddr;
> + unsigned long mode, pix_fmt, res, bpp;
> + unsigned long dst;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
> + if (!np) {
> + pr_err("No DIU node\n");
> + return;
> + }
Shouldn't you be probing as an OF driver instead of manually searching
for the DIU node?
> +
> + diu_reg = of_iomap(np, 0);
> + of_node_put(np);
> + if (!diu_reg) {
> + pr_err("Can't map DIU\n");
> + return;
> + }
> +
> + mode = in_be32(diu_reg + 0x1c);
> + if (mode != 1) {
How can in_be32() return a -1?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 2:05 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 2:05 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
> +void __init mpc5121_ads_init_early(void)
> +{
> + mpc512x_init_diu();
> +}
> +
> define_machine(mpc5121_ads) {
> .name = "MPC5121 ADS",
> .probe = mpc5121_ads_probe,
> .setup_arch = mpc5121_ads_setup_arch,
> .init = mpc512x_init,
> + .init_early = mpc5121_ads_init_early,
How about just doing this?
.init_early = mpc512x_init_diu,
> +void __init mpc512x_generic_init_early(void)
> +{
> + mpc512x_init_diu();
> +}
> +
> +void __init mpc512x_generic_setup_arch(void)
> +{
> + mpc512x_setup_diu();
> +}
> +
> define_machine(mpc5121_generic) {
> .name = "MPC5121 generic",
> .probe = mpc5121_generic_probe,
> .init = mpc512x_init,
> + .init_early = mpc512x_generic_init_early,
> + .setup_arch = mpc512x_generic_setup_arch,
And a similar change here.
> .init_IRQ = mpc512x_init_IRQ,
> .get_irq = ipic_get_irq,
> .calibrate_decr = generic_calibrate_decr,
> diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
> index b2daca0..1ab6d11 100644
> --- a/arch/powerpc/platforms/512x/mpc512x.h
> +++ b/arch/powerpc/platforms/512x/mpc512x.h
> @@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
> extern int __init mpc5121_clk_init(void);
> void __init mpc512x_declare_of_platform_devices(void);
> extern void mpc512x_restart(char *cmd);
> +extern void mpc512x_init_diu(void);
> +extern void mpc512x_setup_diu(void);
> #endif /* __MPC512X_H__ */
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index b7f518a..8e297fa 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -16,7 +16,11 @@
> #include <linux/io.h>
> #include <linux/irq.h>
> #include <linux/of_platform.h>
> +#include <linux/fsl-diu-fb.h>
> +#include <linux/bootmem.h>
> +#include <sysdev/fsl_soc.h>
>
> +#include <asm/cacheflush.h>
> #include <asm/machdep.h>
> #include <asm/ipic.h>
> #include <asm/prom.h>
> @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> ;
> }
>
> +struct fsl_diu_shared_fb {
> + char gamma[0x300]; /* 32-bit aligned! */
char or u8?
> + struct diu_ad ad0; /* 32-bit aligned! */
> + phys_addr_t fb_phys;
> + size_t fb_len;
> + bool in_use;
> +};
Where did "bool" come from? Use "int" instead.
> +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> + int monitor_port)
> +{
> + unsigned int pix_fmt;
> +
> + switch (bits_per_pixel) {
> + case 32:
> + pix_fmt = 0x88883316;
> + break;
> + case 24:
> + pix_fmt = 0x88082219;
> + break;
> + case 16:
> + pix_fmt = 0x65053118;
> + break;
> + default:
> + pix_fmt = 0x00000400;
> + }
> + return pix_fmt;
> +}
This is simpler:
switch (bits_per_pixel) {
case 32:
return 0x88883316;
case 24:
return 0x88082219;
case 16:
return = 0x65053118;
}
return 0x00000400;
}
> + ccm = of_iomap(np, 0);
> + if (!ccm) {
> + pr_err("Can't map clock control module reg.\n");
> + of_node_put(np);
> + return;
> + }
> + of_node_put(np);
This is simpler:
ccm = of_iomap(np, 0);
of_node_put(np);
if (!ccm) {
pr_err("Can't map clock control module reg.\n");
return;
}
> + np = of_find_node_by_type(NULL, "cpu");
> + if (np) {
> + unsigned int size;
> + const unsigned int *prop =
> + of_get_property(np, "bus-frequency", &size);
Since you don't use 'size', you can skip it:
const unsigned int *prop =
of_get_property(np, "bus-frequency", NULL);
> + } else {
> + pr_err("Can't find \"cpu\" node.\n");
'cpu' is simpler than \"cpu\"
> + /* Calculate the pixel clock with the smallest error */
> + /* calculate the following in steps to avoid overflow */
> + pr_debug("DIU pixclock in ps - %d\n", pixclock);
> + temp = (1000000000 / pixclock) * 1000;
I'm pretty sure the compiler will optimize this to:
temp = (1000000000000UL / pixclock);
so you may as well do it that way.
> + pixclock = temp;
> + pr_debug("DIU pixclock freq - %u\n", pixclock);
> +
> + temp = (temp * 5) / 100; /* pixclock * 0.05 */
The compiler will optimize this to:
temp /= 20;
> + pr_debug("deviation = %d\n", temp);
> + minpixclock = pixclock - temp;
> + maxpixclock = pixclock + temp;
> + pr_debug("DIU minpixclock - %lu\n", minpixclock);
> + pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> + pixval = speed_ccb/pixclock;
> + pr_debug("DIU pixval = %lu\n", pixval);
> +
> + err = 100000000;
Why do you assign err to this arbitrary value?
> + bestval = pixval;
> + pr_debug("DIU bestval = %lu\n", bestval);
> +
> + bestfreq = 0;
> + for (i = -1; i <= 1; i++) {
> + temp = speed_ccb / (pixval+i);
> + pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
> + i, pixval, temp);
> + if ((temp < minpixclock) || (temp > maxpixclock))
> + pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
> + minpixclock, maxpixclock);
> + else if (abs(temp - pixclock) < err) {
> + pr_debug("Entered the else if block %d\n", i);
> + err = abs(temp - pixclock);
> + bestval = pixval + i;
> + bestfreq = temp;
> + }
> + }
> +
> + pr_debug("DIU chose = %lx\n", bestval);
> + pr_debug("DIU error = %ld\n NomPixClk ", err);
> + pr_debug("DIU: Best Freq = %lx\n", bestfreq);
> + /* Modify DIU_DIV in CCM SCFR1 */
> + temp = in_be32(ccm + CCM_SCFR1);
Don't use offsets like + CCM_SCFR1. Create a structure and use that instead.
> + pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
> + temp &= ~DIU_DIV_MASK;
> + temp |= (bestval & DIU_DIV_MASK);
> + out_be32(ccm + CCM_SCFR1, temp);
> + pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
> + iounmap(ccm);
> +}
> +
> +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
There's no point in using snprintf since you're printing a string
literal. You can use sprintf.
> +}
> +
> +int mpc512x_set_sysfs_monitor_port(int val)
> +{
> + return 0;
> +}
> +
> +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
> +
> +#if defined(CONFIG_FB_FSL_DIU) || \
> + defined(CONFIG_FB_FSL_DIU_MODULE)
> +static inline void mpc512x_free_bootmem(struct page *page)
> +{
> + __ClearPageReserved(page);
> + BUG_ON(PageTail(page));
> + BUG_ON(atomic_read(&page->_count) > 1);
> + atomic_set(&page->_count, 1);
> + __free_page(page);
> + totalram_pages++;
> +}
> +
> +void mpc512x_release_bootmem(void)
> +{
> + unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
> + unsigned long size = diu_shared_fb.fb_len;
> + unsigned long start, end;
> +
> + if (diu_shared_fb.in_use) {
> + start = PFN_UP(addr);
> + end = PFN_DOWN(addr + size);
> +
> + for (; start < end; start++)
> + mpc512x_free_bootmem(pfn_to_page(start));
> +
> + diu_shared_fb.in_use = false;
> + }
> + diu_ops.release_bootmem = NULL;
> +}
> +#endif
Do you really need to use reserve_bootmem? Have you tried kmalloc or
alloc_pages_exact()?
> +
> +/*
> + * Check if DIU was pre-initialized. If so, perform steps
> + * needed to continue displaying through the whole boot process.
> + * Move area descriptor and gamma table elsewhere, they are
> + * destroyed by bootmem allocator otherwise. The frame buffer
> + * address range will be reserved in setup_arch() after bootmem
> + * allocator is up.
> + */
> +void __init mpc512x_init_diu(void)
> +{
> + struct device_node *np;
> + void __iomem *diu_reg;
> + phys_addr_t desc;
> + void __iomem *vaddr;
> + unsigned long mode, pix_fmt, res, bpp;
> + unsigned long dst;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
> + if (!np) {
> + pr_err("No DIU node\n");
> + return;
> + }
Shouldn't you be probing as an OF driver instead of manually searching
for the DIU node?
> +
> + diu_reg = of_iomap(np, 0);
> + of_node_put(np);
> + if (!diu_reg) {
> + pr_err("Can't map DIU\n");
> + return;
> + }
> +
> + mode = in_be32(diu_reg + 0x1c);
> + if (mode != 1) {
How can in_be32() return a -1?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 2:05 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 2:05 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
> +void __init mpc5121_ads_init_early(void)
> +{
> + =A0 =A0 =A0 mpc512x_init_diu();
> +}
> +
> =A0define_machine(mpc5121_ads) {
> =A0 =A0 =A0 =A0.name =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D "MPC5121 ADS=
",
> =A0 =A0 =A0 =A0.probe =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc5121_ads_=
probe,
> =A0 =A0 =A0 =A0.setup_arch =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc5121_ads_setup_=
arch,
> =A0 =A0 =A0 =A0.init =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_init=
,
> + =A0 =A0 =A0 .init_early =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc5121_ads_init_ea=
rly,
How about just doing this?
.init_early =3D mpc512x_init_diu,
> +void __init mpc512x_generic_init_early(void)
> +{
> + =A0 =A0 =A0 mpc512x_init_diu();
> +}
> +
> +void __init mpc512x_generic_setup_arch(void)
> +{
> + =A0 =A0 =A0 mpc512x_setup_diu();
> +}
> +
> =A0define_machine(mpc5121_generic) {
> =A0 =A0 =A0 =A0.name =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D "MPC5121 gen=
eric",
> =A0 =A0 =A0 =A0.probe =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc5121_gene=
ric_probe,
> =A0 =A0 =A0 =A0.init =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_init=
,
> + =A0 =A0 =A0 .init_early =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_generic_ini=
t_early,
> + =A0 =A0 =A0 .setup_arch =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_generic_set=
up_arch,
And a similar change here.
> =A0 =A0 =A0 =A0.init_IRQ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_init_IRQ=
,
> =A0 =A0 =A0 =A0.get_irq =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D ipic_get_irq,
> =A0 =A0 =A0 =A0.calibrate_decr =A0 =A0 =A0 =A0 =3D generic_calibrate_decr=
,
> diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platfor=
ms/512x/mpc512x.h
> index b2daca0..1ab6d11 100644
> --- a/arch/powerpc/platforms/512x/mpc512x.h
> +++ b/arch/powerpc/platforms/512x/mpc512x.h
> @@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
> =A0extern int __init mpc5121_clk_init(void);
> =A0void __init mpc512x_declare_of_platform_devices(void);
> =A0extern void mpc512x_restart(char *cmd);
> +extern void mpc512x_init_diu(void);
> +extern void mpc512x_setup_diu(void);
> =A0#endif =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* __MPC512X_H_=
_ */
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/=
platforms/512x/mpc512x_shared.c
> index b7f518a..8e297fa 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -16,7 +16,11 @@
> =A0#include <linux/io.h>
> =A0#include <linux/irq.h>
> =A0#include <linux/of_platform.h>
> +#include <linux/fsl-diu-fb.h>
> +#include <linux/bootmem.h>
> +#include <sysdev/fsl_soc.h>
>
> +#include <asm/cacheflush.h>
> =A0#include <asm/machdep.h>
> =A0#include <asm/ipic.h>
> =A0#include <asm/prom.h>
> @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0;
> =A0}
>
> +struct fsl_diu_shared_fb {
> + =A0 =A0 =A0 char =A0 =A0 =A0 =A0 =A0 =A0gamma[0x300]; =A0 /* 32-bit ali=
gned! */
char or u8?
> + =A0 =A0 =A0 struct diu_ad =A0 ad0; =A0 =A0 =A0 =A0 =A0 =A0/* 32-bit ali=
gned! */
> + =A0 =A0 =A0 phys_addr_t =A0 =A0 fb_phys;
> + =A0 =A0 =A0 size_t =A0 =A0 =A0 =A0 =A0fb_len;
> + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0in_use;
> +};
Where did "bool" come from? Use "int" instead.
> +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
int monitor_port)
> +{
> + =A0 =A0 =A0 unsigned int pix_fmt;
> +
> + =A0 =A0 =A0 switch (bits_per_pixel) {
> + =A0 =A0 =A0 case 32:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pix_fmt =3D 0x88883316;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 case 24:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pix_fmt =3D 0x88082219;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 case 16:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pix_fmt =3D 0x65053118;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 default:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pix_fmt =3D 0x00000400;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 return pix_fmt;
> +}
This is simpler:
switch (bits_per_pixel) {
case 32:
return 0x88883316;
case 24:
return 0x88082219;
case 16:
return =3D 0x65053118;
}
return 0x00000400;
}
> + =A0 =A0 =A0 ccm =3D of_iomap(np, 0);
> + =A0 =A0 =A0 if (!ccm) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("Can't map clock control module reg.=
\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 of_node_put(np);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 of_node_put(np);
This is simpler:
ccm =3D of_iomap(np, 0);
of_node_put(np);
if (!ccm) {
pr_err("Can't map clock control module reg.\n");
return;
}
> + =A0 =A0 =A0 np =3D of_find_node_by_type(NULL, "cpu");
> + =A0 =A0 =A0 if (np) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 unsigned int size;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 const unsigned int *prop =3D
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 of_get_property(np, "bus-fr=
equency", &size);
Since you don't use 'size', you can skip it:
const unsigned int *prop =3D
of_get_property(np, "bus-frequency", NULL);
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("Can't find \"cpu\" node.\n");
'cpu' is simpler than \"cpu\"
> + =A0 =A0 =A0 /* Calculate the pixel clock with the smallest error */
> + =A0 =A0 =A0 /* calculate the following in steps to avoid overflow */
> + =A0 =A0 =A0 pr_debug("DIU pixclock in ps - %d\n", pixclock);
> + =A0 =A0 =A0 temp =3D (1000000000 / pixclock) * 1000;
I'm pretty sure the compiler will optimize this to:
temp =3D (1000000000000UL / pixclock);
so you may as well do it that way.
> + =A0 =A0 =A0 pixclock =3D temp;
> + =A0 =A0 =A0 pr_debug("DIU pixclock freq - %u\n", pixclock);
> +
> + =A0 =A0 =A0 temp =3D (temp * 5) / 100; /* pixclock * 0.05 */
The compiler will optimize this to:
temp /=3D 20;
> + =A0 =A0 =A0 pr_debug("deviation =3D %d\n", temp);
> + =A0 =A0 =A0 minpixclock =3D pixclock - temp;
> + =A0 =A0 =A0 maxpixclock =3D pixclock + temp;
> + =A0 =A0 =A0 pr_debug("DIU minpixclock - %lu\n", minpixclock);
> + =A0 =A0 =A0 pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> + =A0 =A0 =A0 pixval =3D speed_ccb/pixclock;
> + =A0 =A0 =A0 pr_debug("DIU pixval =3D %lu\n", pixval);
> +
> + =A0 =A0 =A0 err =3D 100000000;
Why do you assign err to this arbitrary value?
> + =A0 =A0 =A0 bestval =3D pixval;
> + =A0 =A0 =A0 pr_debug("DIU bestval =3D %lu\n", bestval);
> +
> + =A0 =A0 =A0 bestfreq =3D 0;
> + =A0 =A0 =A0 for (i =3D -1; i <=3D 1; i++) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 temp =3D speed_ccb / (pixval+i);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_debug("DIU test pixval i=3D%d, pixval=3D=
%lu, temp freq. =3D %u\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 i, pixval, temp);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((temp < minpixclock) || (temp > maxpixc=
lock))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_debug("DIU exceeds monit=
or range (%lu to %lu)\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 minpixclock=
, maxpixclock);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (abs(temp - pixclock) < err) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_debug("Entered the else =
if block %d\n", i);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D abs(temp - pixclock=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bestval =3D pixval + i;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bestfreq =3D temp;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 pr_debug("DIU chose =3D %lx\n", bestval);
> + =A0 =A0 =A0 pr_debug("DIU error =3D %ld\n NomPixClk ", err);
> + =A0 =A0 =A0 pr_debug("DIU: Best Freq =3D %lx\n", bestfreq);
> + =A0 =A0 =A0 /* Modify DIU_DIV in CCM SCFR1 */
> + =A0 =A0 =A0 temp =3D in_be32(ccm + CCM_SCFR1);
Don't use offsets like + CCM_SCFR1. Create a structure and use that instea=
d.
> + =A0 =A0 =A0 pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
> + =A0 =A0 =A0 temp &=3D ~DIU_DIV_MASK;
> + =A0 =A0 =A0 temp |=3D (bestval & DIU_DIV_MASK);
> + =A0 =A0 =A0 out_be32(ccm + CCM_SCFR1, temp);
> + =A0 =A0 =A0 pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
> + =A0 =A0 =A0 iounmap(ccm);
> +}
> +
> +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> +{
> + =A0 =A0 =A0 return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
There's no point in using snprintf since you're printing a string
literal. You can use sprintf.
> +}
> +
> +int mpc512x_set_sysfs_monitor_port(int val)
> +{
> + =A0 =A0 =A0 return 0;
> +}
> +
> +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_sha=
red_fb;
> +
> +#if defined(CONFIG_FB_FSL_DIU) || \
> + =A0 =A0defined(CONFIG_FB_FSL_DIU_MODULE)
> +static inline void mpc512x_free_bootmem(struct page *page)
> +{
> + =A0 =A0 =A0 __ClearPageReserved(page);
> + =A0 =A0 =A0 BUG_ON(PageTail(page));
> + =A0 =A0 =A0 BUG_ON(atomic_read(&page->_count) > 1);
> + =A0 =A0 =A0 atomic_set(&page->_count, 1);
> + =A0 =A0 =A0 __free_page(page);
> + =A0 =A0 =A0 totalram_pages++;
> +}
> +
> +void mpc512x_release_bootmem(void)
> +{
> + =A0 =A0 =A0 unsigned long addr =3D diu_shared_fb.fb_phys & PAGE_MASK;
> + =A0 =A0 =A0 unsigned long size =3D diu_shared_fb.fb_len;
> + =A0 =A0 =A0 unsigned long start, end;
> +
> + =A0 =A0 =A0 if (diu_shared_fb.in_use) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 start =3D PFN_UP(addr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 end =3D PFN_DOWN(addr + size);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (; start < end; start++)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc512x_free_bootmem(pfn_to=
_page(start));
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 diu_shared_fb.in_use =3D false;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 diu_ops.release_bootmem =3D NULL;
> +}
> +#endif
Do you really need to use reserve_bootmem? Have you tried kmalloc or
alloc_pages_exact()?
> +
> +/*
> + * Check if DIU was pre-initialized. If so, perform steps
> + * needed to continue displaying through the whole boot process.
> + * Move area descriptor and gamma table elsewhere, they are
> + * destroyed by bootmem allocator otherwise. The frame buffer
> + * address range will be reserved in setup_arch() after bootmem
> + * allocator is up.
> + */
> +void __init mpc512x_init_diu(void)
> +{
> + =A0 =A0 =A0 struct device_node *np;
> + =A0 =A0 =A0 void __iomem *diu_reg;
> + =A0 =A0 =A0 phys_addr_t desc;
> + =A0 =A0 =A0 void __iomem *vaddr;
> + =A0 =A0 =A0 unsigned long mode, pix_fmt, res, bpp;
> + =A0 =A0 =A0 unsigned long dst;
> +
> + =A0 =A0 =A0 np =3D of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu=
");
> + =A0 =A0 =A0 if (!np) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("No DIU node\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> + =A0 =A0 =A0 }
Shouldn't you be probing as an OF driver instead of manually searching
for the DIU node?
> +
> + =A0 =A0 =A0 diu_reg =3D of_iomap(np, 0);
> + =A0 =A0 =A0 of_node_put(np);
> + =A0 =A0 =A0 if (!diu_reg) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("Can't map DIU\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 mode =3D in_be32(diu_reg + 0x1c);
> + =A0 =A0 =A0 if (mode !=3D 1) {
How can in_be32() return a -1?
--=20
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 2:05 ` Timur Tabi
(?)
@ 2010-04-30 10:19 ` Anatolij Gustschin
-1 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 10:19 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, 29 Apr 2010 21:05:26 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
>
>
> > +void __init mpc5121_ads_init_early(void)
> > +{
> > + mpc512x_init_diu();
> > +}
> > +
> > define_machine(mpc5121_ads) {
> > .name = "MPC5121 ADS",
> > .probe = mpc5121_ads_probe,
> > .setup_arch = mpc5121_ads_setup_arch,
> > .init = mpc512x_init,
> > + .init_early = mpc5121_ads_init_early,
>
> How about just doing this?
>
> .init_early = mpc512x_init_diu,
I thought it should be prepared for adding other code here.
mpc5121_ads_init_early() is generic and could contain other
things as well. I would vote for current version.
> > +void __init mpc512x_generic_init_early(void)
> > +{
> > + mpc512x_init_diu();
> > +}
> > +
> > +void __init mpc512x_generic_setup_arch(void)
> > +{
> > + mpc512x_setup_diu();
> > +}
> > +
> > define_machine(mpc5121_generic) {
> > .name = "MPC5121 generic",
> > .probe = mpc5121_generic_probe,
> > .init = mpc512x_init,
> > + .init_early = mpc512x_generic_init_early,
> > + .setup_arch = mpc512x_generic_setup_arch,
>
> And a similar change here.
Same here.
...
> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> > index b7f518a..8e297fa 100644
> > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> > @@ -16,7 +16,11 @@
> > #include <linux/io.h>
> > #include <linux/irq.h>
> > #include <linux/of_platform.h>
> > +#include <linux/fsl-diu-fb.h>
> > +#include <linux/bootmem.h>
> > +#include <sysdev/fsl_soc.h>
> >
> > +#include <asm/cacheflush.h>
> > #include <asm/machdep.h>
> > #include <asm/ipic.h>
> > #include <asm/prom.h>
> > @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> > ;
> > }
> >
> > +struct fsl_diu_shared_fb {
> > + char gamma[0x300]; /* 32-bit aligned! */
>
> char or u8?
Will use u8.
> > + struct diu_ad ad0; /* 32-bit aligned! */
> > + phys_addr_t fb_phys;
> > + size_t fb_len;
> > + bool in_use;
> > +};
>
> Where did "bool" come from? Use "int" instead.
It is common practise to use "bool" type, grep in drivers dir.
> > +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> > + int monitor_port)
> > +{
> > + unsigned int pix_fmt;
> > +
> > + switch (bits_per_pixel) {
> > + case 32:
> > + pix_fmt = 0x88883316;
> > + break;
> > + case 24:
> > + pix_fmt = 0x88082219;
> > + break;
> > + case 16:
> > + pix_fmt = 0x65053118;
> > + break;
> > + default:
> > + pix_fmt = 0x00000400;
> > + }
> > + return pix_fmt;
> > +}
>
> This is simpler:
>
> switch (bits_per_pixel) {
> case 32:
> return 0x88883316;
> case 24:
> return 0x88082219;
> case 16:
> return = 0x65053118;
> }
>
> return 0x00000400;
> }
Will simplify as suggested, thanks!
> > + ccm = of_iomap(np, 0);
> > + if (!ccm) {
> > + pr_err("Can't map clock control module reg.\n");
> > + of_node_put(np);
> > + return;
> > + }
> > + of_node_put(np);
>
> This is simpler:
>
> ccm = of_iomap(np, 0);
> of_node_put(np);
> if (!ccm) {
> pr_err("Can't map clock control module reg.\n");
> return;
> }
OK, will fix, thanks.
>
> > + np = of_find_node_by_type(NULL, "cpu");
> > + if (np) {
> > + unsigned int size;
> > + const unsigned int *prop > > + of_get_property(np, "bus-frequency", &size);
>
> Since you don't use 'size', you can skip it:
>
> const unsigned int *prop > of_get_property(np, "bus-frequency", NULL);
>
> > + } else {
> > + pr_err("Can't find \"cpu\" node.\n");
>
> 'cpu' is simpler than \"cpu\"
Will simplify, too.
> > + /* Calculate the pixel clock with the smallest error */
> > + /* calculate the following in steps to avoid overflow */
> > + pr_debug("DIU pixclock in ps - %d\n", pixclock);
> > + temp = (1000000000 / pixclock) * 1000;
>
> I'm pretty sure the compiler will optimize this to:
>
> temp = (1000000000000UL / pixclock);
>
> so you may as well do it that way.
??
1000000000000 is _not_ UL, but UUL.
>
> > + pixclock = temp;
> > + pr_debug("DIU pixclock freq - %u\n", pixclock);
> > +
> > + temp = (temp * 5) / 100; /* pixclock * 0.05 */
>
> The compiler will optimize this to:
>
> temp /= 20;
Can do it, too. Thanks.
> > + pr_debug("deviation = %d\n", temp);
> > + minpixclock = pixclock - temp;
> > + maxpixclock = pixclock + temp;
> > + pr_debug("DIU minpixclock - %lu\n", minpixclock);
> > + pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> > + pixval = speed_ccb/pixclock;
> > + pr_debug("DIU pixval = %lu\n", pixval);
> > +
> > + err = 100000000;
>
> Why do you assign err to this arbitrary value?
Dunno. It is Freescale's code and I do not have time to check
and understand each bit of it and to explain it.
> > + bestval = pixval;
> > + pr_debug("DIU bestval = %lu\n", bestval);
> > +
> > + bestfreq = 0;
> > + for (i = -1; i <= 1; i++) {
> > + temp = speed_ccb / (pixval+i);
> > + pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
> > + i, pixval, temp);
> > + if ((temp < minpixclock) || (temp > maxpixclock))
> > + pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
> > + minpixclock, maxpixclock);
> > + else if (abs(temp - pixclock) < err) {
> > + pr_debug("Entered the else if block %d\n", i);
> > + err = abs(temp - pixclock);
> > + bestval = pixval + i;
> > + bestfreq = temp;
> > + }
> > + }
> > +
> > + pr_debug("DIU chose = %lx\n", bestval);
> > + pr_debug("DIU error = %ld\n NomPixClk ", err);
> > + pr_debug("DIU: Best Freq = %lx\n", bestfreq);
> > + /* Modify DIU_DIV in CCM SCFR1 */
> > + temp = in_be32(ccm + CCM_SCFR1);
>
> Don't use offsets like + CCM_SCFR1. Create a structure and use that instead.
Done in next patch version.
> > + pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
> > + temp &= ~DIU_DIV_MASK;
> > + temp |= (bestval & DIU_DIV_MASK);
> > + out_be32(ccm + CCM_SCFR1, temp);
> > + pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
> > + iounmap(ccm);
> > +}
> > +
> > +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> > +{
> > + return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
>
> There's no point in using snprintf since you're printing a string
> literal. You can use sprintf.
Will do, thanks.
> > +}
> > +
> > +int mpc512x_set_sysfs_monitor_port(int val)
> > +{
> > + return 0;
> > +}
> > +
> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
> > +
> > +#if defined(CONFIG_FB_FSL_DIU) || \
> > + defined(CONFIG_FB_FSL_DIU_MODULE)
> > +static inline void mpc512x_free_bootmem(struct page *page)
> > +{
> > + __ClearPageReserved(page);
> > + BUG_ON(PageTail(page));
> > + BUG_ON(atomic_read(&page->_count) > 1);
> > + atomic_set(&page->_count, 1);
> > + __free_page(page);
> > + totalram_pages++;
> > +}
> > +
> > +void mpc512x_release_bootmem(void)
> > +{
> > + unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
> > + unsigned long size = diu_shared_fb.fb_len;
> > + unsigned long start, end;
> > +
> > + if (diu_shared_fb.in_use) {
> > + start = PFN_UP(addr);
> > + end = PFN_DOWN(addr + size);
> > +
> > + for (; start < end; start++)
> > + mpc512x_free_bootmem(pfn_to_page(start));
> > +
> > + diu_shared_fb.in_use = false;
> > + }
> > + diu_ops.release_bootmem = NULL;
> > +}
> > +#endif
>
> Do you really need to use reserve_bootmem? Have you tried kmalloc or
> alloc_pages_exact()?
Yes. No, it is too early to use them here.
> > +
> > +/*
> > + * Check if DIU was pre-initialized. If so, perform steps
> > + * needed to continue displaying through the whole boot process.
> > + * Move area descriptor and gamma table elsewhere, they are
> > + * destroyed by bootmem allocator otherwise. The frame buffer
> > + * address range will be reserved in setup_arch() after bootmem
> > + * allocator is up.
> > + */
> > +void __init mpc512x_init_diu(void)
> > +{
> > + struct device_node *np;
> > + void __iomem *diu_reg;
> > + phys_addr_t desc;
> > + void __iomem *vaddr;
> > + unsigned long mode, pix_fmt, res, bpp;
> > + unsigned long dst;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
> > + if (!np) {
> > + pr_err("No DIU node\n");
> > + return;
> > + }
>
> Shouldn't you be probing as an OF driver instead of manually searching
> for the DIU node?
No, not here.
> > +
> > + diu_reg = of_iomap(np, 0);
> > + of_node_put(np);
> > + if (!diu_reg) {
> > + pr_err("Can't map DIU\n");
> > + return;
> > + }
> > +
> > + mode = in_be32(diu_reg + 0x1c);
> > + if (mode != 1) {
>
> How can in_be32() return a -1?
It is a 1, not -1. I will use appropriate macro here and also
change to use a struct instead of adding offset to register base.
Thanks for your review and comments!
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 10:19 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 10:19 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, 29 Apr 2010 21:05:26 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
>
>
> > +void __init mpc5121_ads_init_early(void)
> > +{
> > + mpc512x_init_diu();
> > +}
> > +
> > define_machine(mpc5121_ads) {
> > .name = "MPC5121 ADS",
> > .probe = mpc5121_ads_probe,
> > .setup_arch = mpc5121_ads_setup_arch,
> > .init = mpc512x_init,
> > + .init_early = mpc5121_ads_init_early,
>
> How about just doing this?
>
> .init_early = mpc512x_init_diu,
I thought it should be prepared for adding other code here.
mpc5121_ads_init_early() is generic and could contain other
things as well. I would vote for current version.
> > +void __init mpc512x_generic_init_early(void)
> > +{
> > + mpc512x_init_diu();
> > +}
> > +
> > +void __init mpc512x_generic_setup_arch(void)
> > +{
> > + mpc512x_setup_diu();
> > +}
> > +
> > define_machine(mpc5121_generic) {
> > .name = "MPC5121 generic",
> > .probe = mpc5121_generic_probe,
> > .init = mpc512x_init,
> > + .init_early = mpc512x_generic_init_early,
> > + .setup_arch = mpc512x_generic_setup_arch,
>
> And a similar change here.
Same here.
...
> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> > index b7f518a..8e297fa 100644
> > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> > @@ -16,7 +16,11 @@
> > #include <linux/io.h>
> > #include <linux/irq.h>
> > #include <linux/of_platform.h>
> > +#include <linux/fsl-diu-fb.h>
> > +#include <linux/bootmem.h>
> > +#include <sysdev/fsl_soc.h>
> >
> > +#include <asm/cacheflush.h>
> > #include <asm/machdep.h>
> > #include <asm/ipic.h>
> > #include <asm/prom.h>
> > @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> > ;
> > }
> >
> > +struct fsl_diu_shared_fb {
> > + char gamma[0x300]; /* 32-bit aligned! */
>
> char or u8?
Will use u8.
> > + struct diu_ad ad0; /* 32-bit aligned! */
> > + phys_addr_t fb_phys;
> > + size_t fb_len;
> > + bool in_use;
> > +};
>
> Where did "bool" come from? Use "int" instead.
It is common practise to use "bool" type, grep in drivers dir.
> > +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> > + int monitor_port)
> > +{
> > + unsigned int pix_fmt;
> > +
> > + switch (bits_per_pixel) {
> > + case 32:
> > + pix_fmt = 0x88883316;
> > + break;
> > + case 24:
> > + pix_fmt = 0x88082219;
> > + break;
> > + case 16:
> > + pix_fmt = 0x65053118;
> > + break;
> > + default:
> > + pix_fmt = 0x00000400;
> > + }
> > + return pix_fmt;
> > +}
>
> This is simpler:
>
> switch (bits_per_pixel) {
> case 32:
> return 0x88883316;
> case 24:
> return 0x88082219;
> case 16:
> return = 0x65053118;
> }
>
> return 0x00000400;
> }
Will simplify as suggested, thanks!
> > + ccm = of_iomap(np, 0);
> > + if (!ccm) {
> > + pr_err("Can't map clock control module reg.\n");
> > + of_node_put(np);
> > + return;
> > + }
> > + of_node_put(np);
>
> This is simpler:
>
> ccm = of_iomap(np, 0);
> of_node_put(np);
> if (!ccm) {
> pr_err("Can't map clock control module reg.\n");
> return;
> }
OK, will fix, thanks.
>
> > + np = of_find_node_by_type(NULL, "cpu");
> > + if (np) {
> > + unsigned int size;
> > + const unsigned int *prop =
> > + of_get_property(np, "bus-frequency", &size);
>
> Since you don't use 'size', you can skip it:
>
> const unsigned int *prop =
> of_get_property(np, "bus-frequency", NULL);
>
> > + } else {
> > + pr_err("Can't find \"cpu\" node.\n");
>
> 'cpu' is simpler than \"cpu\"
Will simplify, too.
> > + /* Calculate the pixel clock with the smallest error */
> > + /* calculate the following in steps to avoid overflow */
> > + pr_debug("DIU pixclock in ps - %d\n", pixclock);
> > + temp = (1000000000 / pixclock) * 1000;
>
> I'm pretty sure the compiler will optimize this to:
>
> temp = (1000000000000UL / pixclock);
>
> so you may as well do it that way.
??
1000000000000 is _not_ UL, but UUL.
>
> > + pixclock = temp;
> > + pr_debug("DIU pixclock freq - %u\n", pixclock);
> > +
> > + temp = (temp * 5) / 100; /* pixclock * 0.05 */
>
> The compiler will optimize this to:
>
> temp /= 20;
Can do it, too. Thanks.
> > + pr_debug("deviation = %d\n", temp);
> > + minpixclock = pixclock - temp;
> > + maxpixclock = pixclock + temp;
> > + pr_debug("DIU minpixclock - %lu\n", minpixclock);
> > + pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> > + pixval = speed_ccb/pixclock;
> > + pr_debug("DIU pixval = %lu\n", pixval);
> > +
> > + err = 100000000;
>
> Why do you assign err to this arbitrary value?
Dunno. It is Freescale's code and I do not have time to check
and understand each bit of it and to explain it.
> > + bestval = pixval;
> > + pr_debug("DIU bestval = %lu\n", bestval);
> > +
> > + bestfreq = 0;
> > + for (i = -1; i <= 1; i++) {
> > + temp = speed_ccb / (pixval+i);
> > + pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
> > + i, pixval, temp);
> > + if ((temp < minpixclock) || (temp > maxpixclock))
> > + pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
> > + minpixclock, maxpixclock);
> > + else if (abs(temp - pixclock) < err) {
> > + pr_debug("Entered the else if block %d\n", i);
> > + err = abs(temp - pixclock);
> > + bestval = pixval + i;
> > + bestfreq = temp;
> > + }
> > + }
> > +
> > + pr_debug("DIU chose = %lx\n", bestval);
> > + pr_debug("DIU error = %ld\n NomPixClk ", err);
> > + pr_debug("DIU: Best Freq = %lx\n", bestfreq);
> > + /* Modify DIU_DIV in CCM SCFR1 */
> > + temp = in_be32(ccm + CCM_SCFR1);
>
> Don't use offsets like + CCM_SCFR1. Create a structure and use that instead.
Done in next patch version.
> > + pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
> > + temp &= ~DIU_DIV_MASK;
> > + temp |= (bestval & DIU_DIV_MASK);
> > + out_be32(ccm + CCM_SCFR1, temp);
> > + pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
> > + iounmap(ccm);
> > +}
> > +
> > +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> > +{
> > + return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n");
>
> There's no point in using snprintf since you're printing a string
> literal. You can use sprintf.
Will do, thanks.
> > +}
> > +
> > +int mpc512x_set_sysfs_monitor_port(int val)
> > +{
> > + return 0;
> > +}
> > +
> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
> > +
> > +#if defined(CONFIG_FB_FSL_DIU) || \
> > + defined(CONFIG_FB_FSL_DIU_MODULE)
> > +static inline void mpc512x_free_bootmem(struct page *page)
> > +{
> > + __ClearPageReserved(page);
> > + BUG_ON(PageTail(page));
> > + BUG_ON(atomic_read(&page->_count) > 1);
> > + atomic_set(&page->_count, 1);
> > + __free_page(page);
> > + totalram_pages++;
> > +}
> > +
> > +void mpc512x_release_bootmem(void)
> > +{
> > + unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
> > + unsigned long size = diu_shared_fb.fb_len;
> > + unsigned long start, end;
> > +
> > + if (diu_shared_fb.in_use) {
> > + start = PFN_UP(addr);
> > + end = PFN_DOWN(addr + size);
> > +
> > + for (; start < end; start++)
> > + mpc512x_free_bootmem(pfn_to_page(start));
> > +
> > + diu_shared_fb.in_use = false;
> > + }
> > + diu_ops.release_bootmem = NULL;
> > +}
> > +#endif
>
> Do you really need to use reserve_bootmem? Have you tried kmalloc or
> alloc_pages_exact()?
Yes. No, it is too early to use them here.
> > +
> > +/*
> > + * Check if DIU was pre-initialized. If so, perform steps
> > + * needed to continue displaying through the whole boot process.
> > + * Move area descriptor and gamma table elsewhere, they are
> > + * destroyed by bootmem allocator otherwise. The frame buffer
> > + * address range will be reserved in setup_arch() after bootmem
> > + * allocator is up.
> > + */
> > +void __init mpc512x_init_diu(void)
> > +{
> > + struct device_node *np;
> > + void __iomem *diu_reg;
> > + phys_addr_t desc;
> > + void __iomem *vaddr;
> > + unsigned long mode, pix_fmt, res, bpp;
> > + unsigned long dst;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
> > + if (!np) {
> > + pr_err("No DIU node\n");
> > + return;
> > + }
>
> Shouldn't you be probing as an OF driver instead of manually searching
> for the DIU node?
No, not here.
> > +
> > + diu_reg = of_iomap(np, 0);
> > + of_node_put(np);
> > + if (!diu_reg) {
> > + pr_err("Can't map DIU\n");
> > + return;
> > + }
> > +
> > + mode = in_be32(diu_reg + 0x1c);
> > + if (mode != 1) {
>
> How can in_be32() return a -1?
It is a 1, not -1. I will use appropriate macro here and also
change to use a struct instead of adding offset to register base.
Thanks for your review and comments!
Anatolij
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 10:19 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 10:19 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Thu, 29 Apr 2010 21:05:26 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:
>=20
>=20
> > +void __init mpc5121_ads_init_early(void)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu();
> > +}
> > +
> > =C2=A0define_machine(mpc5121_ads) {
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =3D "MPC5121 ADS",
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_ads_probe,
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =3D mpc5121_ads_setup_arch,
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init,
> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =3D mpc5121_ads_init_early,
>=20
> How about just doing this?
>=20
> .init_early =3D mpc512x_init_diu,
I thought it should be prepared for adding other code here.
mpc5121_ads_init_early() is generic and could contain other
things as well. I would vote for current version.
> > +void __init mpc512x_generic_init_early(void)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu();
> > +}
> > +
> > +void __init mpc512x_generic_setup_arch(void)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_setup_diu();
> > +}
> > +
> > =C2=A0define_machine(mpc5121_generic) {
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =3D "MPC5121 generic",
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_generic_probe,
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init,
> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =3D mpc512x_generic_init_early,
> > + =C2=A0 =C2=A0 =C2=A0 .setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =3D mpc512x_generic_setup_arch,
>=20
> And a similar change here.
Same here.
...
> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerp=
c/platforms/512x/mpc512x_shared.c
> > index b7f518a..8e297fa 100644
> > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> > @@ -16,7 +16,11 @@
> > =C2=A0#include <linux/io.h>
> > =C2=A0#include <linux/irq.h>
> > =C2=A0#include <linux/of_platform.h>
> > +#include <linux/fsl-diu-fb.h>
> > +#include <linux/bootmem.h>
> > +#include <sysdev/fsl_soc.h>
> >
> > +#include <asm/cacheflush.h>
> > =C2=A0#include <asm/machdep.h>
> > =C2=A0#include <asm/ipic.h>
> > =C2=A0#include <asm/prom.h>
> > @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0;
> > =C2=A0}
> >
> > +struct fsl_diu_shared_fb {
> > + =C2=A0 =C2=A0 =C2=A0 char =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ga=
mma[0x300]; =C2=A0 /* 32-bit aligned! */
>=20
> char or u8?
Will use u8.
> > + =C2=A0 =C2=A0 =C2=A0 struct diu_ad =C2=A0 ad0; =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0/* 32-bit aligned! */
> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t =C2=A0 =C2=A0 fb_phys;
> > + =C2=A0 =C2=A0 =C2=A0 size_t =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fb_len;
> > + =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0in=
_use;
> > +};
>=20
> Where did "bool" come from? Use "int" instead.
It is common practise to use "bool" type, grep in drivers dir.
> > +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int monitor_port)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 unsigned int pix_fmt;
> > +
> > + =C2=A0 =C2=A0 =C2=A0 switch (bits_per_pixel) {
> > + =C2=A0 =C2=A0 =C2=A0 case 32:
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x888833=
16;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> > + =C2=A0 =C2=A0 =C2=A0 case 24:
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x880822=
19;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> > + =C2=A0 =C2=A0 =C2=A0 case 16:
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x650531=
18;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> > + =C2=A0 =C2=A0 =C2=A0 default:
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x000004=
00;
> > + =C2=A0 =C2=A0 =C2=A0 }
> > + =C2=A0 =C2=A0 =C2=A0 return pix_fmt;
> > +}
>=20
> This is simpler:
>=20
> switch (bits_per_pixel) {
> case 32:
> return 0x88883316;
> case 24:
> return 0x88082219;
> case 16:
> return =3D 0x65053118;
> }
>=20
> return 0x00000400;
> }
Will simplify as suggested, thanks!
> > + =C2=A0 =C2=A0 =C2=A0 ccm =3D of_iomap(np, 0);
> > + =C2=A0 =C2=A0 =C2=A0 if (!ccm) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't map cl=
ock control module reg.\n");
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 of_node_put(np);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> > + =C2=A0 =C2=A0 =C2=A0 }
> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np);
>=20
> This is simpler:
>=20
> ccm =3D of_iomap(np, 0);
> of_node_put(np);
> if (!ccm) {
> pr_err("Can't map clock control module reg.\n");
> return;
> }
OK, will fix, thanks.
>=20
> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_node_by_type(NULL, "cpu");
> > + =C2=A0 =C2=A0 =C2=A0 if (np) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned int size;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 const unsigned int *=
prop =3D
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 of_get_property(np, "bus-frequency", &size);
>=20
> Since you don't use 'size', you can skip it:
>=20
> const unsigned int *prop =3D
> of_get_property(np, "bus-frequency", NULL);
>=20
> > + =C2=A0 =C2=A0 =C2=A0 } else {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't find \=
"cpu\" node.\n");
>=20
> 'cpu' is simpler than \"cpu\"
Will simplify, too.
> > + =C2=A0 =C2=A0 =C2=A0 /* Calculate the pixel clock with the smallest e=
rror */
> > + =C2=A0 =C2=A0 =C2=A0 /* calculate the following in steps to avoid ove=
rflow */
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixclock in ps - %d\n", pixclock);
> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (1000000000 / pixclock) * 1000;
>=20
> I'm pretty sure the compiler will optimize this to:
>=20
> temp =3D (1000000000000UL / pixclock);
>=20
> so you may as well do it that way.
??
1000000000000 is _not_ UL, but UUL.
>=20
> > + =C2=A0 =C2=A0 =C2=A0 pixclock =3D temp;
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixclock freq - %u\n", pixclock);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (temp * 5) / 100; /* pixclock * 0.05 */
>=20
> The compiler will optimize this to:
>=20
> temp /=3D 20;
Can do it, too. Thanks.
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("deviation =3D %d\n", temp);
> > + =C2=A0 =C2=A0 =C2=A0 minpixclock =3D pixclock - temp;
> > + =C2=A0 =C2=A0 =C2=A0 maxpixclock =3D pixclock + temp;
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU minpixclock - %lu\n", minpixclock);
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
> > + =C2=A0 =C2=A0 =C2=A0 pixval =3D speed_ccb/pixclock;
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixval =3D %lu\n", pixval);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 err =3D 100000000;
>=20
> Why do you assign err to this arbitrary value?
Dunno. It is Freescale's code and I do not have time to check
and understand each bit of it and to explain it.
> > + =C2=A0 =C2=A0 =C2=A0 bestval =3D pixval;
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU bestval =3D %lu\n", bestval);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 bestfreq =3D 0;
> > + =C2=A0 =C2=A0 =C2=A0 for (i =3D -1; i <=3D 1; i++) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 temp =3D speed_ccb /=
(pixval+i);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU test p=
ixval i=3D%d, pixval=3D%lu, temp freq. =3D %u\n",
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 i, pixval, temp);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((temp < minpixcl=
ock) || (temp > maxpixclock))
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 minpixclock, maxpixclock);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else if (abs(temp - =
pixclock) < err) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 pr_debug("Entered the else if block %d\n", i);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 err =3D abs(temp - pixclock);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 bestval =3D pixval + i;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 bestfreq =3D temp;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> > + =C2=A0 =C2=A0 =C2=A0 }
> > +
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU chose =3D %lx\n", bestval);
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU error =3D %ld\n NomPixClk ", err);
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Best Freq =3D %lx\n", bestfreq);
> > + =C2=A0 =C2=A0 =C2=A0 /* Modify DIU_DIV in CCM SCFR1 */
> > + =C2=A0 =C2=A0 =C2=A0 temp =3D in_be32(ccm + CCM_SCFR1);
>=20
> Don't use offsets like + CCM_SCFR1. Create a structure and use that inst=
ead.
Done in next patch version.
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Current value of SCFR1: 0x%08x\n"=
, temp);
> > + =C2=A0 =C2=A0 =C2=A0 temp &=3D ~DIU_DIV_MASK;
> > + =C2=A0 =C2=A0 =C2=A0 temp |=3D (bestval & DIU_DIV_MASK);
> > + =C2=A0 =C2=A0 =C2=A0 out_be32(ccm + CCM_SCFR1, temp);
> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Modified value of SCFR1: 0x%08x\n=
", temp);
> > + =C2=A0 =C2=A0 =C2=A0 iounmap(ccm);
> > +}
> > +
> > +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n"=
);
>=20
> There's no point in using snprintf since you're printing a string
> literal. You can use sprintf.
Will do, thanks.
> > +}
> > +
> > +int mpc512x_set_sysfs_monitor_port(int val)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 return 0;
> > +}
> > +
> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_s=
hared_fb;
> > +
> > +#if defined(CONFIG_FB_FSL_DIU) || \
> > + =C2=A0 =C2=A0defined(CONFIG_FB_FSL_DIU_MODULE)
> > +static inline void mpc512x_free_bootmem(struct page *page)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 __ClearPageReserved(page);
> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(PageTail(page));
> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(atomic_read(&page->_count) > 1);
> > + =C2=A0 =C2=A0 =C2=A0 atomic_set(&page->_count, 1);
> > + =C2=A0 =C2=A0 =C2=A0 __free_page(page);
> > + =C2=A0 =C2=A0 =C2=A0 totalram_pages++;
> > +}
> > +
> > +void mpc512x_release_bootmem(void)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 unsigned long addr =3D diu_shared_fb.fb_phys & P=
AGE_MASK;
> > + =C2=A0 =C2=A0 =C2=A0 unsigned long size =3D diu_shared_fb.fb_len;
> > + =C2=A0 =C2=A0 =C2=A0 unsigned long start, end;
> > +
> > + =C2=A0 =C2=A0 =C2=A0 if (diu_shared_fb.in_use) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 start =3D PFN_UP(add=
r);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 end =3D PFN_DOWN(add=
r + size);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (; start < end; =
start++)
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 mpc512x_free_bootmem(pfn_to_page(start));
> > +
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 diu_shared_fb.in_use=
=3D false;
> > + =C2=A0 =C2=A0 =C2=A0 }
> > + =C2=A0 =C2=A0 =C2=A0 diu_ops.release_bootmem =3D NULL;
> > +}
> > +#endif
>=20
> Do you really need to use reserve_bootmem? Have you tried kmalloc or
> alloc_pages_exact()?
Yes. No, it is too early to use them here.
> > +
> > +/*
> > + * Check if DIU was pre-initialized. If so, perform steps
> > + * needed to continue displaying through the whole boot process.
> > + * Move area descriptor and gamma table elsewhere, they are
> > + * destroyed by bootmem allocator otherwise. The frame buffer
> > + * address range will be reserved in setup_arch() after bootmem
> > + * allocator is up.
> > + */
> > +void __init mpc512x_init_diu(void)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 struct device_node *np;
> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *diu_reg;
> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t desc;
> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *vaddr;
> > + =C2=A0 =C2=A0 =C2=A0 unsigned long mode, pix_fmt, res, bpp;
> > + =C2=A0 =C2=A0 =C2=A0 unsigned long dst;
> > +
> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_compatible_node(NULL, NULL, "fsl,=
mpc5121-diu");
> > + =C2=A0 =C2=A0 =C2=A0 if (!np) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("No DIU node\=
n");
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> > + =C2=A0 =C2=A0 =C2=A0 }
>=20
> Shouldn't you be probing as an OF driver instead of manually searching
> for the DIU node?
No, not here.
> > +
> > + =C2=A0 =C2=A0 =C2=A0 diu_reg =3D of_iomap(np, 0);
> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np);
> > + =C2=A0 =C2=A0 =C2=A0 if (!diu_reg) {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't map DI=
U\n");
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> > + =C2=A0 =C2=A0 =C2=A0 }
> > +
> > + =C2=A0 =C2=A0 =C2=A0 mode =3D in_be32(diu_reg + 0x1c);
> > + =C2=A0 =C2=A0 =C2=A0 if (mode !=3D 1) {
>=20
> How can in_be32() return a -1?
It is a 1, not -1. I will use appropriate macro here and also
change to use a struct instead of adding offset to register base.
Thanks for your review and comments!
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 10:19 ` Anatolij Gustschin
(?)
@ 2010-04-30 15:08 ` Timur Tabi
-1 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 15:08 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> How about just doing this?
>>
>> .init_early = mpc512x_init_diu,
>
> I thought it should be prepared for adding other code here.
> mpc5121_ads_init_early() is generic and could contain other
> things as well. I would vote for current version.
Do you have any plans to add any additional code? If not, then I say
skip the middle-man. If someone ever needs to do more, he can always
put that function back.
>> I'm pretty sure the compiler will optimize this to:
>>
>> temp = (1000000000000UL / pixclock);
>>
>> so you may as well do it that way.
>
> ??
> 1000000000000 is _not_ UL, but UUL.
That's what I meant. Actually, I think it's ULL. Regardless, I think
the compiler will see the "1000000000 ... * 1000" and just combine
them together. You're not actually outsmarting the compiler.
>> > + err = 100000000;
>>
>> Why do you assign err to this arbitrary value?
>
> Dunno. It is Freescale's code and I do not have time to check
> and understand each bit of it and to explain it.
*sigh* You're not the first person to modify the DIU driver without
understanding what it all does. I suspect that the original author
should have done this:
err = -1;
because he wanted it to be the largest possible integer.
>> Do you really need to use reserve_bootmem? Have you tried kmalloc or
>> alloc_pages_exact()?
>
> Yes. No, it is too early to use them here.
There was a recent change in the kernel that allows kmalloc to work
earlier than before. Take a look at commit
85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
warning").
>> > + mode = in_be32(diu_reg + 0x1c);
>> > + if (mode != 1) {
>>
>> How can in_be32() return a -1?
>
> It is a 1, not -1. I will use appropriate macro here and also
> change to use a struct instead of adding offset to register base.
Sorry, I misread the code. I could have sworn it read "mode != -1"
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 15:08 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 15:08 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> How about just doing this?
>>
>> .init_early = mpc512x_init_diu,
>
> I thought it should be prepared for adding other code here.
> mpc5121_ads_init_early() is generic and could contain other
> things as well. I would vote for current version.
Do you have any plans to add any additional code? If not, then I say
skip the middle-man. If someone ever needs to do more, he can always
put that function back.
>> I'm pretty sure the compiler will optimize this to:
>>
>> temp = (1000000000000UL / pixclock);
>>
>> so you may as well do it that way.
>
> ??
> 1000000000000 is _not_ UL, but UUL.
That's what I meant. Actually, I think it's ULL. Regardless, I think
the compiler will see the "1000000000 ... * 1000" and just combine
them together. You're not actually outsmarting the compiler.
>> > + err = 100000000;
>>
>> Why do you assign err to this arbitrary value?
>
> Dunno. It is Freescale's code and I do not have time to check
> and understand each bit of it and to explain it.
*sigh* You're not the first person to modify the DIU driver without
understanding what it all does. I suspect that the original author
should have done this:
err = -1;
because he wanted it to be the largest possible integer.
>> Do you really need to use reserve_bootmem? Have you tried kmalloc or
>> alloc_pages_exact()?
>
> Yes. No, it is too early to use them here.
There was a recent change in the kernel that allows kmalloc to work
earlier than before. Take a look at commit
85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
warning").
>> > + mode = in_be32(diu_reg + 0x1c);
>> > + if (mode != 1) {
>>
>> How can in_be32() return a -1?
>
> It is a 1, not -1. I will use appropriate macro here and also
> change to use a struct instead of adding offset to register base.
Sorry, I misread the code. I could have sworn it read "mode != -1"
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 15:08 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 15:08 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> How about just doing this?
>>
>> .init_early =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc512x_init_diu,
>
> I thought it should be prepared for adding other code here.
> mpc5121_ads_init_early() is generic and could contain other
> things as well. I would vote for current version.
Do you have any plans to add any additional code? If not, then I say
skip the middle-man. If someone ever needs to do more, he can always
put that function back.
>> I'm pretty sure the compiler will optimize this to:
>>
>> =A0 =A0 temp =3D (1000000000000UL / pixclock);
>>
>> so you may as well do it that way.
>
> ??
> 1000000000000 is _not_ UL, but UUL.
That's what I meant. Actually, I think it's ULL. Regardless, I think
the compiler will see the "1000000000 ... * 1000" and just combine
them together. You're not actually outsmarting the compiler.
>> > + =A0 =A0 =A0 err =3D 100000000;
>>
>> Why do you assign err to this arbitrary value?
>
> Dunno. It is Freescale's code and I do not have time to check
> and understand each bit of it and to explain it.
*sigh* You're not the first person to modify the DIU driver without
understanding what it all does. I suspect that the original author
should have done this:
err =3D -1;
because he wanted it to be the largest possible integer.
>> Do you really need to use reserve_bootmem? =A0Have you tried kmalloc or
>> alloc_pages_exact()?
>
> Yes. No, it is too early to use them here.
There was a recent change in the kernel that allows kmalloc to work
earlier than before. Take a look at commit
85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
warning").
>> > + =A0 =A0 =A0 mode =3D in_be32(diu_reg + 0x1c);
>> > + =A0 =A0 =A0 if (mode !=3D 1) {
>>
>> How can in_be32() return a -1?
>
> It is a 1, not -1. I will use appropriate macro here and also
> change to use a struct instead of adding offset to register base.
Sorry, I misread the code. I could have sworn it read "mode !=3D -1"
--=20
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread[parent not found: <x2ned82fe3e1004300808q757826cs864ac1c7c082f81-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 15:08 ` Timur Tabi
(?)
@ 2010-04-30 16:22 ` Scott Wood
-1 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 16:22 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, Apr 30, 2010 at 10:08:45AM -0500, Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
>
> >> I'm pretty sure the compiler will optimize this to:
> >>
> >> temp = (1000000000000UL / pixclock);
> >>
> >> so you may as well do it that way.
> >
> > ??
> > 1000000000000 is _not_ UL, but UUL.
>
> That's what I meant. Actually, I think it's ULL. Regardless, I think
> the compiler will see the "1000000000 ... * 1000" and just combine
> them together. You're not actually outsmarting the compiler.
The compiler will do no such thing. That's a valid transformation when
doing pure math, but not when working with integers.
> >> > + err = 100000000;
> >>
> >> Why do you assign err to this arbitrary value?
> >
> > Dunno. It is Freescale's code and I do not have time to check
> > and understand each bit of it and to explain it.
>
> *sigh* You're not the first person to modify the DIU driver without
> understanding what it all does. I suspect that the original author
> should have done this:
>
> err = -1;
>
> because he wanted it to be the largest possible integer.
-1 is not the largest possible integer. LONG_MAX, perhaps?
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 16:22 ` Scott Wood
0 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 16:22 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, Apr 30, 2010 at 10:08:45AM -0500, Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust-ynQEQJNshbs@public.gmane.org> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
>
> >> I'm pretty sure the compiler will optimize this to:
> >>
> >> temp = (1000000000000UL / pixclock);
> >>
> >> so you may as well do it that way.
> >
> > ??
> > 1000000000000 is _not_ UL, but UUL.
>
> That's what I meant. Actually, I think it's ULL. Regardless, I think
> the compiler will see the "1000000000 ... * 1000" and just combine
> them together. You're not actually outsmarting the compiler.
The compiler will do no such thing. That's a valid transformation when
doing pure math, but not when working with integers.
> >> > + err = 100000000;
> >>
> >> Why do you assign err to this arbitrary value?
> >
> > Dunno. It is Freescale's code and I do not have time to check
> > and understand each bit of it and to explain it.
>
> *sigh* You're not the first person to modify the DIU driver without
> understanding what it all does. I suspect that the original author
> should have done this:
>
> err = -1;
>
> because he wanted it to be the largest possible integer.
-1 is not the largest possible integer. LONG_MAX, perhaps?
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 16:22 ` Scott Wood
0 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 16:22 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, Anatolij Gustschin, yorksun
On Fri, Apr 30, 2010 at 10:08:45AM -0500, Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
>
> >> I'm pretty sure the compiler will optimize this to:
> >>
> >> temp = (1000000000000UL / pixclock);
> >>
> >> so you may as well do it that way.
> >
> > ??
> > 1000000000000 is _not_ UL, but UUL.
>
> That's what I meant. Actually, I think it's ULL. Regardless, I think
> the compiler will see the "1000000000 ... * 1000" and just combine
> them together. You're not actually outsmarting the compiler.
The compiler will do no such thing. That's a valid transformation when
doing pure math, but not when working with integers.
> >> > + err = 100000000;
> >>
> >> Why do you assign err to this arbitrary value?
> >
> > Dunno. It is Freescale's code and I do not have time to check
> > and understand each bit of it and to explain it.
>
> *sigh* You're not the first person to modify the DIU driver without
> understanding what it all does. I suspect that the original author
> should have done this:
>
> err = -1;
>
> because he wanted it to be the largest possible integer.
-1 is not the largest possible integer. LONG_MAX, perhaps?
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
[parent not found: <20100430162254.GA24285-1MYqz8GpK7RekFaExTCHk1jVikpgYyvb5NbjCUgZEJk@public.gmane.org>]
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 16:22 ` Scott Wood
(?)
@ 2010-04-30 18:18 ` Timur Tabi
-1 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 18:18 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrote:
>> That's what I meant. Actually, I think it's ULL. Regardless, I think
>> the compiler will see the "1000000000 ... * 1000" and just combine
>> them together. You're not actually outsmarting the compiler.
>
> The compiler will do no such thing. That's a valid transformation when
> doing pure math, but not when working with integers.
I ran some tests, and it appears you're right. I doesn't make a lot
of sense to me, but whatever.
However, "(1000000000 / pixclock) * 1000" produces a result that's
less accurate than "1000000000000ULL / pixclock". Unfortunately, that
math caused a linker problem with __udivdi3 when I tried it, so maybe
you can use do_div() instead?
>> err = -1;
>>
>> because he wanted it to be the largest possible integer.
>
> -1 is not the largest possible integer. LONG_MAX, perhaps?
What, you don't like implicit casting of -1 to an unsigned? :-)
Since err is a long integer, LONG_MAX is the better choice.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 18:18 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 18:18 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> That's what I meant. Actually, I think it's ULL. Regardless, I think
>> the compiler will see the "1000000000 ... * 1000" and just combine
>> them together. You're not actually outsmarting the compiler.
>
> The compiler will do no such thing. That's a valid transformation when
> doing pure math, but not when working with integers.
I ran some tests, and it appears you're right. I doesn't make a lot
of sense to me, but whatever.
However, "(1000000000 / pixclock) * 1000" produces a result that's
less accurate than "1000000000000ULL / pixclock". Unfortunately, that
math caused a linker problem with __udivdi3 when I tried it, so maybe
you can use do_div() instead?
>> err = -1;
>>
>> because he wanted it to be the largest possible integer.
>
> -1 is not the largest possible integer. LONG_MAX, perhaps?
What, you don't like implicit casting of -1 to an unsigned? :-)
Since err is a long integer, LONG_MAX is the better choice.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 18:18 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 18:18 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, Anatolij Gustschin, yorksun
On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrot=
e:
>> That's what I meant. =A0Actually, I think it's ULL. =A0Regardless, I thi=
nk
>> the compiler will see the =A0"1000000000 ... * 1000" and just combine
>> them together. =A0You're not actually outsmarting the compiler.
>
> The compiler will do no such thing. =A0That's a valid transformation when
> doing pure math, but not when working with integers.
I ran some tests, and it appears you're right. I doesn't make a lot
of sense to me, but whatever.
However, "(1000000000 / pixclock) * 1000" produces a result that's
less accurate than "1000000000000ULL / pixclock". Unfortunately, that
math caused a linker problem with __udivdi3 when I tried it, so maybe
you can use do_div() instead?
>> =A0 =A0 err =3D -1;
>>
>> because he wanted it to be the largest possible integer.
>
> -1 is not the largest possible integer. =A0LONG_MAX, perhaps?
What, you don't like implicit casting of -1 to an unsigned? :-)
Since err is a long integer, LONG_MAX is the better choice.
--=20
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread
[parent not found: <r2sed82fe3e1004301118xc52b46cfi879de534283fd51-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 18:18 ` Timur Tabi
(?)
@ 2010-04-30 20:40 ` Scott Wood
-1 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 20:40 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrote:
>
>>> That's what I meant. Actually, I think it's ULL. Regardless, I think
>>> the compiler will see the "1000000000 ... * 1000" and just combine
>>> them together. You're not actually outsmarting the compiler.
>> The compiler will do no such thing. That's a valid transformation when
>> doing pure math, but not when working with integers.
>
> I ran some tests, and it appears you're right. I doesn't make a lot
> of sense to me, but whatever.
>
> However, "(1000000000 / pixclock) * 1000" produces a result that's
> less accurate than "1000000000000ULL / pixclock".
Precisely, that's what makes it a distinct computation -- as far as the
compiler knows, it could be intentional. Plus, turning it into 64-bit
math would invoke a library call for 64-bit division, which wouldn't be
much of an optimization anyway.
The question is whether the loss of accuracy matters in this case.
>>> err = -1;
>>>
>>> because he wanted it to be the largest possible integer.
>> -1 is not the largest possible integer. LONG_MAX, perhaps?
>
> What, you don't like implicit casting of -1 to an unsigned? :-)
I like it even less when the variable is signed and it's still supposed
to be larger than positive numbers. :-)
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 20:40 ` Scott Wood
0 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 20:40 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>
>>> That's what I meant. Actually, I think it's ULL. Regardless, I think
>>> the compiler will see the "1000000000 ... * 1000" and just combine
>>> them together. You're not actually outsmarting the compiler.
>> The compiler will do no such thing. That's a valid transformation when
>> doing pure math, but not when working with integers.
>
> I ran some tests, and it appears you're right. I doesn't make a lot
> of sense to me, but whatever.
>
> However, "(1000000000 / pixclock) * 1000" produces a result that's
> less accurate than "1000000000000ULL / pixclock".
Precisely, that's what makes it a distinct computation -- as far as the
compiler knows, it could be intentional. Plus, turning it into 64-bit
math would invoke a library call for 64-bit division, which wouldn't be
much of an optimization anyway.
The question is whether the loss of accuracy matters in this case.
>>> err = -1;
>>>
>>> because he wanted it to be the largest possible integer.
>> -1 is not the largest possible integer. LONG_MAX, perhaps?
>
> What, you don't like implicit casting of -1 to an unsigned? :-)
I like it even less when the variable is signed and it's still supposed
to be larger than positive numbers. :-)
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 20:40 ` Scott Wood
0 siblings, 0 replies; 84+ messages in thread
From: Scott Wood @ 2010-04-30 20:40 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, Anatolij Gustschin, yorksun
Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrote:
>
>>> That's what I meant. Actually, I think it's ULL. Regardless, I think
>>> the compiler will see the "1000000000 ... * 1000" and just combine
>>> them together. You're not actually outsmarting the compiler.
>> The compiler will do no such thing. That's a valid transformation when
>> doing pure math, but not when working with integers.
>
> I ran some tests, and it appears you're right. I doesn't make a lot
> of sense to me, but whatever.
>
> However, "(1000000000 / pixclock) * 1000" produces a result that's
> less accurate than "1000000000000ULL / pixclock".
Precisely, that's what makes it a distinct computation -- as far as the
compiler knows, it could be intentional. Plus, turning it into 64-bit
math would invoke a library call for 64-bit division, which wouldn't be
much of an optimization anyway.
The question is whether the loss of accuracy matters in this case.
>>> err = -1;
>>>
>>> because he wanted it to be the largest possible integer.
>> -1 is not the largest possible integer. LONG_MAX, perhaps?
>
> What, you don't like implicit casting of -1 to an unsigned? :-)
I like it even less when the variable is signed and it's still supposed
to be larger than positive numbers. :-)
-Scott
^ permalink raw reply [flat|nested] 84+ messages in thread
[parent not found: <4BDB402C.9080301-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 20:40 ` Scott Wood
(?)
@ 2010-05-01 15:15 ` Anatolij Gustschin
-1 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-05-01 15:15 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, 30 Apr 2010 15:40:12 -0500
Scott Wood <scottwood@freescale.com> wrote:
> Timur Tabi wrote:
> > On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrote:
> >
> >>> That's what I meant. Actually, I think it's ULL. Regardless, I think
> >>> the compiler will see the "1000000000 ... * 1000" and just combine
> >>> them together. You're not actually outsmarting the compiler.
> >> The compiler will do no such thing. That's a valid transformation when
> >> doing pure math, but not when working with integers.
> >
> > I ran some tests, and it appears you're right. I doesn't make a lot
> > of sense to me, but whatever.
> >
> > However, "(1000000000 / pixclock) * 1000" produces a result that's
> > less accurate than "1000000000000ULL / pixclock".
>
> Precisely, that's what makes it a distinct computation -- as far as the
> compiler knows, it could be intentional. Plus, turning it into 64-bit
> math would invoke a library call for 64-bit division, which wouldn't be
> much of an optimization anyway.
>
> The question is whether the loss of accuracy matters in this case.
Here, this loss of accuracy doesn't matter at all. Max. possible
loss by this current conversion is 999 HZ compared to conversion
using 64-bit division. Further computation tolerates 5% deviation
for pixclock and selects best possible value. This deviation is
by far greater than 999 HZ. It is 156862 HZ for lowest configurable
pixel clock.
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-05-01 15:15 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-05-01 15:15 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, 30 Apr 2010 15:40:12 -0500
Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Timur Tabi wrote:
> > On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> >
> >>> That's what I meant. Actually, I think it's ULL. Regardless, I think
> >>> the compiler will see the "1000000000 ... * 1000" and just combine
> >>> them together. You're not actually outsmarting the compiler.
> >> The compiler will do no such thing. That's a valid transformation when
> >> doing pure math, but not when working with integers.
> >
> > I ran some tests, and it appears you're right. I doesn't make a lot
> > of sense to me, but whatever.
> >
> > However, "(1000000000 / pixclock) * 1000" produces a result that's
> > less accurate than "1000000000000ULL / pixclock".
>
> Precisely, that's what makes it a distinct computation -- as far as the
> compiler knows, it could be intentional. Plus, turning it into 64-bit
> math would invoke a library call for 64-bit division, which wouldn't be
> much of an optimization anyway.
>
> The question is whether the loss of accuracy matters in this case.
Here, this loss of accuracy doesn't matter at all. Max. possible
loss by this current conversion is 999 HZ compared to conversion
using 64-bit division. Further computation tolerates 5% deviation
for pixclock and selects best possible value. This deviation is
by far greater than 999 HZ. It is 156862 HZ for lowest configurable
pixel clock.
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-05-01 15:15 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-05-01 15:15 UTC (permalink / raw)
To: Scott Wood
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, Timur Tabi, yorksun
On Fri, 30 Apr 2010 15:40:12 -0500
Scott Wood <scottwood@freescale.com> wrote:
> Timur Tabi wrote:
> > On Fri, Apr 30, 2010 at 11:22 AM, Scott Wood <scottwood@freescale.com> wrote:
> >
> >>> That's what I meant. Actually, I think it's ULL. Regardless, I think
> >>> the compiler will see the "1000000000 ... * 1000" and just combine
> >>> them together. You're not actually outsmarting the compiler.
> >> The compiler will do no such thing. That's a valid transformation when
> >> doing pure math, but not when working with integers.
> >
> > I ran some tests, and it appears you're right. I doesn't make a lot
> > of sense to me, but whatever.
> >
> > However, "(1000000000 / pixclock) * 1000" produces a result that's
> > less accurate than "1000000000000ULL / pixclock".
>
> Precisely, that's what makes it a distinct computation -- as far as the
> compiler knows, it could be intentional. Plus, turning it into 64-bit
> math would invoke a library call for 64-bit division, which wouldn't be
> much of an optimization anyway.
>
> The question is whether the loss of accuracy matters in this case.
Here, this loss of accuracy doesn't matter at all. Max. possible
loss by this current conversion is 999 HZ compared to conversion
using 64-bit division. Further computation tolerates 5% deviation
for pixclock and selects best possible value. This deviation is
by far greater than 999 HZ. It is 156862 HZ for lowest configurable
pixel clock.
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 15:08 ` Timur Tabi
(?)
@ 2010-04-30 17:00 ` Anatolij Gustschin
-1 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 17:00 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, 30 Apr 2010 10:08:45 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
Currently I do not have such plans. Ok will skip them.
...
> >> Do you really need to use reserve_bootmem? Have you tried kmalloc or
> >> alloc_pages_exact()?
> >
> > Yes. No, it is too early to use them here.
>
> There was a recent change in the kernel that allows kmalloc to work
> earlier than before. Take a look at commit
> 85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
> warning").
Thanks. Sorry for my wrong answer above, now I remember the logic
behind this and will try to explain. Actually the reason I do not
use kmalloc() here is that I do not want to _copy_ bitmap data to
newly allocated frame buffer area (It will negatively affect boot
time). Instead I reserve the already configured frame buffer area
so that it won't be destroyed. The starting address of the area
to reserve and also the lenght is passed to reserve_bootmem().
This is the real reason for using reserve_bootmem() here.
I could alloc new bitmap area using allocators, but then I have
to copy the bitmap data (splash image) to newly allocated area
and have to re-configure the descriptors to display from new
bitmap buffer.
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 17:00 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 17:00 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, wd-ynQEQJNshbs,
dzu-ynQEQJNshbs, John Rigby,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A,
yorksun-KZfg59tc24xl57MIdRCFDg
On Fri, 30 Apr 2010 10:08:45 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
Currently I do not have such plans. Ok will skip them.
...
> >> Do you really need to use reserve_bootmem? Have you tried kmalloc or
> >> alloc_pages_exact()?
> >
> > Yes. No, it is too early to use them here.
>
> There was a recent change in the kernel that allows kmalloc to work
> earlier than before. Take a look at commit
> 85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
> warning").
Thanks. Sorry for my wrong answer above, now I remember the logic
behind this and will try to explain. Actually the reason I do not
use kmalloc() here is that I do not want to _copy_ bitmap data to
newly allocated frame buffer area (It will negatively affect boot
time). Instead I reserve the already configured frame buffer area
so that it won't be destroyed. The starting address of the area
to reserve and also the lenght is passed to reserve_bootmem().
This is the real reason for using reserve_bootmem() here.
I could alloc new bitmap area using allocators, but then I have
to copy the bitmap data (splash image) to newly allocated area
and have to re-configure the descriptors to display from new
bitmap buffer.
Anatolij
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 17:00 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 17:00 UTC (permalink / raw)
To: Timur Tabi
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, 30 Apr 2010 10:08:45 -0500
Timur Tabi <timur.tabi@gmail.com> wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust@denx.de> wrote:
>=20
> >> How about just doing this?
> >>
> >> .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init=
_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>=20
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
Currently I do not have such plans. Ok will skip them.
...
> >> Do you really need to use reserve_bootmem? =C2=A0Have you tried kmallo=
c or
> >> alloc_pages_exact()?
> >
> > Yes. No, it is too early to use them here.
>=20
> There was a recent change in the kernel that allows kmalloc to work
> earlier than before. Take a look at commit
> 85355bb272db31a3f2dd99d547eef794805e1319 ("powerpc: Fix mpic alloc
> warning").
Thanks. Sorry for my wrong answer above, now I remember the logic
behind this and will try to explain. Actually the reason I do not
use kmalloc() here is that I do not want to _copy_ bitmap data to
newly allocated frame buffer area (It will negatively affect boot
time). Instead I reserve the already configured frame buffer area
so that it won't be destroyed. The starting address of the area
to reserve and also the lenght is passed to reserve_bootmem().
This is the real reason for using reserve_bootmem() here.
I could alloc new bitmap area using allocators, but then I have
to copy the bitmap data (splash image) to newly allocated area
and have to re-configure the descriptors to display from new
bitmap buffer.
Anatolij
^ permalink raw reply [flat|nested] 84+ messages in thread* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 17:00 ` Anatolij Gustschin
@ 2010-04-30 18:29 ` Timur Tabi
-1 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 18:29 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, Apr 30, 2010 at 12:00 PM, Anatolij Gustschin <agust@denx.de> wrote:
> Thanks. Sorry for my wrong answer above, now I remember the logic
> behind this and will try to explain. Actually the reason I do not
> use kmalloc() here is that I do not want to _copy_ bitmap data to
> newly allocated frame buffer area (It will negatively affect boot
> time). Instead I reserve the already configured frame buffer area
> so that it won't be destroyed. The starting address of the area
> to reserve and also the lenght is passed to reserve_bootmem().
> This is the real reason for using reserve_bootmem() here.
> I could alloc new bitmap area using allocators, but then I have
> to copy the bitmap data (splash image) to newly allocated area
> and have to re-configure the descriptors to display from new
> bitmap buffer.
Ok, I understand. Please add this comment to the code, so that no one
else will wonder what you're doing.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread
* Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 18:29 ` Timur Tabi
0 siblings, 0 replies; 84+ messages in thread
From: Timur Tabi @ 2010-04-30 18:29 UTC (permalink / raw)
To: Anatolij Gustschin
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss,
linuxppc-dev, yorksun
On Fri, Apr 30, 2010 at 12:00 PM, Anatolij Gustschin <agust@denx.de> wrote:
> Thanks. Sorry for my wrong answer above, now I remember the logic
> behind this and will try to explain. Actually the reason I do not
> use kmalloc() here is that I do not want to _copy_ bitmap data to
> newly allocated frame buffer area (It will negatively affect boot
> time). Instead I reserve the already configured frame buffer area
> so that it won't be destroyed. The starting address of the area
> to reserve and also the lenght is passed to reserve_bootmem().
> This is the real reason for using reserve_bootmem() here.
> I could alloc new bitmap area using allocators, but then I have
> to copy the bitmap data (splash image) to newly allocated area
> and have to re-configure the descriptors to display from new
> bitmap buffer.
Ok, I understand. Please add this comment to the code, so that no one
else will wonder what you're doing.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 84+ messages in thread
* [PATCH v2 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-29 23:49 ` Anatolij Gustschin
@ 2010-04-30 10:40 ` Anatolij Gustschin
-1 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 10:40 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss, yorksun
MPC5121 DIU configuration/setup as initialized by the boot
loader currently will get lost while booting Linux. As a
result displaying the boot splash is not possible through
the boot process.
To prevent this we reserve configured DIU frame buffer
address range while booting and preserve AOI descriptor
and gamma table so that DIU continues displaying through
the whole boot process. On first open from user space
DIU frame buffer driver releases the reserved frame
buffer area and continues to operate as usual.
Signed-off-by: John Rigby <jrigby@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
v1 -> v2:
- use struct for CCM register access, don't use offset macros,
so CCM struct definition is added now
- use struct for DIU descriptors access
- simplify code and correct variable types as suggested by Timur
arch/powerpc/include/asm/mpc5121.h | 32 +++
arch/powerpc/platforms/512x/mpc5121_ads.c | 7 +
arch/powerpc/platforms/512x/mpc5121_generic.c | 12 +
arch/powerpc/platforms/512x/mpc512x.h | 2 +
arch/powerpc/platforms/512x/mpc512x_shared.c | 274 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 1 +
drivers/video/fsl-diu-fb.c | 17 ++-
7 files changed, 343 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index e6a30bb..8c0ab2c 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -21,4 +21,36 @@ struct mpc512x_reset_module {
u32 rcer; /* Reset Control Enable Register */
};
+/*
+ * Clock Control Module
+ */
+struct mpc512x_ccm {
+ u32 spmr; /* System PLL Mode Register */
+ u32 sccr1; /* System Clock Control Register 1 */
+ u32 sccr2; /* System Clock Control Register 2 */
+ u32 scfr1; /* System Clock Frequency Register 1 */
+ u32 scfr2; /* System Clock Frequency Register 2 */
+ u32 scfr2s; /* System Clock Frequency Shadow Register 2 */
+ u32 bcr; /* Bread Crumb Register */
+ u32 p0ccr; /* PSC0 Clock Control Register */
+ u32 p1ccr; /* PSC1 CCR */
+ u32 p2ccr; /* PSC2 CCR */
+ u32 p3ccr; /* PSC3 CCR */
+ u32 p4ccr; /* PSC4 CCR */
+ u32 p5ccr; /* PSC5 CCR */
+ u32 p6ccr; /* PSC6 CCR */
+ u32 p7ccr; /* PSC7 CCR */
+ u32 p8ccr; /* PSC8 CCR */
+ u32 p9ccr; /* PSC9 CCR */
+ u32 p10ccr; /* PSC10 CCR */
+ u32 p11ccr; /* PSC11 CCR */
+ u32 spccr; /* SPDIF Clock Control Register */
+ u32 cccr; /* CFM Clock Control Register */
+ u32 dccr; /* DIU Clock Control Register */
+ u32 m1ccr; /* MSCAN1 CCR */
+ u32 m2ccr; /* MSCAN2 CCR */
+ u32 m3ccr; /* MSCAN3 CCR */
+ u32 m4ccr; /* MSCAN4 CCR */
+ u8 res[0x98]; /* Reserved */
+};
#endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index ee6ae12..082aa6f 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -42,6 +42,7 @@ static void __init mpc5121_ads_setup_arch(void)
for_each_compatible_node(np, "pci", "fsl,mpc5121-pci")
mpc83xx_add_bridge(np);
#endif
+ mpc512x_setup_diu();
}
static void __init mpc5121_ads_init_IRQ(void)
@@ -60,11 +61,17 @@ static int __init mpc5121_ads_probe(void)
return of_flat_dt_is_compatible(root, "fsl,mpc5121ads");
}
+void __init mpc5121_ads_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
define_machine(mpc5121_ads) {
.name = "MPC5121 ADS",
.probe = mpc5121_ads_probe,
.setup_arch = mpc5121_ads_setup_arch,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
.init_IRQ = mpc5121_ads_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index a6c0e3a..c5ecb3d 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -48,10 +48,22 @@ static int __init mpc5121_generic_probe(void)
return board[i] != NULL;
}
+void __init mpc512x_generic_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
+void __init mpc512x_generic_setup_arch(void)
+{
+ mpc512x_setup_diu();
+}
+
define_machine(mpc5121_generic) {
.name = "MPC5121 generic",
.probe = mpc5121_generic_probe,
.init = mpc512x_init,
+ .init_early = mpc512x_generic_init_early,
+ .setup_arch = mpc512x_generic_setup_arch,
.init_IRQ = mpc512x_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..1ab6d11 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
extern int __init mpc5121_clk_init(void);
void __init mpc512x_declare_of_platform_devices(void);
extern void mpc512x_restart(char *cmd);
+extern void mpc512x_init_diu(void);
+extern void mpc512x_setup_diu(void);
#endif /* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b7f518a..e5a8e8a 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -16,7 +16,11 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/of_platform.h>
+#include <linux/fsl-diu-fb.h>
+#include <linux/bootmem.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/cacheflush.h>
#include <asm/machdep.h>
#include <asm/ipic.h>
#include <asm/prom.h>
@@ -53,6 +57,276 @@ void mpc512x_restart(char *cmd)
;
}
+struct fsl_diu_shared_fb {
+ u8 gamma[0x300]; /* 32-bit aligned! */
+ struct diu_ad ad0; /* 32-bit aligned! */
+ phys_addr_t fb_phys;
+ size_t fb_len;
+ bool in_use;
+};
+
+unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
+ int monitor_port)
+{
+ switch (bits_per_pixel) {
+ case 32:
+ return 0x88883316;
+ case 24:
+ return 0x88082219;
+ case 16:
+ return 0x65053118;
+ }
+ return 0x00000400;
+}
+
+void mpc512x_set_gamma_table(int monitor_port, char *gamma_table_base)
+{
+}
+
+void mpc512x_set_monitor_port(int monitor_port)
+{
+}
+
+#define DIU_DIV_MASK 0x000000ff
+void mpc512x_set_pixel_clock(unsigned int pixclock)
+{
+ unsigned long bestval, bestfreq, speed_ccb, busfreq;
+ unsigned long minpixclock, maxpixclock, pixval;
+ struct mpc512x_ccm __iomem *ccm;
+ struct device_node *np;
+ u32 temp;
+ long err;
+ int i;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-clock");
+ if (!np) {
+ pr_err("Can't find clock control module.\n");
+ return;
+ }
+
+ ccm = of_iomap(np, 0);
+ of_node_put(np);
+ if (!ccm) {
+ pr_err("Can't map clock control module reg.\n");
+ return;
+ }
+
+ np = of_find_node_by_type(NULL, "cpu");
+ if (np) {
+ const unsigned int *prop + of_get_property(np, "bus-frequency", NULL);
+
+ of_node_put(np);
+ if (prop) {
+ busfreq = *prop;
+ } else {
+ pr_err("Can't get bus-frequency property\n");
+ return;
+ }
+ } else {
+ pr_err("Can't find 'cpu' node.\n");
+ return;
+ }
+
+ /* Pixel Clock configuration */
+ pr_debug("DIU: Bus Frequency = %lu\n", busfreq);
+ speed_ccb = busfreq * 4;
+
+ /* Calculate the pixel clock with the smallest error */
+ /* calculate the following in steps to avoid overflow */
+ pr_debug("DIU pixclock in ps - %d\n", pixclock);
+ temp = (1000000000 / pixclock) * 1000;
+ pixclock = temp;
+ pr_debug("DIU pixclock freq - %u\n", pixclock);
+
+ temp = temp / 20; /* pixclock * 0.05 */
+ pr_debug("deviation = %d\n", temp);
+ minpixclock = pixclock - temp;
+ maxpixclock = pixclock + temp;
+ pr_debug("DIU minpixclock - %lu\n", minpixclock);
+ pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
+ pixval = speed_ccb/pixclock;
+ pr_debug("DIU pixval = %lu\n", pixval);
+
+ err = 100000000;
+ bestval = pixval;
+ pr_debug("DIU bestval = %lu\n", bestval);
+
+ bestfreq = 0;
+ for (i = -1; i <= 1; i++) {
+ temp = speed_ccb / (pixval+i);
+ pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
+ i, pixval, temp);
+ if ((temp < minpixclock) || (temp > maxpixclock))
+ pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
+ minpixclock, maxpixclock);
+ else if (abs(temp - pixclock) < err) {
+ pr_debug("Entered the else if block %d\n", i);
+ err = abs(temp - pixclock);
+ bestval = pixval + i;
+ bestfreq = temp;
+ }
+ }
+
+ pr_debug("DIU chose = %lx\n", bestval);
+ pr_debug("DIU error = %ld\n NomPixClk ", err);
+ pr_debug("DIU: Best Freq = %lx\n", bestfreq);
+ /* Modify DIU_DIV in CCM SCFR1 */
+ temp = in_be32(&ccm->scfr1);
+ pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
+ temp &= ~DIU_DIV_MASK;
+ temp |= (bestval & DIU_DIV_MASK);
+ out_be32(&ccm->scfr1, temp);
+ pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
+ iounmap(ccm);
+}
+
+ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
+{
+ return sprintf(buf, "0 - 5121 LCD\n");
+}
+
+int mpc512x_set_sysfs_monitor_port(int val)
+{
+ return 0;
+}
+
+static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+static inline void mpc512x_free_bootmem(struct page *page)
+{
+ __ClearPageReserved(page);
+ BUG_ON(PageTail(page));
+ BUG_ON(atomic_read(&page->_count) > 1);
+ atomic_set(&page->_count, 1);
+ __free_page(page);
+ totalram_pages++;
+}
+
+void mpc512x_release_bootmem(void)
+{
+ unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
+ unsigned long size = diu_shared_fb.fb_len;
+ unsigned long start, end;
+
+ if (diu_shared_fb.in_use) {
+ start = PFN_UP(addr);
+ end = PFN_DOWN(addr + size);
+
+ for (; start < end; start++)
+ mpc512x_free_bootmem(pfn_to_page(start));
+
+ diu_shared_fb.in_use = false;
+ }
+ diu_ops.release_bootmem = NULL;
+}
+#endif
+
+/*
+ * Check if DIU was pre-initialized. If so, perform steps
+ * needed to continue displaying through the whole boot process.
+ * Move area descriptor and gamma table elsewhere, they are
+ * destroyed by bootmem allocator otherwise. The frame buffer
+ * address range will be reserved in setup_arch() after bootmem
+ * allocator is up.
+ */
+void __init mpc512x_init_diu(void)
+{
+ struct device_node *np;
+ struct diu __iomem *diu_reg;
+ phys_addr_t desc;
+ void __iomem *vaddr;
+ unsigned long mode, pix_fmt, res, bpp;
+ unsigned long dst;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
+ if (!np) {
+ pr_err("No DIU node\n");
+ return;
+ }
+
+ diu_reg = of_iomap(np, 0);
+ of_node_put(np);
+ if (!diu_reg) {
+ pr_err("Can't map DIU\n");
+ return;
+ }
+
+ mode = in_be32(&diu_reg->diu_mode);
+ if (mode != MFB_MODE1) {
+ pr_info("%s: DIU OFF\n", __func__);
+ goto out;
+ }
+
+ desc = in_be32(&diu_reg->desc[0]);
+ vaddr = ioremap(desc, sizeof(struct diu_ad));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ goto out;
+ }
+ memcpy(&diu_shared_fb.ad0, vaddr, sizeof(struct diu_ad));
+ /* flush fb area descriptor */
+ dst = (unsigned long)&diu_shared_fb.ad0;
+ flush_dcache_range(dst, dst + sizeof(struct diu_ad) - 1);
+
+ res = in_be32(&diu_reg->disp_size);
+ pix_fmt = in_le32(vaddr);
+ bpp = ((pix_fmt >> 16) & 0x3) + 1;
+ diu_shared_fb.fb_phys = in_le32(vaddr + 4);
+ diu_shared_fb.fb_len = ((res & 0xfff0000) >> 16) * (res & 0xfff) * bpp;
+ diu_shared_fb.in_use = true;
+ iounmap(vaddr);
+
+ desc = in_be32(&diu_reg->gamma);
+ vaddr = ioremap(desc, sizeof(diu_shared_fb.gamma));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ diu_shared_fb.in_use = false;
+ goto out;
+ }
+ memcpy(&diu_shared_fb.gamma, vaddr, sizeof(diu_shared_fb.gamma));
+ /* flush gamma table */
+ dst = (unsigned long)&diu_shared_fb.gamma;
+ flush_dcache_range(dst, dst + sizeof(diu_shared_fb.gamma) - 1);
+
+ iounmap(vaddr);
+ out_be32(&diu_reg->gamma, virt_to_phys(&diu_shared_fb.gamma));
+ out_be32(&diu_reg->desc[1], 0);
+ out_be32(&diu_reg->desc[2], 0);
+ out_be32(&diu_reg->desc[0], virt_to_phys(&diu_shared_fb.ad0));
+
+out:
+ iounmap(diu_reg);
+}
+
+void __init mpc512x_setup_diu(void)
+{
+ int ret;
+
+ if (diu_shared_fb.in_use) {
+ ret = reserve_bootmem(diu_shared_fb.fb_phys,
+ diu_shared_fb.fb_len,
+ BOOTMEM_EXCLUSIVE);
+ if (ret) {
+ pr_err("%s: reserve bootmem failed\n", __func__);
+ diu_shared_fb.in_use = false;
+ }
+ }
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+ diu_ops.get_pixel_format = mpc512x_get_pixel_format;
+ diu_ops.set_gamma_table = mpc512x_set_gamma_table;
+ diu_ops.set_monitor_port = mpc512x_set_monitor_port;
+ diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
+ diu_ops.show_monitor_port = mpc512x_show_monitor_port;
+ diu_ops.set_sysfs_monitor_port = mpc512x_set_sysfs_monitor_port;
+ diu_ops.release_bootmem = mpc512x_release_bootmem;
+#endif
+}
+
void __init mpc512x_init_IRQ(void)
{
struct device_node *np;
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 42381bb..5360948 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -30,6 +30,7 @@ struct platform_diu_data_ops {
void (*set_pixel_clock) (unsigned int pixclock);
ssize_t (*show_monitor_port) (int monitor_port, char *buf);
int (*set_sysfs_monitor_port) (int val);
+ void (*release_bootmem) (void);
};
extern struct platform_diu_data_ops diu_ops;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 7acdc09..81dec09 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1103,6 +1103,10 @@ static int fsl_diu_open(struct fb_info *info, int user)
struct mfb_info *mfbi = info->par;
int res = 0;
+ /* free boot splash memory on first /dev/fb0 open */
+ if (!mfbi->index && diu_ops.release_bootmem)
+ diu_ops.release_bootmem();
+
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count = 1) {
@@ -1430,6 +1434,7 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
int ret, i, error = 0;
struct resource res;
struct fsl_diu_data *machine_data;
+ int diu_mode;
machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
if (!machine_data)
@@ -1466,7 +1471,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
goto error2;
}
- out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
+ diu_mode = in_be32(&dr.diu_reg->diu_mode);
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
/* Get the IRQ of the DIU */
machine_data->irq = irq_of_parse_and_map(np, 0);
@@ -1514,7 +1521,13 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
machine_data->dummy_ad->offset_xyd = 0;
machine_data->dummy_ad->next_ad = 0;
- out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+ /*
+ * Let DIU display splash screen if it was pre-initialized
+ * by the bootloader, set dummy area descriptor otherwise.
+ */
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+
out_be32(&dr.diu_reg->desc[1], machine_data->dummy_ad->paddr);
out_be32(&dr.diu_reg->desc[2], machine_data->dummy_ad->paddr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 84+ messages in thread* [PATCH v2 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-04-30 10:40 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-04-30 10:40 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fbdev, wd, dzu, John Rigby, devicetree-discuss, yorksun
MPC5121 DIU configuration/setup as initialized by the boot
loader currently will get lost while booting Linux. As a
result displaying the boot splash is not possible through
the boot process.
To prevent this we reserve configured DIU frame buffer
address range while booting and preserve AOI descriptor
and gamma table so that DIU continues displaying through
the whole boot process. On first open from user space
DIU frame buffer driver releases the reserved frame
buffer area and continues to operate as usual.
Signed-off-by: John Rigby <jrigby@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
v1 -> v2:
- use struct for CCM register access, don't use offset macros,
so CCM struct definition is added now
- use struct for DIU descriptors access
- simplify code and correct variable types as suggested by Timur
arch/powerpc/include/asm/mpc5121.h | 32 +++
arch/powerpc/platforms/512x/mpc5121_ads.c | 7 +
arch/powerpc/platforms/512x/mpc5121_generic.c | 12 +
arch/powerpc/platforms/512x/mpc512x.h | 2 +
arch/powerpc/platforms/512x/mpc512x_shared.c | 274 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 1 +
drivers/video/fsl-diu-fb.c | 17 ++-
7 files changed, 343 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index e6a30bb..8c0ab2c 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -21,4 +21,36 @@ struct mpc512x_reset_module {
u32 rcer; /* Reset Control Enable Register */
};
+/*
+ * Clock Control Module
+ */
+struct mpc512x_ccm {
+ u32 spmr; /* System PLL Mode Register */
+ u32 sccr1; /* System Clock Control Register 1 */
+ u32 sccr2; /* System Clock Control Register 2 */
+ u32 scfr1; /* System Clock Frequency Register 1 */
+ u32 scfr2; /* System Clock Frequency Register 2 */
+ u32 scfr2s; /* System Clock Frequency Shadow Register 2 */
+ u32 bcr; /* Bread Crumb Register */
+ u32 p0ccr; /* PSC0 Clock Control Register */
+ u32 p1ccr; /* PSC1 CCR */
+ u32 p2ccr; /* PSC2 CCR */
+ u32 p3ccr; /* PSC3 CCR */
+ u32 p4ccr; /* PSC4 CCR */
+ u32 p5ccr; /* PSC5 CCR */
+ u32 p6ccr; /* PSC6 CCR */
+ u32 p7ccr; /* PSC7 CCR */
+ u32 p8ccr; /* PSC8 CCR */
+ u32 p9ccr; /* PSC9 CCR */
+ u32 p10ccr; /* PSC10 CCR */
+ u32 p11ccr; /* PSC11 CCR */
+ u32 spccr; /* SPDIF Clock Control Register */
+ u32 cccr; /* CFM Clock Control Register */
+ u32 dccr; /* DIU Clock Control Register */
+ u32 m1ccr; /* MSCAN1 CCR */
+ u32 m2ccr; /* MSCAN2 CCR */
+ u32 m3ccr; /* MSCAN3 CCR */
+ u32 m4ccr; /* MSCAN4 CCR */
+ u8 res[0x98]; /* Reserved */
+};
#endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index ee6ae12..082aa6f 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -42,6 +42,7 @@ static void __init mpc5121_ads_setup_arch(void)
for_each_compatible_node(np, "pci", "fsl,mpc5121-pci")
mpc83xx_add_bridge(np);
#endif
+ mpc512x_setup_diu();
}
static void __init mpc5121_ads_init_IRQ(void)
@@ -60,11 +61,17 @@ static int __init mpc5121_ads_probe(void)
return of_flat_dt_is_compatible(root, "fsl,mpc5121ads");
}
+void __init mpc5121_ads_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
define_machine(mpc5121_ads) {
.name = "MPC5121 ADS",
.probe = mpc5121_ads_probe,
.setup_arch = mpc5121_ads_setup_arch,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
.init_IRQ = mpc5121_ads_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index a6c0e3a..c5ecb3d 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -48,10 +48,22 @@ static int __init mpc5121_generic_probe(void)
return board[i] != NULL;
}
+void __init mpc512x_generic_init_early(void)
+{
+ mpc512x_init_diu();
+}
+
+void __init mpc512x_generic_setup_arch(void)
+{
+ mpc512x_setup_diu();
+}
+
define_machine(mpc5121_generic) {
.name = "MPC5121 generic",
.probe = mpc5121_generic_probe,
.init = mpc512x_init,
+ .init_early = mpc512x_generic_init_early,
+ .setup_arch = mpc512x_generic_setup_arch,
.init_IRQ = mpc512x_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..1ab6d11 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
extern int __init mpc5121_clk_init(void);
void __init mpc512x_declare_of_platform_devices(void);
extern void mpc512x_restart(char *cmd);
+extern void mpc512x_init_diu(void);
+extern void mpc512x_setup_diu(void);
#endif /* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b7f518a..e5a8e8a 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -16,7 +16,11 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/of_platform.h>
+#include <linux/fsl-diu-fb.h>
+#include <linux/bootmem.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/cacheflush.h>
#include <asm/machdep.h>
#include <asm/ipic.h>
#include <asm/prom.h>
@@ -53,6 +57,276 @@ void mpc512x_restart(char *cmd)
;
}
+struct fsl_diu_shared_fb {
+ u8 gamma[0x300]; /* 32-bit aligned! */
+ struct diu_ad ad0; /* 32-bit aligned! */
+ phys_addr_t fb_phys;
+ size_t fb_len;
+ bool in_use;
+};
+
+unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
+ int monitor_port)
+{
+ switch (bits_per_pixel) {
+ case 32:
+ return 0x88883316;
+ case 24:
+ return 0x88082219;
+ case 16:
+ return 0x65053118;
+ }
+ return 0x00000400;
+}
+
+void mpc512x_set_gamma_table(int monitor_port, char *gamma_table_base)
+{
+}
+
+void mpc512x_set_monitor_port(int monitor_port)
+{
+}
+
+#define DIU_DIV_MASK 0x000000ff
+void mpc512x_set_pixel_clock(unsigned int pixclock)
+{
+ unsigned long bestval, bestfreq, speed_ccb, busfreq;
+ unsigned long minpixclock, maxpixclock, pixval;
+ struct mpc512x_ccm __iomem *ccm;
+ struct device_node *np;
+ u32 temp;
+ long err;
+ int i;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-clock");
+ if (!np) {
+ pr_err("Can't find clock control module.\n");
+ return;
+ }
+
+ ccm = of_iomap(np, 0);
+ of_node_put(np);
+ if (!ccm) {
+ pr_err("Can't map clock control module reg.\n");
+ return;
+ }
+
+ np = of_find_node_by_type(NULL, "cpu");
+ if (np) {
+ const unsigned int *prop =
+ of_get_property(np, "bus-frequency", NULL);
+
+ of_node_put(np);
+ if (prop) {
+ busfreq = *prop;
+ } else {
+ pr_err("Can't get bus-frequency property\n");
+ return;
+ }
+ } else {
+ pr_err("Can't find 'cpu' node.\n");
+ return;
+ }
+
+ /* Pixel Clock configuration */
+ pr_debug("DIU: Bus Frequency = %lu\n", busfreq);
+ speed_ccb = busfreq * 4;
+
+ /* Calculate the pixel clock with the smallest error */
+ /* calculate the following in steps to avoid overflow */
+ pr_debug("DIU pixclock in ps - %d\n", pixclock);
+ temp = (1000000000 / pixclock) * 1000;
+ pixclock = temp;
+ pr_debug("DIU pixclock freq - %u\n", pixclock);
+
+ temp = temp / 20; /* pixclock * 0.05 */
+ pr_debug("deviation = %d\n", temp);
+ minpixclock = pixclock - temp;
+ maxpixclock = pixclock + temp;
+ pr_debug("DIU minpixclock - %lu\n", minpixclock);
+ pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
+ pixval = speed_ccb/pixclock;
+ pr_debug("DIU pixval = %lu\n", pixval);
+
+ err = 100000000;
+ bestval = pixval;
+ pr_debug("DIU bestval = %lu\n", bestval);
+
+ bestfreq = 0;
+ for (i = -1; i <= 1; i++) {
+ temp = speed_ccb / (pixval+i);
+ pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
+ i, pixval, temp);
+ if ((temp < minpixclock) || (temp > maxpixclock))
+ pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
+ minpixclock, maxpixclock);
+ else if (abs(temp - pixclock) < err) {
+ pr_debug("Entered the else if block %d\n", i);
+ err = abs(temp - pixclock);
+ bestval = pixval + i;
+ bestfreq = temp;
+ }
+ }
+
+ pr_debug("DIU chose = %lx\n", bestval);
+ pr_debug("DIU error = %ld\n NomPixClk ", err);
+ pr_debug("DIU: Best Freq = %lx\n", bestfreq);
+ /* Modify DIU_DIV in CCM SCFR1 */
+ temp = in_be32(&ccm->scfr1);
+ pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
+ temp &= ~DIU_DIV_MASK;
+ temp |= (bestval & DIU_DIV_MASK);
+ out_be32(&ccm->scfr1, temp);
+ pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
+ iounmap(ccm);
+}
+
+ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
+{
+ return sprintf(buf, "0 - 5121 LCD\n");
+}
+
+int mpc512x_set_sysfs_monitor_port(int val)
+{
+ return 0;
+}
+
+static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+static inline void mpc512x_free_bootmem(struct page *page)
+{
+ __ClearPageReserved(page);
+ BUG_ON(PageTail(page));
+ BUG_ON(atomic_read(&page->_count) > 1);
+ atomic_set(&page->_count, 1);
+ __free_page(page);
+ totalram_pages++;
+}
+
+void mpc512x_release_bootmem(void)
+{
+ unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
+ unsigned long size = diu_shared_fb.fb_len;
+ unsigned long start, end;
+
+ if (diu_shared_fb.in_use) {
+ start = PFN_UP(addr);
+ end = PFN_DOWN(addr + size);
+
+ for (; start < end; start++)
+ mpc512x_free_bootmem(pfn_to_page(start));
+
+ diu_shared_fb.in_use = false;
+ }
+ diu_ops.release_bootmem = NULL;
+}
+#endif
+
+/*
+ * Check if DIU was pre-initialized. If so, perform steps
+ * needed to continue displaying through the whole boot process.
+ * Move area descriptor and gamma table elsewhere, they are
+ * destroyed by bootmem allocator otherwise. The frame buffer
+ * address range will be reserved in setup_arch() after bootmem
+ * allocator is up.
+ */
+void __init mpc512x_init_diu(void)
+{
+ struct device_node *np;
+ struct diu __iomem *diu_reg;
+ phys_addr_t desc;
+ void __iomem *vaddr;
+ unsigned long mode, pix_fmt, res, bpp;
+ unsigned long dst;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
+ if (!np) {
+ pr_err("No DIU node\n");
+ return;
+ }
+
+ diu_reg = of_iomap(np, 0);
+ of_node_put(np);
+ if (!diu_reg) {
+ pr_err("Can't map DIU\n");
+ return;
+ }
+
+ mode = in_be32(&diu_reg->diu_mode);
+ if (mode != MFB_MODE1) {
+ pr_info("%s: DIU OFF\n", __func__);
+ goto out;
+ }
+
+ desc = in_be32(&diu_reg->desc[0]);
+ vaddr = ioremap(desc, sizeof(struct diu_ad));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ goto out;
+ }
+ memcpy(&diu_shared_fb.ad0, vaddr, sizeof(struct diu_ad));
+ /* flush fb area descriptor */
+ dst = (unsigned long)&diu_shared_fb.ad0;
+ flush_dcache_range(dst, dst + sizeof(struct diu_ad) - 1);
+
+ res = in_be32(&diu_reg->disp_size);
+ pix_fmt = in_le32(vaddr);
+ bpp = ((pix_fmt >> 16) & 0x3) + 1;
+ diu_shared_fb.fb_phys = in_le32(vaddr + 4);
+ diu_shared_fb.fb_len = ((res & 0xfff0000) >> 16) * (res & 0xfff) * bpp;
+ diu_shared_fb.in_use = true;
+ iounmap(vaddr);
+
+ desc = in_be32(&diu_reg->gamma);
+ vaddr = ioremap(desc, sizeof(diu_shared_fb.gamma));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ diu_shared_fb.in_use = false;
+ goto out;
+ }
+ memcpy(&diu_shared_fb.gamma, vaddr, sizeof(diu_shared_fb.gamma));
+ /* flush gamma table */
+ dst = (unsigned long)&diu_shared_fb.gamma;
+ flush_dcache_range(dst, dst + sizeof(diu_shared_fb.gamma) - 1);
+
+ iounmap(vaddr);
+ out_be32(&diu_reg->gamma, virt_to_phys(&diu_shared_fb.gamma));
+ out_be32(&diu_reg->desc[1], 0);
+ out_be32(&diu_reg->desc[2], 0);
+ out_be32(&diu_reg->desc[0], virt_to_phys(&diu_shared_fb.ad0));
+
+out:
+ iounmap(diu_reg);
+}
+
+void __init mpc512x_setup_diu(void)
+{
+ int ret;
+
+ if (diu_shared_fb.in_use) {
+ ret = reserve_bootmem(diu_shared_fb.fb_phys,
+ diu_shared_fb.fb_len,
+ BOOTMEM_EXCLUSIVE);
+ if (ret) {
+ pr_err("%s: reserve bootmem failed\n", __func__);
+ diu_shared_fb.in_use = false;
+ }
+ }
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+ diu_ops.get_pixel_format = mpc512x_get_pixel_format;
+ diu_ops.set_gamma_table = mpc512x_set_gamma_table;
+ diu_ops.set_monitor_port = mpc512x_set_monitor_port;
+ diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
+ diu_ops.show_monitor_port = mpc512x_show_monitor_port;
+ diu_ops.set_sysfs_monitor_port = mpc512x_set_sysfs_monitor_port;
+ diu_ops.release_bootmem = mpc512x_release_bootmem;
+#endif
+}
+
void __init mpc512x_init_IRQ(void)
{
struct device_node *np;
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 42381bb..5360948 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -30,6 +30,7 @@ struct platform_diu_data_ops {
void (*set_pixel_clock) (unsigned int pixclock);
ssize_t (*show_monitor_port) (int monitor_port, char *buf);
int (*set_sysfs_monitor_port) (int val);
+ void (*release_bootmem) (void);
};
extern struct platform_diu_data_ops diu_ops;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 7acdc09..81dec09 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1103,6 +1103,10 @@ static int fsl_diu_open(struct fb_info *info, int user)
struct mfb_info *mfbi = info->par;
int res = 0;
+ /* free boot splash memory on first /dev/fb0 open */
+ if (!mfbi->index && diu_ops.release_bootmem)
+ diu_ops.release_bootmem();
+
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count == 1) {
@@ -1430,6 +1434,7 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
int ret, i, error = 0;
struct resource res;
struct fsl_diu_data *machine_data;
+ int diu_mode;
machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
if (!machine_data)
@@ -1466,7 +1471,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
goto error2;
}
- out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
+ diu_mode = in_be32(&dr.diu_reg->diu_mode);
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
/* Get the IRQ of the DIU */
machine_data->irq = irq_of_parse_and_map(np, 0);
@@ -1514,7 +1521,13 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
machine_data->dummy_ad->offset_xyd = 0;
machine_data->dummy_ad->next_ad = 0;
- out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+ /*
+ * Let DIU display splash screen if it was pre-initialized
+ * by the bootloader, set dummy area descriptor otherwise.
+ */
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+
out_be32(&dr.diu_reg->desc[1], machine_data->dummy_ad->paddr);
out_be32(&dr.diu_reg->desc[2], machine_data->dummy_ad->paddr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 84+ messages in thread* [PATCH v3 3/5] powerpc/mpc5121: shared DIU framebuffer support
2010-04-30 10:40 ` Anatolij Gustschin
@ 2010-05-03 10:49 ` Anatolij Gustschin
-1 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-05-03 10:49 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fbdev, wd, dzu, John Rigby, Anatolij Gustschin, yorksun
MPC5121 DIU configuration/setup as initialized by the boot
loader currently will get lost while booting Linux. As a
result displaying the boot splash is not possible through
the boot process.
To prevent this we reserve configured DIU frame buffer
address range while booting and preserve AOI descriptor
and gamma table so that DIU continues displaying through
the whole boot process. On first open from user space
DIU frame buffer driver releases the reserved frame
buffer area and continues to operate as usual.
Signed-off-by: John Rigby <jrigby@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
v2 -> v3:
- remove generic wrappers for early_init() and
setup_arch() and assign needed callbacks directly
as requested by Timur
- add comment explaning the reason for using
reserve_bootmem()
- use less confusing name for "speed" variable in
mpc512x_set_pixel_clock() and add a comment. Also
initialize "err" variable to LONG_MAX.
arch/powerpc/include/asm/mpc5121.h | 32 +++
arch/powerpc/platforms/512x/mpc5121_ads.c | 2 +
arch/powerpc/platforms/512x/mpc5121_generic.c | 2 +
arch/powerpc/platforms/512x/mpc512x.h | 2 +
arch/powerpc/platforms/512x/mpc512x_shared.c | 284 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 1 +
drivers/video/fsl-diu-fb.c | 17 ++-
7 files changed, 338 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index e6a30bb..8c0ab2c 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -21,4 +21,36 @@ struct mpc512x_reset_module {
u32 rcer; /* Reset Control Enable Register */
};
+/*
+ * Clock Control Module
+ */
+struct mpc512x_ccm {
+ u32 spmr; /* System PLL Mode Register */
+ u32 sccr1; /* System Clock Control Register 1 */
+ u32 sccr2; /* System Clock Control Register 2 */
+ u32 scfr1; /* System Clock Frequency Register 1 */
+ u32 scfr2; /* System Clock Frequency Register 2 */
+ u32 scfr2s; /* System Clock Frequency Shadow Register 2 */
+ u32 bcr; /* Bread Crumb Register */
+ u32 p0ccr; /* PSC0 Clock Control Register */
+ u32 p1ccr; /* PSC1 CCR */
+ u32 p2ccr; /* PSC2 CCR */
+ u32 p3ccr; /* PSC3 CCR */
+ u32 p4ccr; /* PSC4 CCR */
+ u32 p5ccr; /* PSC5 CCR */
+ u32 p6ccr; /* PSC6 CCR */
+ u32 p7ccr; /* PSC7 CCR */
+ u32 p8ccr; /* PSC8 CCR */
+ u32 p9ccr; /* PSC9 CCR */
+ u32 p10ccr; /* PSC10 CCR */
+ u32 p11ccr; /* PSC11 CCR */
+ u32 spccr; /* SPDIF Clock Control Register */
+ u32 cccr; /* CFM Clock Control Register */
+ u32 dccr; /* DIU Clock Control Register */
+ u32 m1ccr; /* MSCAN1 CCR */
+ u32 m2ccr; /* MSCAN2 CCR */
+ u32 m3ccr; /* MSCAN3 CCR */
+ u32 m4ccr; /* MSCAN4 CCR */
+ u8 res[0x98]; /* Reserved */
+};
#endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index ee6ae12..dcef6ad 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -42,6 +42,7 @@ static void __init mpc5121_ads_setup_arch(void)
for_each_compatible_node(np, "pci", "fsl,mpc5121-pci")
mpc83xx_add_bridge(np);
#endif
+ mpc512x_setup_diu();
}
static void __init mpc5121_ads_init_IRQ(void)
@@ -65,6 +66,7 @@ define_machine(mpc5121_ads) {
.probe = mpc5121_ads_probe,
.setup_arch = mpc5121_ads_setup_arch,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
.init_IRQ = mpc5121_ads_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index a6c0e3a..e487eb0 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -52,6 +52,8 @@ define_machine(mpc5121_generic) {
.name = "MPC5121 generic",
.probe = mpc5121_generic_probe,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
+ .setup_arch = mpc512x_setup_diu,
.init_IRQ = mpc512x_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..1ab6d11 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
extern int __init mpc5121_clk_init(void);
void __init mpc512x_declare_of_platform_devices(void);
extern void mpc512x_restart(char *cmd);
+extern void mpc512x_init_diu(void);
+extern void mpc512x_setup_diu(void);
#endif /* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b7f518a..6203a34 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -16,7 +16,11 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/of_platform.h>
+#include <linux/fsl-diu-fb.h>
+#include <linux/bootmem.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/cacheflush.h>
#include <asm/machdep.h>
#include <asm/ipic.h>
#include <asm/prom.h>
@@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
;
}
+struct fsl_diu_shared_fb {
+ u8 gamma[0x300]; /* 32-bit aligned! */
+ struct diu_ad ad0; /* 32-bit aligned! */
+ phys_addr_t fb_phys;
+ size_t fb_len;
+ bool in_use;
+};
+
+unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
+ int monitor_port)
+{
+ switch (bits_per_pixel) {
+ case 32:
+ return 0x88883316;
+ case 24:
+ return 0x88082219;
+ case 16:
+ return 0x65053118;
+ }
+ return 0x00000400;
+}
+
+void mpc512x_set_gamma_table(int monitor_port, char *gamma_table_base)
+{
+}
+
+void mpc512x_set_monitor_port(int monitor_port)
+{
+}
+
+#define DIU_DIV_MASK 0x000000ff
+void mpc512x_set_pixel_clock(unsigned int pixclock)
+{
+ unsigned long bestval, bestfreq, speed, busfreq;
+ unsigned long minpixclock, maxpixclock, pixval;
+ struct mpc512x_ccm __iomem *ccm;
+ struct device_node *np;
+ u32 temp;
+ long err;
+ int i;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-clock");
+ if (!np) {
+ pr_err("Can't find clock control module.\n");
+ return;
+ }
+
+ ccm = of_iomap(np, 0);
+ of_node_put(np);
+ if (!ccm) {
+ pr_err("Can't map clock control module reg.\n");
+ return;
+ }
+
+ np = of_find_node_by_type(NULL, "cpu");
+ if (np) {
+ const unsigned int *prop + of_get_property(np, "bus-frequency", NULL);
+
+ of_node_put(np);
+ if (prop) {
+ busfreq = *prop;
+ } else {
+ pr_err("Can't get bus-frequency property\n");
+ return;
+ }
+ } else {
+ pr_err("Can't find 'cpu' node.\n");
+ return;
+ }
+
+ /* Pixel Clock configuration */
+ pr_debug("DIU: Bus Frequency = %lu\n", busfreq);
+ speed = busfreq * 4; /* DIU_DIV ratio is 4 * CSB_CLK / DIU_CLK */
+
+ /* Calculate the pixel clock with the smallest error */
+ /* calculate the following in steps to avoid overflow */
+ pr_debug("DIU pixclock in ps - %d\n", pixclock);
+ temp = (1000000000 / pixclock) * 1000;
+ pixclock = temp;
+ pr_debug("DIU pixclock freq - %u\n", pixclock);
+
+ temp = temp / 20; /* pixclock * 0.05 */
+ pr_debug("deviation = %d\n", temp);
+ minpixclock = pixclock - temp;
+ maxpixclock = pixclock + temp;
+ pr_debug("DIU minpixclock - %lu\n", minpixclock);
+ pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
+ pixval = speed/pixclock;
+ pr_debug("DIU pixval = %lu\n", pixval);
+
+ err = LONG_MAX;
+ bestval = pixval;
+ pr_debug("DIU bestval = %lu\n", bestval);
+
+ bestfreq = 0;
+ for (i = -1; i <= 1; i++) {
+ temp = speed / (pixval+i);
+ pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
+ i, pixval, temp);
+ if ((temp < minpixclock) || (temp > maxpixclock))
+ pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
+ minpixclock, maxpixclock);
+ else if (abs(temp - pixclock) < err) {
+ pr_debug("Entered the else if block %d\n", i);
+ err = abs(temp - pixclock);
+ bestval = pixval + i;
+ bestfreq = temp;
+ }
+ }
+
+ pr_debug("DIU chose = %lx\n", bestval);
+ pr_debug("DIU error = %ld\n NomPixClk ", err);
+ pr_debug("DIU: Best Freq = %lx\n", bestfreq);
+ /* Modify DIU_DIV in CCM SCFR1 */
+ temp = in_be32(&ccm->scfr1);
+ pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
+ temp &= ~DIU_DIV_MASK;
+ temp |= (bestval & DIU_DIV_MASK);
+ out_be32(&ccm->scfr1, temp);
+ pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
+ iounmap(ccm);
+}
+
+ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
+{
+ return sprintf(buf, "0 - 5121 LCD\n");
+}
+
+int mpc512x_set_sysfs_monitor_port(int val)
+{
+ return 0;
+}
+
+static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+static inline void mpc512x_free_bootmem(struct page *page)
+{
+ __ClearPageReserved(page);
+ BUG_ON(PageTail(page));
+ BUG_ON(atomic_read(&page->_count) > 1);
+ atomic_set(&page->_count, 1);
+ __free_page(page);
+ totalram_pages++;
+}
+
+void mpc512x_release_bootmem(void)
+{
+ unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
+ unsigned long size = diu_shared_fb.fb_len;
+ unsigned long start, end;
+
+ if (diu_shared_fb.in_use) {
+ start = PFN_UP(addr);
+ end = PFN_DOWN(addr + size);
+
+ for (; start < end; start++)
+ mpc512x_free_bootmem(pfn_to_page(start));
+
+ diu_shared_fb.in_use = false;
+ }
+ diu_ops.release_bootmem = NULL;
+}
+#endif
+
+/*
+ * Check if DIU was pre-initialized. If so, perform steps
+ * needed to continue displaying through the whole boot process.
+ * Move area descriptor and gamma table elsewhere, they are
+ * destroyed by bootmem allocator otherwise. The frame buffer
+ * address range will be reserved in setup_arch() after bootmem
+ * allocator is up.
+ */
+void __init mpc512x_init_diu(void)
+{
+ struct device_node *np;
+ struct diu __iomem *diu_reg;
+ phys_addr_t desc;
+ void __iomem *vaddr;
+ unsigned long mode, pix_fmt, res, bpp;
+ unsigned long dst;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
+ if (!np) {
+ pr_err("No DIU node\n");
+ return;
+ }
+
+ diu_reg = of_iomap(np, 0);
+ of_node_put(np);
+ if (!diu_reg) {
+ pr_err("Can't map DIU\n");
+ return;
+ }
+
+ mode = in_be32(&diu_reg->diu_mode);
+ if (mode != MFB_MODE1) {
+ pr_info("%s: DIU OFF\n", __func__);
+ goto out;
+ }
+
+ desc = in_be32(&diu_reg->desc[0]);
+ vaddr = ioremap(desc, sizeof(struct diu_ad));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ goto out;
+ }
+ memcpy(&diu_shared_fb.ad0, vaddr, sizeof(struct diu_ad));
+ /* flush fb area descriptor */
+ dst = (unsigned long)&diu_shared_fb.ad0;
+ flush_dcache_range(dst, dst + sizeof(struct diu_ad) - 1);
+
+ res = in_be32(&diu_reg->disp_size);
+ pix_fmt = in_le32(vaddr);
+ bpp = ((pix_fmt >> 16) & 0x3) + 1;
+ diu_shared_fb.fb_phys = in_le32(vaddr + 4);
+ diu_shared_fb.fb_len = ((res & 0xfff0000) >> 16) * (res & 0xfff) * bpp;
+ diu_shared_fb.in_use = true;
+ iounmap(vaddr);
+
+ desc = in_be32(&diu_reg->gamma);
+ vaddr = ioremap(desc, sizeof(diu_shared_fb.gamma));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ diu_shared_fb.in_use = false;
+ goto out;
+ }
+ memcpy(&diu_shared_fb.gamma, vaddr, sizeof(diu_shared_fb.gamma));
+ /* flush gamma table */
+ dst = (unsigned long)&diu_shared_fb.gamma;
+ flush_dcache_range(dst, dst + sizeof(diu_shared_fb.gamma) - 1);
+
+ iounmap(vaddr);
+ out_be32(&diu_reg->gamma, virt_to_phys(&diu_shared_fb.gamma));
+ out_be32(&diu_reg->desc[1], 0);
+ out_be32(&diu_reg->desc[2], 0);
+ out_be32(&diu_reg->desc[0], virt_to_phys(&diu_shared_fb.ad0));
+
+out:
+ iounmap(diu_reg);
+}
+
+void __init mpc512x_setup_diu(void)
+{
+ int ret;
+
+ /*
+ * We do not allocate and configure new area for bitmap buffer
+ * because it would requere copying bitmap data (splash image)
+ * and so negatively affect boot time. Instead we reserve the
+ * already configured frame buffer area so that it won't be
+ * destroyed. The starting address of the area to reserve and
+ * also it's length is passed to reserve_bootmem(). It will be
+ * freed later on first open of fbdev, when splash image is not
+ * needed any more.
+ */
+ if (diu_shared_fb.in_use) {
+ ret = reserve_bootmem(diu_shared_fb.fb_phys,
+ diu_shared_fb.fb_len,
+ BOOTMEM_EXCLUSIVE);
+ if (ret) {
+ pr_err("%s: reserve bootmem failed\n", __func__);
+ diu_shared_fb.in_use = false;
+ }
+ }
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+ diu_ops.get_pixel_format = mpc512x_get_pixel_format;
+ diu_ops.set_gamma_table = mpc512x_set_gamma_table;
+ diu_ops.set_monitor_port = mpc512x_set_monitor_port;
+ diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
+ diu_ops.show_monitor_port = mpc512x_show_monitor_port;
+ diu_ops.set_sysfs_monitor_port = mpc512x_set_sysfs_monitor_port;
+ diu_ops.release_bootmem = mpc512x_release_bootmem;
+#endif
+}
+
void __init mpc512x_init_IRQ(void)
{
struct device_node *np;
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 42381bb..5360948 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -30,6 +30,7 @@ struct platform_diu_data_ops {
void (*set_pixel_clock) (unsigned int pixclock);
ssize_t (*show_monitor_port) (int monitor_port, char *buf);
int (*set_sysfs_monitor_port) (int val);
+ void (*release_bootmem) (void);
};
extern struct platform_diu_data_ops diu_ops;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 7acdc09..81dec09 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1103,6 +1103,10 @@ static int fsl_diu_open(struct fb_info *info, int user)
struct mfb_info *mfbi = info->par;
int res = 0;
+ /* free boot splash memory on first /dev/fb0 open */
+ if (!mfbi->index && diu_ops.release_bootmem)
+ diu_ops.release_bootmem();
+
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count = 1) {
@@ -1430,6 +1434,7 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
int ret, i, error = 0;
struct resource res;
struct fsl_diu_data *machine_data;
+ int diu_mode;
machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
if (!machine_data)
@@ -1466,7 +1471,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
goto error2;
}
- out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
+ diu_mode = in_be32(&dr.diu_reg->diu_mode);
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
/* Get the IRQ of the DIU */
machine_data->irq = irq_of_parse_and_map(np, 0);
@@ -1514,7 +1521,13 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
machine_data->dummy_ad->offset_xyd = 0;
machine_data->dummy_ad->next_ad = 0;
- out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+ /*
+ * Let DIU display splash screen if it was pre-initialized
+ * by the bootloader, set dummy area descriptor otherwise.
+ */
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+
out_be32(&dr.diu_reg->desc[1], machine_data->dummy_ad->paddr);
out_be32(&dr.diu_reg->desc[2], machine_data->dummy_ad->paddr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 84+ messages in thread* [PATCH v3 3/5] powerpc/mpc5121: shared DIU framebuffer support
@ 2010-05-03 10:49 ` Anatolij Gustschin
0 siblings, 0 replies; 84+ messages in thread
From: Anatolij Gustschin @ 2010-05-03 10:49 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fbdev, wd, dzu, John Rigby, Anatolij Gustschin, yorksun
MPC5121 DIU configuration/setup as initialized by the boot
loader currently will get lost while booting Linux. As a
result displaying the boot splash is not possible through
the boot process.
To prevent this we reserve configured DIU frame buffer
address range while booting and preserve AOI descriptor
and gamma table so that DIU continues displaying through
the whole boot process. On first open from user space
DIU frame buffer driver releases the reserved frame
buffer area and continues to operate as usual.
Signed-off-by: John Rigby <jrigby@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
v2 -> v3:
- remove generic wrappers for early_init() and
setup_arch() and assign needed callbacks directly
as requested by Timur
- add comment explaning the reason for using
reserve_bootmem()
- use less confusing name for "speed" variable in
mpc512x_set_pixel_clock() and add a comment. Also
initialize "err" variable to LONG_MAX.
arch/powerpc/include/asm/mpc5121.h | 32 +++
arch/powerpc/platforms/512x/mpc5121_ads.c | 2 +
arch/powerpc/platforms/512x/mpc5121_generic.c | 2 +
arch/powerpc/platforms/512x/mpc512x.h | 2 +
arch/powerpc/platforms/512x/mpc512x_shared.c | 284 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 1 +
drivers/video/fsl-diu-fb.c | 17 ++-
7 files changed, 338 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index e6a30bb..8c0ab2c 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -21,4 +21,36 @@ struct mpc512x_reset_module {
u32 rcer; /* Reset Control Enable Register */
};
+/*
+ * Clock Control Module
+ */
+struct mpc512x_ccm {
+ u32 spmr; /* System PLL Mode Register */
+ u32 sccr1; /* System Clock Control Register 1 */
+ u32 sccr2; /* System Clock Control Register 2 */
+ u32 scfr1; /* System Clock Frequency Register 1 */
+ u32 scfr2; /* System Clock Frequency Register 2 */
+ u32 scfr2s; /* System Clock Frequency Shadow Register 2 */
+ u32 bcr; /* Bread Crumb Register */
+ u32 p0ccr; /* PSC0 Clock Control Register */
+ u32 p1ccr; /* PSC1 CCR */
+ u32 p2ccr; /* PSC2 CCR */
+ u32 p3ccr; /* PSC3 CCR */
+ u32 p4ccr; /* PSC4 CCR */
+ u32 p5ccr; /* PSC5 CCR */
+ u32 p6ccr; /* PSC6 CCR */
+ u32 p7ccr; /* PSC7 CCR */
+ u32 p8ccr; /* PSC8 CCR */
+ u32 p9ccr; /* PSC9 CCR */
+ u32 p10ccr; /* PSC10 CCR */
+ u32 p11ccr; /* PSC11 CCR */
+ u32 spccr; /* SPDIF Clock Control Register */
+ u32 cccr; /* CFM Clock Control Register */
+ u32 dccr; /* DIU Clock Control Register */
+ u32 m1ccr; /* MSCAN1 CCR */
+ u32 m2ccr; /* MSCAN2 CCR */
+ u32 m3ccr; /* MSCAN3 CCR */
+ u32 m4ccr; /* MSCAN4 CCR */
+ u8 res[0x98]; /* Reserved */
+};
#endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
index ee6ae12..dcef6ad 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
@@ -42,6 +42,7 @@ static void __init mpc5121_ads_setup_arch(void)
for_each_compatible_node(np, "pci", "fsl,mpc5121-pci")
mpc83xx_add_bridge(np);
#endif
+ mpc512x_setup_diu();
}
static void __init mpc5121_ads_init_IRQ(void)
@@ -65,6 +66,7 @@ define_machine(mpc5121_ads) {
.probe = mpc5121_ads_probe,
.setup_arch = mpc5121_ads_setup_arch,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
.init_IRQ = mpc5121_ads_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
index a6c0e3a..e487eb0 100644
--- a/arch/powerpc/platforms/512x/mpc5121_generic.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -52,6 +52,8 @@ define_machine(mpc5121_generic) {
.name = "MPC5121 generic",
.probe = mpc5121_generic_probe,
.init = mpc512x_init,
+ .init_early = mpc512x_init_diu,
+ .setup_arch = mpc512x_setup_diu,
.init_IRQ = mpc512x_init_IRQ,
.get_irq = ipic_get_irq,
.calibrate_decr = generic_calibrate_decr,
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..1ab6d11 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,6 @@ extern void __init mpc512x_init(void);
extern int __init mpc5121_clk_init(void);
void __init mpc512x_declare_of_platform_devices(void);
extern void mpc512x_restart(char *cmd);
+extern void mpc512x_init_diu(void);
+extern void mpc512x_setup_diu(void);
#endif /* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index b7f518a..6203a34 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -16,7 +16,11 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/of_platform.h>
+#include <linux/fsl-diu-fb.h>
+#include <linux/bootmem.h>
+#include <sysdev/fsl_soc.h>
+#include <asm/cacheflush.h>
#include <asm/machdep.h>
#include <asm/ipic.h>
#include <asm/prom.h>
@@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)
;
}
+struct fsl_diu_shared_fb {
+ u8 gamma[0x300]; /* 32-bit aligned! */
+ struct diu_ad ad0; /* 32-bit aligned! */
+ phys_addr_t fb_phys;
+ size_t fb_len;
+ bool in_use;
+};
+
+unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,
+ int monitor_port)
+{
+ switch (bits_per_pixel) {
+ case 32:
+ return 0x88883316;
+ case 24:
+ return 0x88082219;
+ case 16:
+ return 0x65053118;
+ }
+ return 0x00000400;
+}
+
+void mpc512x_set_gamma_table(int monitor_port, char *gamma_table_base)
+{
+}
+
+void mpc512x_set_monitor_port(int monitor_port)
+{
+}
+
+#define DIU_DIV_MASK 0x000000ff
+void mpc512x_set_pixel_clock(unsigned int pixclock)
+{
+ unsigned long bestval, bestfreq, speed, busfreq;
+ unsigned long minpixclock, maxpixclock, pixval;
+ struct mpc512x_ccm __iomem *ccm;
+ struct device_node *np;
+ u32 temp;
+ long err;
+ int i;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-clock");
+ if (!np) {
+ pr_err("Can't find clock control module.\n");
+ return;
+ }
+
+ ccm = of_iomap(np, 0);
+ of_node_put(np);
+ if (!ccm) {
+ pr_err("Can't map clock control module reg.\n");
+ return;
+ }
+
+ np = of_find_node_by_type(NULL, "cpu");
+ if (np) {
+ const unsigned int *prop =
+ of_get_property(np, "bus-frequency", NULL);
+
+ of_node_put(np);
+ if (prop) {
+ busfreq = *prop;
+ } else {
+ pr_err("Can't get bus-frequency property\n");
+ return;
+ }
+ } else {
+ pr_err("Can't find 'cpu' node.\n");
+ return;
+ }
+
+ /* Pixel Clock configuration */
+ pr_debug("DIU: Bus Frequency = %lu\n", busfreq);
+ speed = busfreq * 4; /* DIU_DIV ratio is 4 * CSB_CLK / DIU_CLK */
+
+ /* Calculate the pixel clock with the smallest error */
+ /* calculate the following in steps to avoid overflow */
+ pr_debug("DIU pixclock in ps - %d\n", pixclock);
+ temp = (1000000000 / pixclock) * 1000;
+ pixclock = temp;
+ pr_debug("DIU pixclock freq - %u\n", pixclock);
+
+ temp = temp / 20; /* pixclock * 0.05 */
+ pr_debug("deviation = %d\n", temp);
+ minpixclock = pixclock - temp;
+ maxpixclock = pixclock + temp;
+ pr_debug("DIU minpixclock - %lu\n", minpixclock);
+ pr_debug("DIU maxpixclock - %lu\n", maxpixclock);
+ pixval = speed/pixclock;
+ pr_debug("DIU pixval = %lu\n", pixval);
+
+ err = LONG_MAX;
+ bestval = pixval;
+ pr_debug("DIU bestval = %lu\n", bestval);
+
+ bestfreq = 0;
+ for (i = -1; i <= 1; i++) {
+ temp = speed / (pixval+i);
+ pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n",
+ i, pixval, temp);
+ if ((temp < minpixclock) || (temp > maxpixclock))
+ pr_debug("DIU exceeds monitor range (%lu to %lu)\n",
+ minpixclock, maxpixclock);
+ else if (abs(temp - pixclock) < err) {
+ pr_debug("Entered the else if block %d\n", i);
+ err = abs(temp - pixclock);
+ bestval = pixval + i;
+ bestfreq = temp;
+ }
+ }
+
+ pr_debug("DIU chose = %lx\n", bestval);
+ pr_debug("DIU error = %ld\n NomPixClk ", err);
+ pr_debug("DIU: Best Freq = %lx\n", bestfreq);
+ /* Modify DIU_DIV in CCM SCFR1 */
+ temp = in_be32(&ccm->scfr1);
+ pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp);
+ temp &= ~DIU_DIV_MASK;
+ temp |= (bestval & DIU_DIV_MASK);
+ out_be32(&ccm->scfr1, temp);
+ pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp);
+ iounmap(ccm);
+}
+
+ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)
+{
+ return sprintf(buf, "0 - 5121 LCD\n");
+}
+
+int mpc512x_set_sysfs_monitor_port(int val)
+{
+ return 0;
+}
+
+static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+static inline void mpc512x_free_bootmem(struct page *page)
+{
+ __ClearPageReserved(page);
+ BUG_ON(PageTail(page));
+ BUG_ON(atomic_read(&page->_count) > 1);
+ atomic_set(&page->_count, 1);
+ __free_page(page);
+ totalram_pages++;
+}
+
+void mpc512x_release_bootmem(void)
+{
+ unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;
+ unsigned long size = diu_shared_fb.fb_len;
+ unsigned long start, end;
+
+ if (diu_shared_fb.in_use) {
+ start = PFN_UP(addr);
+ end = PFN_DOWN(addr + size);
+
+ for (; start < end; start++)
+ mpc512x_free_bootmem(pfn_to_page(start));
+
+ diu_shared_fb.in_use = false;
+ }
+ diu_ops.release_bootmem = NULL;
+}
+#endif
+
+/*
+ * Check if DIU was pre-initialized. If so, perform steps
+ * needed to continue displaying through the whole boot process.
+ * Move area descriptor and gamma table elsewhere, they are
+ * destroyed by bootmem allocator otherwise. The frame buffer
+ * address range will be reserved in setup_arch() after bootmem
+ * allocator is up.
+ */
+void __init mpc512x_init_diu(void)
+{
+ struct device_node *np;
+ struct diu __iomem *diu_reg;
+ phys_addr_t desc;
+ void __iomem *vaddr;
+ unsigned long mode, pix_fmt, res, bpp;
+ unsigned long dst;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu");
+ if (!np) {
+ pr_err("No DIU node\n");
+ return;
+ }
+
+ diu_reg = of_iomap(np, 0);
+ of_node_put(np);
+ if (!diu_reg) {
+ pr_err("Can't map DIU\n");
+ return;
+ }
+
+ mode = in_be32(&diu_reg->diu_mode);
+ if (mode != MFB_MODE1) {
+ pr_info("%s: DIU OFF\n", __func__);
+ goto out;
+ }
+
+ desc = in_be32(&diu_reg->desc[0]);
+ vaddr = ioremap(desc, sizeof(struct diu_ad));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ goto out;
+ }
+ memcpy(&diu_shared_fb.ad0, vaddr, sizeof(struct diu_ad));
+ /* flush fb area descriptor */
+ dst = (unsigned long)&diu_shared_fb.ad0;
+ flush_dcache_range(dst, dst + sizeof(struct diu_ad) - 1);
+
+ res = in_be32(&diu_reg->disp_size);
+ pix_fmt = in_le32(vaddr);
+ bpp = ((pix_fmt >> 16) & 0x3) + 1;
+ diu_shared_fb.fb_phys = in_le32(vaddr + 4);
+ diu_shared_fb.fb_len = ((res & 0xfff0000) >> 16) * (res & 0xfff) * bpp;
+ diu_shared_fb.in_use = true;
+ iounmap(vaddr);
+
+ desc = in_be32(&diu_reg->gamma);
+ vaddr = ioremap(desc, sizeof(diu_shared_fb.gamma));
+ if (!vaddr) {
+ pr_err("Can't map DIU area desc.\n");
+ diu_shared_fb.in_use = false;
+ goto out;
+ }
+ memcpy(&diu_shared_fb.gamma, vaddr, sizeof(diu_shared_fb.gamma));
+ /* flush gamma table */
+ dst = (unsigned long)&diu_shared_fb.gamma;
+ flush_dcache_range(dst, dst + sizeof(diu_shared_fb.gamma) - 1);
+
+ iounmap(vaddr);
+ out_be32(&diu_reg->gamma, virt_to_phys(&diu_shared_fb.gamma));
+ out_be32(&diu_reg->desc[1], 0);
+ out_be32(&diu_reg->desc[2], 0);
+ out_be32(&diu_reg->desc[0], virt_to_phys(&diu_shared_fb.ad0));
+
+out:
+ iounmap(diu_reg);
+}
+
+void __init mpc512x_setup_diu(void)
+{
+ int ret;
+
+ /*
+ * We do not allocate and configure new area for bitmap buffer
+ * because it would requere copying bitmap data (splash image)
+ * and so negatively affect boot time. Instead we reserve the
+ * already configured frame buffer area so that it won't be
+ * destroyed. The starting address of the area to reserve and
+ * also it's length is passed to reserve_bootmem(). It will be
+ * freed later on first open of fbdev, when splash image is not
+ * needed any more.
+ */
+ if (diu_shared_fb.in_use) {
+ ret = reserve_bootmem(diu_shared_fb.fb_phys,
+ diu_shared_fb.fb_len,
+ BOOTMEM_EXCLUSIVE);
+ if (ret) {
+ pr_err("%s: reserve bootmem failed\n", __func__);
+ diu_shared_fb.in_use = false;
+ }
+ }
+
+#if defined(CONFIG_FB_FSL_DIU) || \
+ defined(CONFIG_FB_FSL_DIU_MODULE)
+ diu_ops.get_pixel_format = mpc512x_get_pixel_format;
+ diu_ops.set_gamma_table = mpc512x_set_gamma_table;
+ diu_ops.set_monitor_port = mpc512x_set_monitor_port;
+ diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
+ diu_ops.show_monitor_port = mpc512x_show_monitor_port;
+ diu_ops.set_sysfs_monitor_port = mpc512x_set_sysfs_monitor_port;
+ diu_ops.release_bootmem = mpc512x_release_bootmem;
+#endif
+}
+
void __init mpc512x_init_IRQ(void)
{
struct device_node *np;
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 42381bb..5360948 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -30,6 +30,7 @@ struct platform_diu_data_ops {
void (*set_pixel_clock) (unsigned int pixclock);
ssize_t (*show_monitor_port) (int monitor_port, char *buf);
int (*set_sysfs_monitor_port) (int val);
+ void (*release_bootmem) (void);
};
extern struct platform_diu_data_ops diu_ops;
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 7acdc09..81dec09 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1103,6 +1103,10 @@ static int fsl_diu_open(struct fb_info *info, int user)
struct mfb_info *mfbi = info->par;
int res = 0;
+ /* free boot splash memory on first /dev/fb0 open */
+ if (!mfbi->index && diu_ops.release_bootmem)
+ diu_ops.release_bootmem();
+
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count == 1) {
@@ -1430,6 +1434,7 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
int ret, i, error = 0;
struct resource res;
struct fsl_diu_data *machine_data;
+ int diu_mode;
machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
if (!machine_data)
@@ -1466,7 +1471,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
goto error2;
}
- out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
+ diu_mode = in_be32(&dr.diu_reg->diu_mode);
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
/* Get the IRQ of the DIU */
machine_data->irq = irq_of_parse_and_map(np, 0);
@@ -1514,7 +1521,13 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
machine_data->dummy_ad->offset_xyd = 0;
machine_data->dummy_ad->next_ad = 0;
- out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+ /*
+ * Let DIU display splash screen if it was pre-initialized
+ * by the bootloader, set dummy area descriptor otherwise.
+ */
+ if (diu_mode != MFB_MODE1)
+ out_be32(&dr.diu_reg->desc[0], machine_data->dummy_ad->paddr);
+
out_be32(&dr.diu_reg->desc[1], machine_data->dummy_ad->paddr);
out_be32(&dr.diu_reg->desc[2], machine_data->dummy_ad->paddr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 84+ messages in thread