Re: [PATCH v2 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-07-01 Thread Kirill Smelkov
On Thu, Jun 30, 2011 at 11:01:01AM -0700, Sarah Sharp wrote:
> On Fri, Jun 24, 2011 at 08:48:06PM +0400, Kirill Smelkov wrote:
> > 
> > Changes since v1:
> > 
> > 
> >  - dropped RFC status as "this seems like the sort of feature somebody might
> >reasonably want to use -- if they know exactly what they're doing";
> > 
> >  - new preparatory patch (1/2) which moves already-in-there sysfs code into
> >ehci-sysfs.c;
> > 
> >  - moved uframe_periodic_max parameter from module option to sysfs 
> > attribute,
> >so that it can be set per controller and at runtime, added validity 
> > checks;
> > 
> >  - clarified a bit bandwith analysis for 96% max periodic setup as noticed 
> > by
> >Alan Stern;
> > 
> >  - clarified patch description saying that set in stone 80% max periodic is
> >specified by USB 2.0;
> 
> Have you tested this patch by maxing out this bandwidth on various
> types of host controllers?  It's entirely possible that you'll run into
> vendor-specific bugs if you try to pack the schedule with isochronous
> transfers.  I don't think any hardware designer would seriously test or
> validate their hardware with a schedule that is basically a violation of
> the USB bus spec (more than 80% for periodic transfers).

I've only tested it to work on my HP Mini 5103 with N10 chipset:

kirr@mini:~$ lspci 
00:00.0 Host bridge: Intel Corporation N10 Family DMI Bridge
00:02.0 VGA compatible controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:02.1 Display controller: Intel Corporation N10 Family Integrated 
Graphics Controller
00:1b.0 Audio device: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller (rev 02)
00:1c.0 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 1 
(rev 02)
00:1c.3 PCI bridge: Intel Corporation N10/ICH 7 Family PCI Express Port 4 
(rev 02)
00:1d.0 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 (rev 02)
00:1d.1 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 (rev 02)
00:1d.2 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 (rev 02)
00:1d.3 USB Controller: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 (rev 02)
00:1d.7 USB Controller: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2)
00:1f.0 ISA bridge: Intel Corporation NM10 Family LPC Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation N10/ICH7 Family SATA AHCI 
Controller (rev 02)
01:00.0 Network controller: Broadcom Corporation BCM4313 802.11b/g/n 
Wireless LAN Controller (rev 01)
02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8059 PCI-E 
Gigabit Ethernet Controller (rev 11)

The system works stable with 110us/uframe (~88%) isoc bandwith allocated for
integrated UVC webcam and external EM28XX based capture board.


> But if Alan is fine with giving users a way to shoot themselves in the
> foot, and it's disabled by default, then I don't particularly mind this
> patch.

Yes, it is disabled by default, I mean max periodic bandwidth is set to
100us/uframe by default exactly as it was before the patch. So only
those of us who need the extreme settings are taking the risk - normal
users who do not alter uframe_periodic_max attribute should not see any
change at all.


Thanks for commenting. I'll extend my testing information and notes on
do-not-do-harm bahaviour in updated patch.


Kirill
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-30 Thread Sarah Sharp
On Fri, Jun 24, 2011 at 08:48:06PM +0400, Kirill Smelkov wrote:
> 
> Changes since v1:
> 
> 
>  - dropped RFC status as "this seems like the sort of feature somebody might
>reasonably want to use -- if they know exactly what they're doing";
> 
>  - new preparatory patch (1/2) which moves already-in-there sysfs code into
>ehci-sysfs.c;
> 
>  - moved uframe_periodic_max parameter from module option to sysfs attribute,
>so that it can be set per controller and at runtime, added validity checks;
> 
>  - clarified a bit bandwith analysis for 96% max periodic setup as noticed by
>Alan Stern;
> 
>  - clarified patch description saying that set in stone 80% max periodic is
>specified by USB 2.0;

Have you tested this patch by maxing out this bandwidth on various
types of host controllers?  It's entirely possible that you'll run into
vendor-specific bugs if you try to pack the schedule with isochronous
transfers.  I don't think any hardware designer would seriously test or
validate their hardware with a schedule that is basically a violation of
the USB bus spec (more than 80% for periodic transfers).

But if Alan is fine with giving users a way to shoot themselves in the
foot, and it's disabled by default, then I don't particularly mind this
patch.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth

2011-06-24 Thread Kirill Smelkov

Changes since v1:


 - dropped RFC status as "this seems like the sort of feature somebody might
   reasonably want to use -- if they know exactly what they're doing";

 - new preparatory patch (1/2) which moves already-in-there sysfs code into
   ehci-sysfs.c;

 - moved uframe_periodic_max parameter from module option to sysfs attribute,
   so that it can be set per controller and at runtime, added validity checks;

 - clarified a bit bandwith analysis for 96% max periodic setup as noticed by
   Alan Stern;

 - clarified patch description saying that set in stone 80% max periodic is
   specified by USB 2.0;

Kirill Smelkov (2):
  USB: EHCI: Move sysfs related bits into ehci-sysfs.c
  USB: EHCI: Allow users to override 80% max periodic bandwidth

 drivers/usb/host/ehci-hcd.c   |   11 ++-
 drivers/usb/host/ehci-hub.c   |   75 -
 drivers/usb/host/ehci-sched.c |   17 ++--
 drivers/usb/host/ehci-sysfs.c |  184 +
 drivers/usb/host/ehci.h   |2 +
 5 files changed, 202 insertions(+), 87 deletions(-)
 create mode 100644 drivers/usb/host/ehci-sysfs.c

-- 
1.7.6.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html