From: Adam Lee <[email protected]>
This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7.
1, I do have a Chicony webcam, implements autosuspend in a broken way,
make `poweroff` performs rebooting when its autosuspend enabled.
2, There are other webcams which don't support autosuspend too, like
https://patchwork.kernel.org/patch/2356141/
3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in
a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is
disabled by default.
So, we need to disable autosuspend in uvcvideo, maintaining a quirk list
only for uvcvideo is not a good idea.
Signed-off-by: Adam Lee <[email protected]>
---
drivers/media/usb/uvc/uvc_driver.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 5dbefa6..8556f7c 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1914,7 +1914,6 @@ static int uvc_probe(struct usb_interface *intf,
}
uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
- usb_enable_autosuspend(udev);
return 0;
error:
--
1.7.10.4
Hi Adam,
Thanks for the patch.
On Wednesday 24 April 2013 15:57:19 [email protected] wrote:
> From: Adam Lee <[email protected]>
>
> This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7.
>
> 1, I do have a Chicony webcam, implements autosuspend in a broken way,
> make `poweroff` performs rebooting when its autosuspend enabled.
>
> 2, There are other webcams which don't support autosuspend too, like
> https://patchwork.kernel.org/patch/2356141/
>
> 3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in
> a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is
> disabled by default.
>
> So, we need to disable autosuspend in uvcvideo, maintaining a quirk list
> only for uvcvideo is not a good idea.
>
> Signed-off-by: Adam Lee <[email protected]>
I've received very few bug reports about broken auto-suspend support in UVC
devices. Most of them could be solved by setting the RESET_RESUME quirk in USB
core, only the Creative Live! Cam Optia AF required a quirk in the uvcvideo
driver. I would thus rather use the available quirks (USB_QUIRK_RESET_RESUME
if possible, UVC_QUIRK_DISABLE_AUTOSUSPEND otherwise) than killing power
management for the vast majority of webcams that behave correctly.
> ---
> drivers/media/usb/uvc/uvc_driver.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 5dbefa6..8556f7c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1914,7 +1914,6 @@ static int uvc_probe(struct usb_interface *intf,
> }
>
> uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
> - usb_enable_autosuspend(udev);
> return 0;
>
> error:
--
Regards,
Laurent Pinchart
On Wed, Apr 24, 2013 at 11:17:52AM +0200, Laurent Pinchart wrote:
> Hi Adam,
>
> Thanks for the patch.
>
> On Wednesday 24 April 2013 15:57:19 [email protected] wrote:
> > From: Adam Lee <[email protected]>
> >
> > This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7.
> >
> > 1, I do have a Chicony webcam, implements autosuspend in a broken way,
> > make `poweroff` performs rebooting when its autosuspend enabled.
> >
> > 2, There are other webcams which don't support autosuspend too, like
> > https://patchwork.kernel.org/patch/2356141/
> >
> > 3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in
> > a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is
> > disabled by default.
> >
> > So, we need to disable autosuspend in uvcvideo, maintaining a quirk list
> > only for uvcvideo is not a good idea.
>
> I've received very few bug reports about broken auto-suspend support in UVC
> devices. Most of them could be solved by setting the RESET_RESUME quirk in USB
> core, only the Creative Live! Cam Optia AF required a quirk in the uvcvideo
> driver. I would thus rather use the available quirks (USB_QUIRK_RESET_RESUME
> if possible, UVC_QUIRK_DISABLE_AUTOSUSPEND otherwise) than killing power
> management for the vast majority of webcams that behave correctly.
Here comes another one, integrated Chicony webcam 04f2:b39f, its
autosuspend makes `poweroff` performs rebooting at the laptop I'm
working on. I tried USB_QUIRK_RESET_RESUME, not helping.
The quirks list will go longer and longer absolutely, do uvcvideo wanna
maintain that? And why only uvcvideo do this in kernel space which
against general USB module?
I still suggest we disable it by default, people can enable it in udev
just like almost all distroes do for udisks. Please consider about it.
--
Regards,
Adam Lee
Hardware Enablement
On Thu, Apr 25, 2013 at 02:33:06PM +0800, Adam Lee wrote:
> On Wed, Apr 24, 2013 at 11:17:52AM +0200, Laurent Pinchart wrote:
> > Hi Adam,
> >
> > Thanks for the patch.
> >
> > On Wednesday 24 April 2013 15:57:19 [email protected] wrote:
> > > From: Adam Lee <[email protected]>
> > >
> > > This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7.
> > >
> > > 1, I do have a Chicony webcam, implements autosuspend in a broken way,
> > > make `poweroff` performs rebooting when its autosuspend enabled.
> > >
> > > 2, There are other webcams which don't support autosuspend too, like
> > > https://patchwork.kernel.org/patch/2356141/
> > >
> > > 3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in
> > > a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is
> > > disabled by default.
> > >
> > > So, we need to disable autosuspend in uvcvideo, maintaining a quirk list
> > > only for uvcvideo is not a good idea.
> >
> > I've received very few bug reports about broken auto-suspend support in UVC
> > devices. Most of them could be solved by setting the RESET_RESUME quirk in USB
> > core, only the Creative Live! Cam Optia AF required a quirk in the uvcvideo
> > driver. I would thus rather use the available quirks (USB_QUIRK_RESET_RESUME
> > if possible, UVC_QUIRK_DISABLE_AUTOSUSPEND otherwise) than killing power
> > management for the vast majority of webcams that behave correctly.
>
> Here comes another one, integrated Chicony webcam 04f2:b39f, its
> autosuspend makes `poweroff` performs rebooting at the laptop I'm
> working on. I tried USB_QUIRK_RESET_RESUME, not helping.
>
> The quirks list will go longer and longer absolutely, do uvcvideo wanna
> maintain that? And why only uvcvideo do this in kernel space which
> against general USB module?
>
> I still suggest we disable it by default, people can enable it in udev
> just like almost all distroes do for udisks. Please consider about it.
Hi, Laurent
Any comment of this? I still think it's a risk to enable autosuspend in
uvcvideo by default, there must be users meeting weird issues but didn't
report to you becaue they didn't figured out the cause.
--
Regards,
Adam Lee
Hardware Enablement
Hi Adam,
On Thursday 13 June 2013 17:56:47 Adam Lee wrote:
> On Thu, Apr 25, 2013 at 02:33:06PM +0800, Adam Lee wrote:
> > On Wed, Apr 24, 2013 at 11:17:52AM +0200, Laurent Pinchart wrote:
> > > On Wednesday 24 April 2013 15:57:19 [email protected] wrote:
> > > > From: Adam Lee <[email protected]>
> > > >
> > > > This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7.
> > > >
> > > > 1, I do have a Chicony webcam, implements autosuspend in a broken way,
> > > > make `poweroff` performs rebooting when its autosuspend enabled.
> > > >
> > > > 2, There are other webcams which don't support autosuspend too, like
> > > > https://patchwork.kernel.org/patch/2356141/
> > > >
> > > > 3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in
> > > > a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is
> > > > disabled by default.
> > > >
> > > > So, we need to disable autosuspend in uvcvideo, maintaining a quirk
> > > > list only for uvcvideo is not a good idea.
> > >
> > > I've received very few bug reports about broken auto-suspend support in
> > > UVC devices. Most of them could be solved by setting the RESET_RESUME
> > > quirk in USB core, only the Creative Live! Cam Optia AF required a quirk
> > > in the uvcvideo driver. I would thus rather use the available quirks
> > > (USB_QUIRK_RESET_RESUME if possible, UVC_QUIRK_DISABLE_AUTOSUSPEND
> > > otherwise) than killing power management for the vast majority of
> > > webcams that behave correctly.
> >
> > Here comes another one, integrated Chicony webcam 04f2:b39f, its
> > autosuspend makes `poweroff` performs rebooting at the laptop I'm working
> > on.
That's a pretty bad one :-/ Do you have any idea why it occurs ?
> > I tried USB_QUIRK_RESET_RESUME, not helping.
> >
> > The quirks list will go longer and longer absolutely, do uvcvideo wanna
> > maintain that? And why only uvcvideo do this in kernel space which
> > against general USB module?
> >
> > I still suggest we disable it by default, people can enable it in udev
> > just like almost all distroes do for udisks. Please consider about it.
>
> Any comment of this? I still think it's a risk to enable autosuspend in
> uvcvideo by default, there must be users meeting weird issues but didn't
> report to you becaue they didn't figured out the cause.
I've discussed this issue during LPC last week, and I still believe we should
enable auto-suspend. The feature really saves power, without it my C910
Logitech webcam gets hot even when unused.
If we disable auto-suspend by default and enable it from userspace only a
handful of devices will get auto-suspended. Unless we can get distros to
automatically test auto-suspend on unknown webcam models and report the
results to update a central data base (which would grow much bigger than a
quirks list in the driver in my opinion), disabling auto-suspend would be a
serious regression.
Any comment from other developers from the mailing lists ?
--
Regards,
Laurent Pinchart
On Tue, Sep 24, 2013 at 4:34 AM, Laurent Pinchart
<[email protected]> wrote:
> I've discussed this issue during LPC last week, and I still believe we should
> enable auto-suspend. The feature really saves power, without it my C910
> Logitech webcam gets hot even when unused.
>
> If we disable auto-suspend by default and enable it from userspace only a
> handful of devices will get auto-suspended. Unless we can get distros to
> automatically test auto-suspend on unknown webcam models and report the
> results to update a central data base (which would grow much bigger than a
> quirks list in the driver in my opinion), disabling auto-suspend would be a
> serious regression.
Setting defaults which knowingly cause problems is a horrible idea.
Just because it works for you and your setup is no justification to
force it upon everyone. This is certainly a feature that, if wanted,
can be enabled by the user. I don't see any reasonable argument
against letting the user enable it if he/she wants it.
Ps. Sorry for the double-post if anyone already received this. The
first posting wasn't using plain text and was therefore rejected so
this corrects that.
Hi,
On Tuesday 24 September 2013 08:55:19 VDR User wrote:
> On Tue, Sep 24, 2013 at 4:34 AM, Laurent Pinchart wrote:
> > I've discussed this issue during LPC last week, and I still believe we
> > should enable auto-suspend. The feature really saves power, without it my
> > C910 Logitech webcam gets hot even when unused.
> >
> > If we disable auto-suspend by default and enable it from userspace only a
> > handful of devices will get auto-suspended. Unless we can get distros to
> > automatically test auto-suspend on unknown webcam models and report the
> > results to update a central data base (which would grow much bigger than a
> > quirks list in the driver in my opinion), disabling auto-suspend would be
> > a serious regression.
>
> Setting defaults which knowingly cause problems is a horrible idea. Just
> because it works for you and your setup is no justification to force it upon
> everyone. This is certainly a feature that, if wanted, can be enabled by the
> user.
It's not just my setup, auto-suspend works for the vast majority of webcams.
It has been enabled three years ago, with a report that Fedora had enabled it
by carrying a kernel patch for a while, without any user complaint.
> I don't see any reasonable argument against letting the user enable it if
> he/she wants it.
USB autosuspend is an important power saving feature. I would be fine with
enabling it in userspace if we could find a reasonable, cross-distro way to
create, maintain and distribute the list of devices that support USB
autosuspend properly.
--
Regards,
Laurent Pinchart