David, as akpm suggested, may this patch will solve the dcache aliasing problem?
I will give you a introduction:
The user mmaped page (got by __get_free_page()) is cfag12864b_buffer.
The kernel only access it for reading at the same function 1 or 2 times:
1? memcmp() it against the cache, so we can tell if we must update the screen
2? if true, memcpy() the buffer to the cache buffer
So, if we want the kernel to know the last state of the data, we should call
flush_dcache_page() just once before we access it, right?
The relevant code:
flush_dcache_page(virt_to_page(cfag12864b_buffer));
if (memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
/***... update using cfag12864b_cache ...***/
}
You know, I can't test this stuff ;) so please review and check if it is right.
Thanks you.
---
- remove the "depends on x86" as it is portable again
- memcpy() buffer to cache, then update from cache, not buffer,
This way we only read the mmapped buffer 2 times.
- add a flush_dcache_page() to flush the user mmaped page so
the kernel has the last written data before accessing it.
drivers/auxdisplay/Kconfig | 1 -
drivers/auxdisplay/cfag12864b.c | 9 +++++----
2 files changed, 5 insertions(+), 5 deletions(-)
drivers-add-lcd-support-dcache.patch
Signed-off-by: Miguel Ojeda Sandonis <[email protected]>
---
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 8d41f72..ee30c48 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -64,7 +64,6 @@ config KS0108_DELAY
config CFAG12864B
tristate "CFAG12864B LCD"
- depends on X86
depends on KS0108
default n
---help---
diff --git a/drivers/auxdisplay/cfag12864b.c b/drivers/auxdisplay/cfag12864b.c
index 7b3c9ab..a654d54 100644
--- a/drivers/auxdisplay/cfag12864b.c
+++ b/drivers/auxdisplay/cfag12864b.c
@@ -37,7 +37,7 @@
#include <linux/workqueue.h>
#include <linux/ks0108.h>
#include <linux/cfag12864b.h>
-
+#include <asm/cacheflush.h>
#define CFAG12864B_NAME "cfag12864b"
@@ -272,7 +272,10 @@ static void cfag12864b_update(void *arg)
unsigned char c;
unsigned short i, j, k, b;
+ flush_dcache_page(virt_to_page(cfag12864b_buffer));
if (memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
+ memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
+
for (i = 0; i < CFAG12864B_CONTROLLERS; i++) {
cfag12864b_controller(i);
cfag12864b_nop();
@@ -283,7 +286,7 @@ static void cfag12864b_update(void *arg)
cfag12864b_nop();
for (k = 0; k < CFAG12864B_ADDRESSES; k++) {
for (c = 0, b = 0; b < 8; b++)
- if (cfag12864b_buffer
+ if (cfag12864b_cache
[i * CFAG12864B_ADDRESSES / 8
+ k / 8 + (j * 8 + b) *
CFAG12864B_WIDTH / 8]
@@ -293,8 +296,6 @@ static void cfag12864b_update(void *arg)
}
}
}
-
- memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
}
if (cfag12864b_updating)
David, as akpm suggested, may this patch will solve the dcache aliasing problem?
I will give you a introduction:
The user mmaped page (got by __get_free_page()) is cfag12864b_buffer.
The kernel only access it for reading at the same function 1 or 2 times:
1? memcmp() it against the cache, so we can tell if we must update the screen
2? if true, memcpy() the buffer to the cache buffer
So, if we want the kernel to know the last state of the data, we should call
flush_dcache_page() just once before we access it, right?
The relevant code:
flush_dcache_page(virt_to_page(cfag12864b_buffer));
if (memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
/***... update using cfag12864b_cache ...***/
}
You know, I can't test this stuff ;) so please review and check if it is right.
Thanks you.
---
- remove the "depends on x86" as it is portable again
- memcpy() buffer to cache, then update from cache, not buffer,
This way we only read the mmapped buffer 2 times.
- add a flush_dcache_page() to flush the user mmaped page so
the kernel has the last written data before accessing it.
drivers/auxdisplay/Kconfig | 1 -
drivers/auxdisplay/cfag12864b.c | 9 +++++----
2 files changed, 5 insertions(+), 5 deletions(-)
drivers-add-lcd-support-dcache.patch
Signed-off-by: Miguel Ojeda Sandonis <[email protected]>
---
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 8d41f72..ee30c48 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -64,7 +64,6 @@ config KS0108_DELAY
config CFAG12864B
tristate "CFAG12864B LCD"
- depends on X86
depends on KS0108
default n
---help---
diff --git a/drivers/auxdisplay/cfag12864b.c b/drivers/auxdisplay/cfag12864b.c
index 7b3c9ab..a654d54 100644
--- a/drivers/auxdisplay/cfag12864b.c
+++ b/drivers/auxdisplay/cfag12864b.c
@@ -37,7 +37,7 @@
#include <linux/workqueue.h>
#include <linux/ks0108.h>
#include <linux/cfag12864b.h>
-
+#include <asm/cacheflush.h>
#define CFAG12864B_NAME "cfag12864b"
@@ -272,7 +272,10 @@ static void cfag12864b_update(void *arg)
unsigned char c;
unsigned short i, j, k, b;
+ flush_dcache_page(virt_to_page(cfag12864b_buffer));
if (memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
+ memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
+
for (i = 0; i < CFAG12864B_CONTROLLERS; i++) {
cfag12864b_controller(i);
cfag12864b_nop();
@@ -283,7 +286,7 @@ static void cfag12864b_update(void *arg)
cfag12864b_nop();
for (k = 0; k < CFAG12864B_ADDRESSES; k++) {
for (c = 0, b = 0; b < 8; b++)
- if (cfag12864b_buffer
+ if (cfag12864b_cache
[i *
CFAG12864B_ADDRESSES / 8
+ k / 8 + (j * 8 + b) *
CFAG12864B_WIDTH / 8]
@@ -293,8 +296,6 @@ static void cfag12864b_update(void *arg)
}
}
}
-
- memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
}
if (cfag12864b_updating)
--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm
On Sat, 11 Nov 2006 11:03:51 +0100 Miguel Ojeda wrote:
> David, as akpm suggested, may this patch will solve the dcache aliasing problem?
>
> I will give you a introduction:
>
> The user mmaped page (got by __get_free_page()) is cfag12864b_buffer.
>
> The kernel only access it for reading at the same function 1 or 2 times:
>
> 1? memcmp() it against the cache, so we can tell if we must update the screen
> 2? if true, memcpy() the buffer to the cache buffer
>
> So, if we want the kernel to know the last state of the data, we should call
> flush_dcache_page() just once before we access it, right?
>
> The relevant code:
>
> flush_dcache_page(virt_to_page(cfag12864b_buffer));
> if (memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
> memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
>
> /***... update using cfag12864b_cache ...***/
> }
>
> You know, I can't test this stuff ;) so please review and check if it is right.
>
> Thanks you.
> ---
>
> - remove the "depends on x86" as it is portable again
>
> - memcpy() buffer to cache, then update from cache, not buffer,
> This way we only read the mmapped buffer 2 times.
>
> - add a flush_dcache_page() to flush the user mmaped page so
> the kernel has the last written data before accessing it.
>
> drivers/auxdisplay/Kconfig | 1 -
> drivers/auxdisplay/cfag12864b.c | 9 +++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> drivers-add-lcd-support-dcache.patch
> Signed-off-by: Miguel Ojeda Sandonis <[email protected]>
> ---
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index 8d41f72..ee30c48 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -64,7 +64,6 @@ config KS0108_DELAY
>
> config CFAG12864B
> tristate "CFAG12864B LCD"
> - depends on X86
> depends on KS0108
> default n
> ---help---
Hi,
Shouldn't the framebuffer part of this code (cfag12864bfb) also
depend on CONFIG_FB? Without that, this build error occurs:
cfag12864bfb.c:(.init.text+0xc19d): undefined reference to `framebuffer_alloc'
cfag12864bfb.c:(.init.text+0xc211): undefined reference to `register_framebuffer'
cfag12864bfb.c:(.text+0xf2782): undefined reference to `unregister_framebuffer'
cfag12864bfb.c:(.text+0xf2789): undefined reference to `framebuffer_release'
(from 2.6.20-rc1-mm1)
so you may need to modify the Kconfig and Makefile to have a
separate config entry for cfag12864bfb (or is it always required?).
And while you are there (in the Kconfig file), please change
(hertzs) to (hertz).
---
~Randy
On 12/18/06, Randy Dunlap <[email protected]> wrote:
>
> Hi,
>
> Shouldn't the framebuffer part of this code (cfag12864bfb) also
> depend on CONFIG_FB? Without that, this build error occurs:
>
Indeed it should. Thanks for noticing that!
> cfag12864bfb.c:(.init.text+0xc19d): undefined reference to `framebuffer_alloc'
> cfag12864bfb.c:(.init.text+0xc211): undefined reference to `register_framebuffer'
> cfag12864bfb.c:(.text+0xf2782): undefined reference to `unregister_framebuffer'
> cfag12864bfb.c:(.text+0xf2789): undefined reference to `framebuffer_release'
>
> (from 2.6.20-rc1-mm1)
>
> so you may need to modify the Kconfig and Makefile to have a
> separate config entry for cfag12864bfb (or is it always required?).
>
For normal users, yes, it is the only way (yet) to use cfag12864b
(through a framebuffer). However, I divided the code in logical blocks
so it is more modular (easier understanding, scalability...), so
anyone can easily add a new way to control the LCD without worring
about other unrelated stuff the code.
>
> And while you are there (in the Kconfig file), please change
> (hertzs) to (hertz).
>
Thanks again.
--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm