While looking at a dwc2 recently, I noticed that some platforms still
have the uframe scheduler off. As far as I know, nothing good can
come out of having the uframe scheduler off. Let's turn it on
everywhere. Assuming this all works out then some time in the future
we can gut all the old code that handles the old non-uframe scheduler
and make everything simpler.
This is a repost of the original "RFT" (request for testing) series I
posted a few weeks ago but without the "RFT" prefix. Two of the
patches in the series now have "Tested-by:" tags. Since there should
be nothing platform specific about the uFrame scheduler presumably
this should mean we're good to go? Specificaly note that in at least
one of the "Tested-by" reports this patch caused a notable
improvement.
In response to the original RFT, some notes about testing this:
- In general, try to have lots of things plugged in.
- Plug many different keyboards / mice in. See if keys are
dropped / mouse is jerky.
- Try USB audio (ideally full speed, not high speed). How is
the audio?
- If you're brave, try a USB webcam. How is the video?
...maybe try including patches from crbug.com/820961 too.
Douglas Anderson (4):
usb: dwc2: Turn on uframe_sched on "bcm" platforms
usb: dwc2: Turn on uframe_sched on "his" platforms
usb: dwc2: Turn on uframe_sched on "amlogic" platforms
usb: dwc2: Turn on uframe_sched on "stm32f4x9_fsotg" platforms
drivers/usb/dwc2/params.c | 4 ----
1 file changed, 4 deletions(-)
--
2.18.0.345.g5c9ce644c3-goog
There's no reason to have the uframe scheduler off on dwc2. Running
with uframe_sched = False is equivalent to saying "I don't want to run
the correct code, I want to run the old and incorrect code".
The uframe scheduler has been off on stm32f4x9_fsotg since commit
e35b135055e2 ("usb: dwc2: Add support for STM32F429/439/469 USB OTG
HS/FS in FS mode (internal PHY)"). That commit is pretty recent, so
it's unclear to me why the uframe scheduler was left off. Hopefully
it's because someone copied it from other parameters and didn't think
to try it?
Presumably if everyone is good w/ the uframe_sched turned back on we
can kill all the old and crufty non-uframe sched code.
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/usb/dwc2/params.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 93380f7c32b4..7be35bcde713 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -131,7 +131,6 @@ static void dwc2_set_stm32f4x9_fsotg_params(struct dwc2_hsotg *hsotg)
p->max_packet_count = 256;
p->phy_type = DWC2_PHY_TYPE_PARAM_FS;
p->i2c_enable = false;
- p->uframe_sched = false;
p->activate_stm_fs_transceiver = true;
}
--
2.18.0.345.g5c9ce644c3-goog
There's no reason to have the uframe scheduler off on dwc2. Running
with uframe_sched = False is equivalent to saying "I don't want to run
the correct code, I want to run the old and incorrect code".
The uframe scheduler has been off on Amlogic since commit f94310ac076e
("usb: dwc2: add support for Meson8b and GXBB SoCs"). While this was
after most of the recent improvements, notably the commit 9f9f09b048f5
("usb: dwc2: host: Totally redo the microframe scheduler"), presumably
the parameters were copied from another platform and the uframe
scheduler wasn't tried.
Presumably if everyone is good w/ the uframe_sched turned back on we
can kill all the old and crufty non-uframe sched code.
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/usb/dwc2/params.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index db97bb16ecc4..93380f7c32b4 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -110,7 +110,6 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg)
p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI;
p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 <<
GAHBCFG_HBSTLEN_SHIFT;
- p->uframe_sched = false;
}
static void dwc2_set_amcc_params(struct dwc2_hsotg *hsotg)
--
2.18.0.345.g5c9ce644c3-goog
There's no reason to have the uframe scheduler off on dwc2. Running
with uframe_sched = False is equivalent to saying "I don't want to run
the correct code, I want to run the old and incorrect code".
The uframe scheduler has been off on HiSilicon since commit
37dd9d65cc41 ("usb: dwc2: add support of hi6220"). Since then there
have been many many improvements, notably the commit 9f9f09b048f5
("usb: dwc2: host: Totally redo the microframe scheduler")
Presumably if everyone is good w/ the uframe_sched turned back on we
can kill all the old and crufty non-uframe sched code.
Signed-off-by: Douglas Anderson <[email protected]>
Tested-by: John Stultz <[email protected]>
---
drivers/usb/dwc2/params.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 14b8742e62fa..db97bb16ecc4 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -67,7 +67,6 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
p->reload_ctl = false;
p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
GAHBCFG_HBSTLEN_SHIFT;
- p->uframe_sched = false;
p->change_speed_quirk = true;
p->power_down = false;
}
--
2.18.0.345.g5c9ce644c3-goog
There's no reason to have the uframe scheduler off on dwc2. Running
with uframe_sched = False is equivalent to saying "I don't want to run
the correct code, I want to run the old and incorrect code".
The uframe scheduler has been off on Broadcom since commit
58b179dcf28c ("staging: dwc2: disable uframe_sched on the bcm2835").
Since then there have been many many improvements, notably the commit
9f9f09b048f5 ("usb: dwc2: host: Totally redo the microframe
scheduler")
Presumably if everyone is good w/ the uframe_sched turned back on we
can kill all the old and crufty non-uframe sched code.
Signed-off-by: Douglas Anderson <[email protected]>
Tested-by: Stefan Wahren <[email protected]>
---
drivers/usb/dwc2/params.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index af075d4da895..14b8742e62fa 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -47,7 +47,6 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
p->max_transfer_size = 65535;
p->max_packet_count = 511;
p->ahbcfg = 0x10;
- p->uframe_sched = false;
}
static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
--
2.18.0.345.g5c9ce644c3-goog
Douglas Anderson <[email protected]> writes:
> While looking at a dwc2 recently, I noticed that some platforms still
> have the uframe scheduler off. As far as I know, nothing good can
> come out of having the uframe scheduler off. Let's turn it on
> everywhere. Assuming this all works out then some time in the future
> we can gut all the old code that handles the old non-uframe scheduler
> and make everything simpler.
>
> This is a repost of the original "RFT" (request for testing) series I
> posted a few weeks ago but without the "RFT" prefix. Two of the
> patches in the series now have "Tested-by:" tags. Since there should
> be nothing platform specific about the uFrame scheduler presumably
> this should mean we're good to go? Specificaly note that in at least
> one of the "Tested-by" reports this patch caused a notable
> improvement.
>
> In response to the original RFT, some notes about testing this:
> - In general, try to have lots of things plugged in.
> - Plug many different keyboards / mice in. See if keys are
> dropped / mouse is jerky.
> - Try USB audio (ideally full speed, not high speed). How is
> the audio?
> - If you're brave, try a USB webcam. How is the video?
> ...maybe try including patches from crbug.com/820961 too.
Minas??
--
balbi
On 7/27/2018 12:08 AM, Douglas Anderson wrote:
> While looking at a dwc2 recently, I noticed that some platforms still
> have the uframe scheduler off. As far as I know, nothing good can
> come out of having the uframe scheduler off. Let's turn it on
> everywhere. Assuming this all works out then some time in the future
> we can gut all the old code that handles the old non-uframe scheduler
> and make everything simpler.
>
> This is a repost of the original "RFT" (request for testing) series I
> posted a few weeks ago but without the "RFT" prefix. Two of the
> patches in the series now have "Tested-by:" tags. Since there should
> be nothing platform specific about the uFrame scheduler presumably
> this should mean we're good to go? Specificaly note that in at least
> one of the "Tested-by" reports this patch caused a notable
> improvement.
>
> In response to the original RFT, some notes about testing this:
> - In general, try to have lots of things plugged in.
> - Plug many different keyboards / mice in. See if keys are
> dropped / mouse is jerky.
> - Try USB audio (ideally full speed, not high speed). How is
> the audio?
> - If you're brave, try a USB webcam. How is the video?
> ...maybe try including patches from crbug.com/820961 too.
>
>
> Douglas Anderson (4):
> usb: dwc2: Turn on uframe_sched on "bcm" platforms
> usb: dwc2: Turn on uframe_sched on "his" platforms
> usb: dwc2: Turn on uframe_sched on "amlogic" platforms
> usb: dwc2: Turn on uframe_sched on "stm32f4x9_fsotg" platforms
>
> drivers/usb/dwc2/params.c | 4 ----
> 1 file changed, 4 deletions(-)
>
Reviewed-by: Minas Harutyunyan <[email protected]>
On 7/27/2018 12:08 AM, Douglas Anderson wrote:
> There's no reason to have the uframe scheduler off on dwc2. Running
> with uframe_sched = False is equivalent to saying "I don't want to run
> the correct code, I want to run the old and incorrect code".
>
> The uframe scheduler has been off on HiSilicon since commit
> 37dd9d65cc41 ("usb: dwc2: add support of hi6220"). Since then there
> have been many many improvements, notably the commit 9f9f09b048f5
> ("usb: dwc2: host: Totally redo the microframe scheduler")
>
> Presumably if everyone is good w/ the uframe_sched turned back on we
> can kill all the old and crufty non-uframe sched code.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> Tested-by: John Stultz <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
> ---
>
> drivers/usb/dwc2/params.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 14b8742e62fa..db97bb16ecc4 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -67,7 +67,6 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
> p->reload_ctl = false;
> p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
> GAHBCFG_HBSTLEN_SHIFT;
> - p->uframe_sched = false;
> p->change_speed_quirk = true;
> p->power_down = false;
> }
>
On 7/27/2018 12:08 AM, Douglas Anderson wrote:
> There's no reason to have the uframe scheduler off on dwc2. Running
> with uframe_sched = False is equivalent to saying "I don't want to run
> the correct code, I want to run the old and incorrect code".
>
> The uframe scheduler has been off on Broadcom since commit
> 58b179dcf28c ("staging: dwc2: disable uframe_sched on the bcm2835").
> Since then there have been many many improvements, notably the commit
> 9f9f09b048f5 ("usb: dwc2: host: Totally redo the microframe
> scheduler")
>
> Presumably if everyone is good w/ the uframe_sched turned back on we
> can kill all the old and crufty non-uframe sched code.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> Tested-by: Stefan Wahren <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
> ---
>
> drivers/usb/dwc2/params.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index af075d4da895..14b8742e62fa 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -47,7 +47,6 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
> p->max_transfer_size = 65535;
> p->max_packet_count = 511;
> p->ahbcfg = 0x10;
> - p->uframe_sched = false;
> }
>
> static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
>
On 7/27/2018 12:08 AM, Douglas Anderson wrote:
> There's no reason to have the uframe scheduler off on dwc2. Running
> with uframe_sched = False is equivalent to saying "I don't want to run
> the correct code, I want to run the old and incorrect code".
>
> The uframe scheduler has been off on Amlogic since commit f94310ac076e
> ("usb: dwc2: add support for Meson8b and GXBB SoCs"). While this was
> after most of the recent improvements, notably the commit 9f9f09b048f5
> ("usb: dwc2: host: Totally redo the microframe scheduler"), presumably
> the parameters were copied from another platform and the uframe
> scheduler wasn't tried.
>
> Presumably if everyone is good w/ the uframe_sched turned back on we
> can kill all the old and crufty non-uframe sched code.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Minas Harutyunyan <[email protected]>
> ---
>
> drivers/usb/dwc2/params.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index db97bb16ecc4..93380f7c32b4 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -110,7 +110,6 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg)
> p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI;
> p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 <<
> GAHBCFG_HBSTLEN_SHIFT;
> - p->uframe_sched = false;
> }
>
> static void dwc2_set_amcc_params(struct dwc2_hsotg *hsotg)
>
On 7/27/2018 12:08 AM, Douglas Anderson wrote:
> There's no reason to have the uframe scheduler off on dwc2. Running
> with uframe_sched = False is equivalent to saying "I don't want to run
> the correct code, I want to run the old and incorrect code".
>
> The uframe scheduler has been off on stm32f4x9_fsotg since commit
> e35b135055e2 ("usb: dwc2: Add support for STM32F429/439/469 USB OTG
> HS/FS in FS mode (internal PHY)"). That commit is pretty recent, so
> it's unclear to me why the uframe scheduler was left off. Hopefully
> it's because someone copied it from other parameters and didn't think
> to try it?
>
> Presumably if everyone is good w/ the uframe_sched turned back on we
> can kill all the old and crufty non-uframe sched code.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Minas Harutyunyan <[email protected]>
> ---
>
> drivers/usb/dwc2/params.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 93380f7c32b4..7be35bcde713 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -131,7 +131,6 @@ static void dwc2_set_stm32f4x9_fsotg_params(struct dwc2_hsotg *hsotg)
> p->max_packet_count = 256;
> p->phy_type = DWC2_PHY_TYPE_PARAM_FS;
> p->i2c_enable = false;
> - p->uframe_sched = false;
> p->activate_stm_fs_transceiver = true;
> }
>
>