Messages in this thread Patch in this message | | | Date | Wed, 4 Apr 2007 13:52:54 -0700 | From | Randy Dunlap <> | Subject | Re: [PATCH 68/90] ARM: OMAP: FB sync with N800 tree (support for dynamic SRAM allocations) |
| |
On Wed, 4 Apr 2007 14:05:47 -0400 Tony Lindgren wrote:
Referring to multiple places in this patch, please use the preferred kernel "long comment" style as documented in Documentation/CodingStyle, Ch. 8, Commenting:
The preferred style for long (multi-line) comments is:
/* * This is the preferred style for multi-line * comments in the Linux kernel source code. * Please use it consistently. * * Description: A column of asterisks on the left side, * with beginning and ending almost-blank lines. */
> +/* Get the region_idx`th region from board config/ATAG and convert it to > + * our internal format. > + */ > +static int get_fbmem_region(int region_idx, struct omapfb_mem_region *rg) > +{ > + const struct omap_fbmem_config *conf; > + u32 paddr; > > - if (i == OMAPFB_PLANE_NUM) { > - printk(KERN_ERR "ignoring extra plane info\n"); > + conf = omap_get_nr_config(OMAP_TAG_FBMEM, > + struct omap_fbmem_config, region_idx); > + if (conf == NULL) > + return -ENOENT; > + > + paddr = conf->start; > + /* Low bits encode the page allocation mode, if high bits > + * are zero. Otherwise we need a page aligned fixed > + * address. > + */ > + memset(rg, 0, sizeof(*rg)); > + rg->type = paddr & ~PAGE_MASK; > + rg->paddr = paddr & PAGE_MASK; > + rg->size = PAGE_ALIGN(conf->size); > + return 0; > +} > + > +static int set_fbmem_region_type(struct omapfb_mem_region *rg, int mem_type, > + unsigned long mem_start, > + unsigned long mem_size) > +{ > + /* Check if the configuration specifies the type explicitly. > + * type = 0 && paddr = 0, a default don't care case maps to > + * the SDRAM type. > + */ > + if (rg->type || (!rg->type && !rg->paddr)) > + return 0; > + if (ranges_overlap(rg->paddr, rg->size, mem_start, mem_size)) { > + rg->type = mem_type; > + return 0; > + } > + /* Can't determine it. */ > + return -1; > +} > + > +/* Called from map_io. We need to call to this early enough so that we > + * can reserve the fixed SDRAM regions before VM could get hold of them. > + */ > +void omapfb_reserve_sdram(void) > +{ > + struct bootmem_data *bdata; > + unsigned long sdram_start, sdram_size; > + unsigned long reserved; > + int i; > + > + if (config_invalid) > + return; > + > + bdata = NODE_DATA(0)->bdata; > + sdram_start = bdata->node_boot_start; > + sdram_size = (bdata->node_low_pfn << PAGE_SHIFT) - sdram_start; > + reserved = 0; > + for (i = 0; ; i++) { > + struct omapfb_mem_region rg; > + > + if (get_fbmem_region(i, &rg) < 0) > break; > + if (i == OMAPFB_PLANE_NUM) { > + printk(KERN_ERR > + "Extraneous FB mem configuration entries\n"); > + config_invalid = 1; > + return; > } > - start = fbmem_conf->start; > - size = fbmem_conf->size; > - omapfb_config.mem_desc.region[i].paddr = start; > - omapfb_config.mem_desc.region[i].size = size; > - if (omap_fb_sram_plane != i && start) { > - reserve_bootmem(start, size); > - total_size += size; > + /* Check if it's our memory type. */ > + if (set_fbmem_region_type(&rg, OMAPFB_MEMTYPE_SDRAM, > + sdram_start, sdram_size) < 0 || > + (rg.type != OMAPFB_MEMTYPE_SDRAM)) > + continue; > + BUG_ON(omapfb_config.mem_desc.region[i].size); > + if (check_fbmem_region(i, &rg, sdram_start, sdram_size) < 0) { > + config_invalid = 1; > + return; > } > - i++; > + if (rg.paddr) > + reserve_bootmem(rg.paddr, rg.size); > + reserved += rg.size; > + omapfb_config.mem_desc.region[i] = rg; > + configured_regions++; > } > omapfb_config.mem_desc.region_cnt = i; > - if (total_size) > + if (reserved) > pr_info("Reserving %lu bytes SDRAM for frame buffer\n", > - total_size); > + reserved); > +} > > +/* Called at sram init time, before anything is pushed to the SRAM stack. > + * Because of the stack scheme, we will allocate everything from the > + * start of the lowest address region to the end of SRAM. This will also > + * include padding for page alignment and possible holes between regions. > + * > + * As opposed to the SDRAM case, we'll also do any dynamic allocations at > + * this point, since the driver built as a module would have problem with > + * freeing / reallocating the regions. > + */ > +unsigned long omapfb_reserve_sram(unsigned long sram_pstart, > + unsigned long sram_vstart, > + unsigned long sram_size, > + unsigned long pstart_avail, > + unsigned long size_avail) > +{ > + struct omapfb_mem_region rg; > + unsigned long pend_avail; > + unsigned long reserved; > + int i; > + > + if (config_invalid) > + return 0; > + > + reserved = 0; > + pend_avail = pstart_avail + size_avail; > + for (i = 0; ; i++) { > + if (get_fbmem_region(i, &rg) < 0) > + break; > + if (i == OMAPFB_PLANE_NUM) { > + printk(KERN_ERR > + "Extraneous FB mem configuration entries\n"); > + config_invalid = 1; > + return 0; > + } > + > + /* Check if it's our memory type. */ > + if (set_fbmem_region_type(&rg, OMAPFB_MEMTYPE_SRAM, > + sram_pstart, sram_size) < 0 || > + (rg.type != OMAPFB_MEMTYPE_SRAM)) > + continue; > + BUG_ON(omapfb_config.mem_desc.region[i].size); > + > + if (check_fbmem_region(i, &rg, pstart_avail, size_avail) < 0) { > + config_invalid = 1; > + return 0; > + } > + > + if (!rg.paddr) { > + /* Dynamic allocation */ > + if ((size_avail & PAGE_MASK) < rg.size) { > + printk("Not enough SRAM for FB region %d\n", > + i); > + config_invalid = 1; > + return 0; > + } > + size_avail = (size_avail - rg.size) & PAGE_MASK; > + rg.paddr = pstart_avail + size_avail; > + } > + /* Reserve everything above the start of the region. > + */ > + if (pend_avail - rg.paddr > reserved) > + reserved = pend_avail - rg.paddr; > + size_avail = pend_avail - reserved - pstart_avail; > + > + /* We have a kernel mapping for this already, so the > + * driver won't have to make one. > + */ > + rg.vaddr = (void *)(sram_vstart + rg.paddr - sram_pstart); > + omapfb_config.mem_desc.region[i] = rg; > + configured_regions++; > + } > + omapfb_config.mem_desc.region_cnt = i; > + if (reserved) > + pr_info("Reserving %lu bytes SRAM for frame buffer\n", > + reserved); > + return reserved; > } > > void omapfb_set_ctrl_platform_data(void *data) > @@ -104,10 +300,19 @@ static inline int omap_init_fb(void) > { > const struct omap_lcd_config *conf; > > + if (config_invalid) > + return 0; > + if (configured_regions != omapfb_config.mem_desc.region_cnt) { > + printk(KERN_ERR "Invalid FB mem configuration entries\n"); > + return 0; > + } > conf = omap_get_config(OMAP_TAG_LCD, struct omap_lcd_config); > - if (conf == NULL) > + if (conf == NULL) { > + if (configured_regions) > + /* FB mem config, but no LCD config? */ > + printk(KERN_ERR "Missing LCD configuration\n"); > return 0; > - > + } > omapfb_config.lcd = *conf; > > return platform_device_register(&omap_fb_device);
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |