linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandre Rusev <arusev@ru.mvista.com>
To: eric miao <eric.y.miao@gmail.com>
Cc: linux-fbdev-devel@lists.sourceforge.net,
	linux-kernel <linux-arm-kernel@lists.arm.linux.org.uk>
Subject: Re: [PATCH 0/8] pxafb cleanup
Date: Wed, 27 Feb 2008 12:25:12 +0300	[thread overview]
Message-ID: <47C52C78.1020503@ru.mvista.com> (raw)
In-Reply-To: <f17812d70802261832l78840912r4dcb0403e9fe4385@mail.gmail.com>

eric miao wrote:
> The following series of patches try to address several issues of the
> current PXA framebuffer driver. Some of these patches are already
> submitted for review, here are more:
>
> pxa: un-nest pxafb_parse_options() to cleanup the coding style issue
> pxa: fix various coding style issues for pxafb
> pxa: purge unnecessary pr_debug and comments from pxafb
> pxa: sanitize the usage of #ifdef .. processing pxafb parameters
> pxa: convert fb driver to use ioremap() and __raw_{readl, writel}
> pxa: introduce "struct pxafb_dma_buff" for palette and dma descriptors
> pxa: introduce register independent LCD connection type for pxafb
> pxa: make lubbock/mainstone/zylonite/littleton to use new LCD connection type
>
> Board maintainers are encouraged to use the new way for LCD connection
> types.
>
> To further clean-up the driver, I suggest to make the following changes:
> 1. removal of set_ctrlr_state(), the original code is written so because
> the fb_blank() is called within interrupt context in 2.4 kernel, but this is
> no longer the case in 2.6. And states can be simplified.
>
> 2. introduce a "vram" module parameter or boot option to indicate the
> size of the video memory, so that frame buffer offset can be specified
> within this video memory range, and PAN_DISPLAY can be done.
> And the start offset of each overlay can also be specified.
>
> 3. a "pxafb_layer" structure describing each possible layers,
> base, overlay1 and overlay2 for pxa27x can be described. From this
> layer information, frame buffer device can be dynamically allocated
> and registered. Palette manipulation, parameter checking and
> layer activation code can be generalized.
>   
Some considerations on dynamic allocation of framebuffer/overlys memory
(if applicable).
Early implementation of framebuffer/overlays for pxa270 contained the
following bug:
when resolution of framebuffer/overlay or bpp  was changed and that
framebuffer/overlay
(or some part of it) was mapped by userspace application the driver just
invoke kfree/dma_free_writecombine,
with the memory area still leaving mapped to userspace!
This could cause the corruption of kernel memory.
(for example DirectFB just tried to draw at the previously mapped area
so no more new picture appeared at screen;) )

When we do NOTallocate maximal possible sized piece of memory for
baseplane and overlay in advance
this problem seems still to be an issue :(
I suppose that driver-specific vm_area_ops to be attached by driver to
each mapping did by userland
due to avoid kernel memory corruption at unmap.

Yet the problem is that if userland applicatein frequently use/unuse and
change resolution/bpp of overlay
and forget to unmap it each time, then all consistent/writecombine
memory would be locked by this application.


Allocating maximal possible size (and not using it) is quite adversary
against writecombine/consistent memory pool.
With current definition of CONSISTENT_SIZE for architecture it would be
even impossible to allocate
all the needed memory for all overlays! :(

















-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

  parent reply	other threads:[~2008-02-27  9:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-27  2:32 [PATCH 0/8] pxafb cleanup eric miao
2008-02-27  8:58 ` Alexandre Rusev
2008-02-27 10:08   ` eric miao
2008-02-27  9:36     ` Alexandre Rusev
2008-02-27  9:25 ` Alexandre Rusev [this message]
2008-02-28  0:26   ` eric miao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47C52C78.1020503@ru.mvista.com \
    --to=arusev@ru.mvista.com \
    --cc=eric.y.miao@gmail.com \
    --cc=linux-arm-kernel@lists.arm.linux.org.uk \
    --cc=linux-fbdev-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).