Hi Thomas,
Am 07.11.2012 22:23, schrieb Thomas Martitz:
> Am 07.11.2012 10:11, schrieb Stefan Seyfried:
>> Hi all,
>>
>> I'm new to rockbox development, so bear with me if I'm violating
>> the mailing list etiquette etc.
>>
>> On my sansa clip+, there is a quiet, ticking noise on weak fm radio
>> channels.
>> I found out that this is probably due to the cpu being woken up to
>> update the display 5 times a second, even if the display is turned off.
>>
>> This patch fixes the issue by not updating the display unless it is
>> turned on.
>>
>> diff --git a/apps/radio/radio_skin.c b/apps/radio/radio_skin.c
>> index 521890c..0c05f33 100644
>> --- a/apps/radio/radio_skin.c
>> +++ b/apps/radio/radio_skin.c
>> @@ -96,7 +96,13 @@ void fms_fix_displays(enum fms_exiting toggle_state)
>> int fms_do_button_loop(bool update_screen)
>> {
>> int button = skin_wait_for_action(FM_SCREEN, CONTEXT_FM,
>> - update_screen ? TIMEOUT_NOBLOCK
>> : HZ/5);
>> + update_screen ? TIMEOUT_NOBLOCK :
>> +#if defined(HAVE_LCD_ENABLE) || defined(HAVE_LCD_SLEEP)
>> + (lcd_active() ? HZ/5 :
>> TIMEOUT_BLOCK)
>> +#else
>> + HZ/5
>> +#endif
>> + );
>> #ifdef HAVE_TOUCHSCREEN
>> struct touchregion *region;
>> int offset;
>>
>> Unfortunately, if combined with the "backlight filters first
>> keypress: on" config option, this causes the display (including
>> RSSI) to also not be updated after it is woken up again, simply
>> because skin_wait_for_action() will not return if the keypress
>> was only turning on the backlight.
>> The following patch pushes a dummy event into the button queue
>> to work around that.
>
> I absolutely expected this to happen. Have a look at how the WPS handles
> this very issue which is solved there. See wps_lcd_activation_hook() and
> related, it's actually similar to your solution.
Not really. I'm setting the skin_wait_for_action() to TIMEOUT_BLOCK, the
WPS doesn't do this.
The problem is, that skin_wait_for_action(TIMEOUT_BLOCK) does *not*
return for "display wakeup only" events, because no button event is
generated. This is why I inject a BUTTON_NONE to make it actually return.
At least that's how I understand the issue -- as I wrote, I'm very new
to the rockbox code base and it would be a very bold statement if I said
that I understand 1% of the code :-)
> This fixes too frequent updates which are completely unnecessary and as
> such it is welcome. However, I'm not positive that it's the right fix
> for the radio noise (which still happens with display on, right?).
Yes, with display on, it will wake up the CPU 5 times a second and that
will produce the noise. I doubt that there is much we can do about that.
The original sandisk software does not have that problem, but then it
does not display anything dynamic in the radio screen (no RSSI, no
stereo indicator, nothing).
At first I had suspected that it were actually the i2c transfers for
reading signal strength etc that were causing the interference, but I
removed them all, and the noise was still there. So now I assume that it
is plain CPU activity that interferes with the FM radio receiver.
Maybe it could be made dynamic -- if the skin has no dynamic elements
(no RSSI, no stereo indicator), then even TIMEOUT_BLOCK when the display
is on. However, that's beyond my current capabilities to implement.
And actually I like the RSSI and stereo indicators :-) Better radio
functionality is what made me actually start using rockbox.
Best regards,
Stefan
--
Stefan Seyfried
Linux Consultant & Developer -- GPG Key: 0x731B665B
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537
