Re: [PATCH 68/90] ARM: OMAP: FB sync with N800 tree (support for dynamic SRAM allocations)

Randy Dunlap
2007-04-04T20:52:11Z

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 ***