Active Topics
-
The N900 is now 15 years old! (8)
to Nokia N900 by whayong - 15 hrs, 2 mins ago -
Sailfish OS for the Motorola Moto G7 Power (XT1955-5) - (ocean) (4)
to SailfishOS by Maemish - 5 days, 11 hrs ago - more...
|
2011-10-08
, 21:57
|
|
Posts: 2,154 |
Thanked: 8,464 times |
Joined on May 2010
|
#113
|
First thanks for patches!
My comments:
0001-RX51-bootmenu-add-clr-command-to-clear-the-screen.patch.txt - Please add definitions directly to include/config_cmd_default.h and include/config_cmd_all.h (not into rx51)
0002-RX51-cfb_console-fix-cursor_fix-checks.patch.txt 0003-RX51-cfb_console-console_clear_line-fix-refacored-li.patch.txt - Thanks, this is reason why sometimes I got some bad pixels. I melt this patch into my series.
0004-drivers-video-cfb-console.c-Fix-function-console_scr.patch 0005-cfb_console-fix-console_scrollup.patch.txt - patches deleted (see below)
0006-RX51-add-command-to-set-screen-backlight.patch - there is missing some files in this patch. But we do not want to add new specific command only for n900 (not usefull for upstream). And I tested this patch and it break brightness in Maemo kernel. When I used this patch I was not able to change brightness in Maemo (via controlpanel or sysfs). Can you look at this error?
===
Correct implementation of function console_scrollup is:
memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, CONSOLE_SCROLL_SIZE);
CONSOLE_SCROLL_SIZE is full, not without first 2 bits! This is working fine now.
When I finish my patch series, I will commit all into gitorious
My comments:
0001-RX51-bootmenu-add-clr-command-to-clear-the-screen.patch.txt - Please add definitions directly to include/config_cmd_default.h and include/config_cmd_all.h (not into rx51)
0002-RX51-cfb_console-fix-cursor_fix-checks.patch.txt 0003-RX51-cfb_console-console_clear_line-fix-refacored-li.patch.txt - Thanks, this is reason why sometimes I got some bad pixels. I melt this patch into my series.
0004-drivers-video-cfb-console.c-Fix-function-console_scr.patch 0005-cfb_console-fix-console_scrollup.patch.txt - patches deleted (see below)
0006-RX51-add-command-to-set-screen-backlight.patch - there is missing some files in this patch. But we do not want to add new specific command only for n900 (not usefull for upstream). And I tested this patch and it break brightness in Maemo kernel. When I used this patch I was not able to change brightness in Maemo (via controlpanel or sysfs). Can you look at this error?
===
Correct implementation of function console_scrollup is:
memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, CONSOLE_SCROLL_SIZE);
CONSOLE_SCROLL_SIZE is full, not without first 2 bits! This is working fine now.
When I finish my patch series, I will commit all into gitorious
|
|
2011-10-09
, 10:11
|
|
Posts: 197 |
Thanked: 101 times |
Joined on Dec 2009
@ Netherlands
|
#114
|
Originally Posted by pali
I see what you mean. See attachment for a new version.
0001-RX51-bootmenu-add-clr-command-to-clear-the-screen.patch.txt - Please add definitions directly to include/config_cmd_default.h and include/config_cmd_all.h (not into rx51)
To enable the command for rx51 I guess you still need to add the define to rx51.h manually.
Originally Posted by pali
Yes that is why I added the big WARNING. I noticed the brightness problem too but when I set backlight to 0 just before booting a kernel this problem seems to be gone.
0006-RX51-add-command-to-set-screen-backlight.patch - there is missing some files in this patch. But we do not want to add new specific command only for n900 (not usefull for upstream). And I tested this patch and it break brightness in Maemo kernel. When I used this patch I was not able to change brightness in Maemo (via controlpanel or sysfs). Can you look at this error?
What files are you missing?
The reason that I added it in the n900 specific part is that I'm not sure if the command is useful on other devices. I guess that is the reason you added the regulator patches to the rx51.c file too. If they are generic enough I guess the regulator and backlight code could be moved to power/twl4030.c
or friends.
Originally Posted by pali
CONSOLE_SCROLL_SIZE reflects the number of bytes in the scroll area. but memcpyl() moves ints! To reflect the weirdness in this I think you should still use
Correct implementation of function console_scrollup is:
memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, CONSOLE_SCROLL_SIZE);
CONSOLE_SCROLL_SIZE is full, not without first 2 bits! This is working fine now.
memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, 4*CONSOLE_SCROLL_SIZE >> 2);
PS: maybe you should consider pushing the ANSI and bootmenu patches to denx asap. They are quite useful for everybody right now!
And great you figured about the regulator stuff to enable the mmc cards!
| The Following 2 Users Say Thank You to mirakels For This Useful Post: | ||
|
|
2011-10-09
, 10:27
|
|
Posts: 362 |
Thanked: 426 times |
Joined on Nov 2010
@ Italy, Lombardia
|
#115
|
These future N900 u-boot's images will have a working saveenv command or other solutions to persist modification to env ?
Last edited by Fabry; 2011-10-09 at 10:29.
Last edited by Fabry; 2011-10-09 at 10:29.
|
|
2011-10-09
, 10:50
|
|
Posts: 2,154 |
Thanked: 8,464 times |
Joined on May 2010
|
#116
|
Originally Posted by Fabry
no. but it eMMC part1 (MyDocs) now is workinf so it can be used for this. (but someone must write patch which will use this) but I think that saveenv is not needed...
These future N900 u-boot's images will have a working saveenv command or other solutions to persist modification to env ?
I'm prepairing rx51 config to load bootmenu configuration from eMMC part1 file bootmenu.scr. So changing bootmenu entires wil not need to recompile u-boot. So in Maemo (or in all other systems) will be ulility update-uboot which update bootmenu.scr file in eMMC (MyDocs). We can use similar configuration file like in bootmenu/multiboot for configuring menu entires. And for security, if keyboard will be closed this file will be ignored (so if somebody damage this file, still can boot maemo5 in internal flash)
|
|
2011-10-09
, 11:02
|
|
Posts: 2,154 |
Thanked: 8,464 times |
Joined on May 2010
|
#117
|
Originally Posted by mirakels
yes, ok. it is needed in rx51 and cmd_all
I see what you mean. See attachment for a new version.
To enable the command for rx51 I guess you still need to add the define to rx51.h manually.
Originally Posted by mirakels
yes, this is reason why not to upstream this patch. We need look how maemo kernel set brightness!
Yes that is why I added the big WARNING. I noticed the brightness problem too but when I set backlight to 0 just before booting a kernel this problem seems to be gone.
Originally Posted by mirakels
that for command - but this was only for rx51, so command can be deleted!
What files are you missing?
Originally Posted by mirakels
And (4*CONSOLE_SCROLL_SIZE >> 2) is not same as CONSOLE_SCROLL_SIZE? Byteshift (C >> 2) is same as (C/4), so (4*C >> 2) = (4*C/4) = C
The reason that I added it in the n900 specific part is that I'm not sure if the command is useful on other devices. I guess that is the reason you added the regulator patches to the rx51.c file too. If they are generic enough I guess the regulator and backlight code could be moved to power/twl4030.c
or friends.
CONSOLE_SCROLL_SIZE reflects the number of bytes in the scroll area. but memcpyl() moves ints! To reflect the weirdness in this I think you should still use
memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, 4*CONSOLE_SCROLL_SIZE >> 2);
Originally Posted by mirakels
Sorry, but my bootmenu will not be upstreamed. In upstream will be other implementation of bootmenu (like config for pxecfg) by someone else.
PS: maybe you should consider pushing the ANSI and bootmenu patches to denx asap. They are quite useful for everybody right now!
Originally Posted by mirakels
this could be tested on more HW revisions first! It working on my 2101. Do you have other HW rev?
And great you figured about the regulator stuff to enable the mmc cards!
|
|
2011-10-09
, 12:44
|
|
Posts: 362 |
Thanked: 426 times |
Joined on Nov 2010
@ Italy, Lombardia
|
#118
|
Originally Posted by pali
It's not the same.
And (4*CONSOLE_SCROLL_SIZE >> 2) is not same as CONSOLE_SCROLL_SIZE? Byteshift (C >> 2) is same as (C/4), so (4*C >> 2) = (4*C/4) = C
(4*C>>2) = ((C<<2)>>2) = clear two most significative bits
For example suppose C as byte (unsigned):
If C = 46 (0010 1110) then (4*C>>2) = 46 (0010 1110) also, but if C = 114 (0111 0010) then (4*C>>2) = 50 (0011 0010)
|
|
2011-10-09
, 13:06
|
|
Posts: 197 |
Thanked: 101 times |
Joined on Dec 2009
@ Netherlands
|
#119
|
Originally Posted by pali
brightness seems to be on higher levels. in de nokia patches. I had a quick look but do not see any direvt twl or i2c calls doing brightess.
yes, this is reason why not to upstream this patch. We need look how maemo kernel set brightness!
I took the backlight code right from the kernel source however.
Originally Posted by pali
Yes it will be the same and probably will be compiled away.
And (4*CONSOLE_SCROLL_SIZE >> 2) is not same as CONSOLE_SCROLL_SIZE? Byteshift (C >> 2) is same as (C/4), so (4*C >> 2) = (4*C/4) = C
But in this form it will show the weirdness of needing to scroll way beyond
the actual framebuffer.
Originally Posted by pali
this could be tested on more HW revisions first! It working on my 2101. Do you have other HW rev?
I have 2204. My previous on was a 2104 I think (not sure though) and u-boot worked fine with that model too.
|
|
2011-10-09
, 13:12
|
|
Posts: 2,154 |
Thanked: 8,464 times |
Joined on May 2010
|
#120
|
Originally Posted by Fabry
No, It is same because first is multiply and then is byteshift. Try this:
It's not the same.
(4*C>>2) = ((C<<2)>>2) = clear two most significative bits
For example suppose C as byte (unsigned):
If C = 46 (0010 1110) then (4*C>>2) = 46 (0010 1110) also, but if C = 114 (0111 0010) then (4*C>>2) = 50 (0011 0010)
Code:
printf("%#x %#x\n", 4*114 >> 2, 114);
printf("%#x %#x\n", 4*46 >> 2, 46);
Code:
0x72 0x72 0x2e 0x2e
Furthermore the clearscreen stuff and some cosmetic changes/fixes.