Re: [PATCH] eeepc-laptop: enable camera by default

[Alan Jenkins <sourcejedi.lkml@googlemail.com>]

On 6/6/09, Corentin Chary <corentin.chary@gmail.com> wrote:
> On Fri, Jun 5, 2009 at 11:01 PM, Woody Suwalski<woodys@xandros.com> wrote:
>> Corentin Chary wrote:
>>>
>>> On Fri, Jun 5, 2009 at 10:59 AM, Pekka J Enberg<penberg@cs.helsinki.fi>
>>> wrote:
>>>
>>>>
>>>> From: Pekka Enberg <penberg@cs.helsinki.fi>
>>>>
>>>> If we leave the camera disabled by default, userspace programs (e.g.
>>>> Skype, Cheese) leave the user out in the cold saying that the machine
>>>> "has
>>>> no camera." Therefore, it's better to enable camera by default and let
>>>> people who really don't want it just disable the thing.
>>>>
>>>> Cc: <acpi4asus-user@lists.sourceforge.net>
>>>> Cc: Len Brown <len.brown@intel.com>
>>>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>>>> ---
>>>>  drivers/platform/x86/eeepc-laptop.c |   16 ++++++++++++++++
>>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/eeepc-laptop.c
>>>> b/drivers/platform/x86/eeepc-laptop.c
>>>> index 353a898..c0f8eb1 100644
>>>> --- a/drivers/platform/x86/eeepc-laptop.c
>>>> +++ b/drivers/platform/x86/eeepc-laptop.c
>>>> @@ -334,6 +334,19 @@ static int eeepc_bluetooth_rfkill_state(void *data,
>>>> enum rfkill_state *state)
>>>>  }
>>>>
>>>>  /*
>>>> + * Camera helpers
>>>> + */
>>>> +
>>>> +static void eeepc_enable_camera(void)
>>>> +{
>>>> +       int err;
>>>> +
>>>> +       err = set_acpi(CM_ASL_CAMERA, 1);
>>>> +       if (err)
>>>> +               printk(EEEPC_WARNING "Unable to enable camera %d\n",
>>>> err);
>>>> +}
>>>> +
>>>> +/*
>>>>  * Sys helpers
>>>>  */
>>>>  static int parse_arg(const char *buf, unsigned long count, int *val)
>>>> @@ -996,6 +1009,9 @@ static int __init eeepc_laptop_init(void)
>>>>       result = eeepc_hwmon_init(dev);
>>>>       if (result)
>>>>               goto fail_hwmon;
>>>> +
>>>> +       eeepc_enable_camera();
>>>> +
>>>>       /* Register platform stuff */
>>>>       result = platform_driver_register(&platform_driver);
>>>>       if (result)
>>>> --
>>>> 1.5.6.3
>>>>
>>>>
>>>>
>>>
>>> Hi,
>>> First, I think a one line
>>> set_acpi(CM_ASL_CAMERA, 1);
>>> with a small comment is better. (If it fails, it's because there no
>>> camera, so we don't mind).
>>>
>>> Anyway, I'm not sure it's a good idea to enable the camera by default.
>>> It depends how many
>>> power the camera is using (when idle). Coud you check that (with power
>>> top maybe ?).
>>>
>>> Thanks
>>>
>>
>> From our old measurements:
>>
>> idle camera is ~20mA out of ~650mA total power... (measured on 901 model).
>>
>> Woody
>>
>> --
>> Woody Suwalski, Xandros, Ottawa, Canada, 1-613-842-3498 x414
>>
>>
>
> Do also have some results with
> echo -n auto > /sys/bus/usb/drivers/uvcvideo/*:*/../power/level
>
> ?

Can I also ask if you tested with

CONFIG_NO_HZ=y

?  IIRC, the kernel shipped on my old Eee didn't have NOHZ.  Without
NOHZ, it won't save much _cpu_ power; cutting 10 wakeups/s off >100
won't make much difference.

Ta
Alan