Rockbox

Tasklist

FS#12348 - [New Port] Samsung YP-R0: the first patches

Attached to Project: Rockbox
Opened by Lorenzo Miori (Lorenzo92) - Tuesday, 25 October 2011, 08:35 GMT
Task Type Patches
Category Configuration
Status Unconfirmed
Assigned To No-one
Operating System Another
Severity Low
Priority Normal
Reported Version Release 3.9
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Hi all! As requested, I begin to attach there the patches to make RockBox running, as an application, to the Samsung YP-R0.
Maybe the things to check are:
- is the code clean? Can I continue in this way?
- Are the others platforms compiling as usual?

Then I will have the need for some hints :D

ps: for some reasons, I had to create 2 diff files. One for Rb in general, the other just for /target/hosted/ypr0...After that I will redownload entire svn appliying the patches, I hope this will solve!
This task depends upon

Comment by Lorenzo Miori (Lorenzo92) - Tuesday, 25 October 2011, 10:44 GMT
Okay. The previous patches aren't working correctly !!!

Please use this ;)
Comment by Thomas Martitz (kugel.) - Tuesday, 25 October 2011, 10:58 GMT
This port seems to be a bit special, e.g. with backlight routines etc. Can you make a wiki page describing this port?

I'm unhappy with the amount of copied code. Can't you use the existing files directly, like the paemo port does?
Comment by Lorenzo Miori (Lorenzo92) - Tuesday, 25 October 2011, 15:31 GMT
For button-sdl.c would mean have a mess...Really, I'm not jocking. There is so much stuff that doesn't fit with our device that's quite useless having all in one file.
For lcd stuff, yes it's just a copy, as for kernel. With this you're right. I could simply eliminate them and "restore" original files.
With system-sdl I don't know. I guess it's more or less like buttons: we need code to manage cpu frequency, and for me that strictly a platform-defined thing, not to be put in a general one.

For the other things I will hear mgue, too. Maybe he has ideas hehe
Thanks for reviewing...
Comment by MichaelGiacomelli (saratoga) - Tuesday, 25 October 2011, 15:35 GMT
^^
Maemo port I think.

Minor thing:

keymap-ypr0.c: 2010 Maurus Cuelenaere > 2011 Lorenzo Miori
Comment by Thomas Martitz (kugel.) - Tuesday, 25 October 2011, 16:24 GMT
Speaking of frequency scaling. Why do you want to do that in Rockbox? That's should be handled by the kernel unless it's broken (using ondemand governor or whatever).
Comment by Lorenzo Miori (Lorenzo92) - Tuesday, 25 October 2011, 17:50 GMT
Uhm well...I tought...since there is such a feature in RockBox, why not implementing it? :)
@saratoga: yes I need to fix such things :D thanks (fixed)

And moreover I fixed all the ascodec read/write functions.
Now I need some ideas on how implement the audio control feature (volume, balance) without interfering with audio SDL<->alsa part. As I said as3543 codec functions are ready.
SDL audio must be 100% always; audio volume is controlled in the codec, audio streaming in alsa.
Comment by Thomas Martitz (kugel.) - Wednesday, 26 October 2011, 07:02 GMT
Frequency scaling should be left to the OS. It doesn't belong into an app build.

What are the reasons again for implementing backight and volume control via alsa (sorry if you already told me, but I forgot).
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 26 October 2011, 08:48 GMT
Okay. Probably frequency scaling should be left to the kernel (maybe it's even "better", because actually rb stays to 400 mhz even if it's not necessary...)

But indeed backlight isn't managed at all by the kernel (I remember you that we are on a player, no "real" OS functions, just the linux kernel inside.). So we need to manage it throug as3543.
For audio, we are a player. I cannot keep the alsa volume at 100% (hey the battery will die :D) and the use the SW volume mixer of SDL (it sucks, really...). We need quality.
Alsa driver by Samsung sucks too (no balancing, no nothing) so we need to control directly as3543 for volumes, not through alsa mixer.

Moreover: time isn't managed by the os. We have rtc device (but doing hwclock -h doesn't work, samsung app stores a time related value in /dev/stl1 1 mb nand raw partition) but still it would be better to manage it directly (to have wake up on alarm too maybe).
Headphone detection, still no clue how to implement it but it's an irq.
Battery voltages, no device for them (well there is /dev/r0Bat but I don't understand how to make it work lol). Again, as3543 rulez.
Then radio. There is a Samsung module. I need to understan ioctls codes. POssibly there is a freescale module to test, maybe it's compatbile.

We exploit RockBox as an application just for video and audio data streaming (and of course other things). But much is to be implemented ;)
Comment by Thomas Martitz (kugel.) - Wednesday, 26 October 2011, 09:23 GMT
Why do you find the SW volume mixer of SDL sucks? I never noticed it's bad quality. And SW volume is really trivial, and I always thought it's a lossless operation.

I, though, can understand the desire to exploit more HW features. I think we even support the as3543 natively on some players?
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 26 October 2011, 10:56 GMT
Yes, indeed. AS3543 driver is already present. I guess it will work with only some minor modifications ;)

SDL volume sucks, yes, it's not lossless at all. Having the HW @ max you can hear this when the song is finishing (fading out)...it's like it considers on the peaks. Difficult to explain.
And then battery consumption lol. It's consuming like a truck in this way :D

And for HW things, yes, I want and we must do it. Going under battery voltage isn't so fine (and having a battery indicator is something that's necessary on a player hehe)
Comment by Thomas Martitz (kugel.) - Wednesday, 26 October 2011, 11:06 GMT
The kernel should also handle the battery voltage, no?
Are you sure the battery consumption is much higher at higher volumes? In my experience with other Rockbox daps the volume has little to no influence on the battery.
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 26 October 2011, 17:02 GMT
No, unfortunately not. Samsung is creative hihi He has not adapted the kernel, just for cpu and other important things but nothing more. Indeed it's not supposed to be open to be used by everyone.
Morover samsung didn't include the source of their modules :(

Well for the audio consume related..uhm...uhm...I always tought so, well even in the datasheet there is a picture saying 8 mW @ 102 dB - 4 mW @ 94....Bah I will see. Now I'll implement it to test. Of course I won't send another patch till we decide how to procede!

Thanks
Comment by MichaelGiacomelli (saratoga) - Wednesday, 26 October 2011, 17:11 GMT
So in the stock firmware, the actual music player app handles volume, backlight, etc?
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 26 October 2011, 17:32 GMT
Yep. It makes everything hehe
Comment by Lorenzo Miori (Lorenzo92) - Monday, 31 October 2011, 12:06 GMT
In some days, I will post a new patch.
It will fix/contain at least:

- performance optimized: SDL video part removed, used framebuffer directly instead. This mean rb will run fine on the OF kernel (solves lots of issues).
- ascodec register management it's fixed: next step is to integrate the already existing driver and make it work.
- keymaps heavily improved
- backlight improved, lcd goes to sleep to save power
Comment by Ophir Lojkine (lovasoa) - Saturday, 12 November 2011, 12:45 GMT
I'm very excited about rockbox being ported to YP-R0.
Lorenzo92: What about your patch? Is it simple to help testing your patches (without risks to brick the player) ?
Comment by Lorenzo Miori (Lorenzo92) - Saturday, 12 November 2011, 23:05 GMT
Hi! Okay nice to see some interested people....Yes it is safe to use my modded firmware. It has a safe mode, so once is correctly flashed you won't have any problem of any sort.
BUT I need to upload an updated and cleaned patch. Tomorrow or so I hope to do that, also with instructions ;)
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 16 November 2011, 18:29 GMT
here the patch!
you need to read on ABI forums for successfully compilation...
;)
   YPR0_8.diff (170.1 KiB)
Comment by MichaelGiacomelli (saratoga) - Wednesday, 16 November 2011, 19:01 GMT
Not really that important, but I noticed you changed the AS3543 volume calculations for this port. Is that really needed? Keeping the mixer volume constant will limit the volume range somewhat.
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 16 November 2011, 19:25 GMT
thanks for interesting :)
it was necessary to have a decent volume without too many "low" volume levels...But since it doesn't mute completely when 0 volume, it needs some fine tuning!
A note is that samsung does the same, keeps constant DAC mixer and changes volume only. Indeed, needs testing + discussing.
Comment by MichaelGiacomelli (saratoga) - Wednesday, 16 November 2011, 21:10 GMT
Unless theres a reason you need to do otherwise, you should just use the same volume range as the other AS3543 devices. Introducing an artificial lower limit on volume isn't needed IMO.
Comment by Thomas Martitz (kugel.) - Friday, 02 December 2011, 08:22 GMT
Lorenzo, can you please update the wiki for the installation procedure?
Comment by Thomas Martitz (kugel.) - Saturday, 03 December 2011, 15:59 GMT
Here's my patch. I changed path handling and configure a bit, also got rid of libuisimulator and implemented kernel tick and file I/O natively (similar to android).

Configure is changed so that it expects the toolchain in PATH, and PATHTOSDL must be set when running configure.

Note: I changed the directory to /mnt/media0/.rockbox (with dot), similar to native targets. Also the changed path handling means that gigabeat f/x/s themes from the theme site work ("/.rockbox" in the cfg is a magic for the real directory).

My alsa work is not in it, still working on it.
   YPR0_9.diff (143.8 KiB)
Comment by Thomas Martitz (kugel.) - Tuesday, 06 December 2011, 16:20 GMT
New version with alsa sound
Comment by Thomas Martitz (kugel.) - Wednesday, 07 December 2011, 15:08 GMT
Lots of cleanup, e.g. in the lcd and button drivers. Remove PLATFORM_YPR0 define. This is one particular device, not sure if it deserves a PLATFORM define (but I'm pondering a PLATFORM_LINUX one instead).

It also adds a nice CPU debug screen in System->Debug->View CPU stats.

I'd say this is nearly ready for SVN. I'm going to look at the keymaps before committing. After that, we need to figure out a proper installation routine and what to do about USB (switch/reboot into safe mode or implement in rockbox?)
Comment by Thomas Martitz (kugel.) - Tuesday, 13 December 2011, 22:31 GMT
Update:

New toolchain: http://www.alice-dsl.net/simonemartitz/rockbox/arm-ypr0-linux-gnueabi.zip (for 64bit linux)
Bump alsa buffer for less cpu usage and cpu freq stats (thanks to lorenzo)
Significantly changed keymaps.
Fixed LCD
Comment by Thomas Martitz (kugel.) - Tuesday, 20 December 2011, 08:25 GMT
Sync to SVN.

The patch looks fine to me. Hopefully we can get it in soon.
Comment by Thomas Martitz (kugel.) - Friday, 23 December 2011, 18:57 GMT
This patch adds the infrastructure to build a patched firmware which can load rockbox. The shell sources .rockbox/rockbox.sh which sets stuff up so rockbox can be loaded.
Using the 3 scripts, {unpack,patch,pack}-firmware.sh, you can build such a patched firmware.

FFS: I'm unable to add MuonEncrypt. I tried lots of different filenames, even names of files that I can upload. Get it from http://www.alice-dsl.net/simonemartitz/rockbox/MuonEncrypt

CAUTION: Place the binary files (both!) where they belong (see the diff stat output below) to before making any attempt to patch a firmware. I could image that the usb safe mode is broken without safe_mode.raw.
Comment by Lorenzo Miori (Lorenzo92) - Tuesday, 03 January 2012, 13:06 GMT
Here another patch for R0.

This adds GPIO management possibility + the headphone sense!
So it adds to rockbox another interesting feature....
Comment by Lorenzo Miori (Lorenzo92) - Sunday, 08 January 2012, 19:34 GMT
This patch adds keypad reading using gpio pins. This is better to do since that way we can have multiple keypresses, while Samsung's r0Btn driver does not handle them.

+ it has a GPIO list ("meaningful pins") updated and reviewed ;)
Comment by Lorenzo Miori (Lorenzo92) - Saturday, 18 February 2012, 10:51 GMT
Hey All!

In attachment you can find the patch to enable (finally) the FM radio also on our YP-R0! ;)
At the moment RDS is not implemented, but I will find out how to enable it indeed.
"Basic" radio usage works perfectly, scan, tuning, etc everything the si4709 offers. Audio path switching works. It happens something that recovering audio playback from radio doesn't work anymore until a reboot: it is possibly related to ALSA crash, can we disable alsa when stopping a song?

To achieve the goal, I needed to write a new kernel module. In attachment the sources of it (need to see how to insert it in rockbox trunk...) and the binary version to do a quick test.

NOTE: to load the module, just
rmmod si470x.ko
insmod /mnt/media0/si4709.ko

Enjoy
Comment by Lorenzo Miori (Lorenzo92) - Tuesday, 21 February 2012, 23:01 GMT
Ops! I forgot a file in the previous patch! Put it in the target/hosted/ypr0 folder.

Sorry :)
Comment by Thomas Martitz (kugel.) - Saturday, 25 February 2012, 08:27 GMT
Great work!

About the integration into the rockbox tree: I suggest putting the files required for build (i.e. the .c and .h plus necessary kernel headers) into utils/ypr0tools/su4700-module. Then compile it and put it into the rootfs of the patched firmware. I assume we don't need to change it anymore even for adding rds support?
Comment by Lorenzo Miori (Lorenzo92) - Saturday, 25 February 2012, 08:48 GMT
Well, instead of putting the module directly into the firmware, we could put it into .rockbox build...
But indeed to add RDS we don't need other features, maybe I need to delete the things related to the IRQ catcher. We will do this into rockbox in a thread (I've tried to copy the code from the fmradio-as3514 implementation, but the thread doesn't want to work, it crashes...not kind of thing I can manage :D)

So ok it should be fine having it into the firmware too ;)
By the way, don't forget to add the module loading lines to the startup script!
Comment by Thomas Martitz (kugel.) - Wednesday, 14 March 2012, 13:33 GMT
How do you compile the module?

I looked at the code. I have some comments, but I would rather do that review on gerrit because it's a PITA here.
Comment by Lorenzo Miori (Lorenzo92) - Wednesday, 14 March 2012, 15:04 GMT
Hey! Thanks for giving an eye ;)

First of all, I compiled my module inserting it directly into the kernel provided with Samsung, but I'm pretty sure we can compile it with our toolchain. For example samsung did so in his "modules" (drivers) package into the opensource package. Still need to see how, but it's possible.
So the idea is to make it building when patching the firmware, am I correct? And then placing it to the modules folder, loading it by rockbox script blablabla

For the git question, yeah indeed, I tried to do that but I had problems while committing. I will retry asap, but I'm a little busy these days at university and more. So if you want, you could upload commit to git if you're willing. I won't complain if my name is not in the patch tracking :p

Loading...