| Message ID | 1627635002-24521-8-git-send-email-chunfeng.yun@mediatek.com |
|---|---|
| State | Accepted |
| Commit | 451d3912586aad3f71f1c97780a1c21e3de98413 |
| Headers | show |
| Series | [01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support' | expand |
Hi Chunfeng, On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > Use @bw_budget_table[] to update fs bus bandwidth due to > not all microframes consume @bw_cost_per_microframe, see > setup_sch_info(). > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/usb/host/xhci-mtk-sch.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c > index 0bb1a6295d64..10c0f0f6461f 100644 > --- a/drivers/usb/host/xhci-mtk-sch.c > +++ b/drivers/usb/host/xhci-mtk-sch.c > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset) > * Compared with hs bus, no matter what ep type, > * the hub will always delay one uframe to send data > */ > - for (j = 0; j < sch_ep->cs_count; j++) { > - tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe; > + for (j = 0; j < sch_ep->num_budget_microframes; j++) { > + tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j]; > if (tmp > FS_PAYLOAD_MAX) > return -ESCH_BW_OVERFLOW; > } > @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used) > { > struct mu3h_sch_tt *tt = sch_ep->sch_tt; > u32 base, num_esit; > - int bw_updated; > int i, j; > > num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit; > > - if (used) > - bw_updated = sch_ep->bw_cost_per_microframe; > - else > - bw_updated = -sch_ep->bw_cost_per_microframe; > - > for (i = 0; i < num_esit; i++) { > base = sch_ep->offset + i * sch_ep->esit; > > - for (j = 0; j < sch_ep->cs_count; j++) > - tt->fs_bus_bw[base + j] += bw_updated; > + for (j = 0; j < sch_ep->num_budget_microframes; j++) > + if (used) > + tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j]; > + else > + tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j]; I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth but I think this approach could trigger a problem. for example, if there are two endpoints scheduled in the same u-frame index, * ep1out = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0 * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... } --> y0 (If this is possible allocation from this patch), I guess xhci-mtk could have some problems on internal scheduling? > } > > if (used) > -- > 2.18.0 >
On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote: > Hi Chunfeng, > > On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > > > Use @bw_budget_table[] to update fs bus bandwidth due to > > not all microframes consume @bw_cost_per_microframe, see > > setup_sch_info(). > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > drivers/usb/host/xhci-mtk-sch.c | 17 +++++++---------- > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c > > index 0bb1a6295d64..10c0f0f6461f 100644 > > --- a/drivers/usb/host/xhci-mtk-sch.c > > +++ b/drivers/usb/host/xhci-mtk-sch.c > > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset) > > * Compared with hs bus, no matter what ep type, > > * the hub will always delay one uframe to send data > > */ > > - for (j = 0; j < sch_ep->cs_count; j++) { > > - tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe; > > + for (j = 0; j < sch_ep->num_budget_microframes; j++) { > > + tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j]; > > if (tmp > FS_PAYLOAD_MAX) > > return -ESCH_BW_OVERFLOW; > > } > > @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used) > > { > > struct mu3h_sch_tt *tt = sch_ep->sch_tt; > > u32 base, num_esit; > > - int bw_updated; > > int i, j; > > > > num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit; > > > > - if (used) > > - bw_updated = sch_ep->bw_cost_per_microframe; > > - else > > - bw_updated = -sch_ep->bw_cost_per_microframe; > > - > > for (i = 0; i < num_esit; i++) { > > base = sch_ep->offset + i * sch_ep->esit; > > > > - for (j = 0; j < sch_ep->cs_count; j++) > > - tt->fs_bus_bw[base + j] += bw_updated; > > + for (j = 0; j < sch_ep->num_budget_microframes; j++) > > + if (used) > > + tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j]; > > + else > > + tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j]; > > I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth > but I think this approach could trigger a problem. See updat_bus_bw(), when add fs ep's bandwidth, it uses bw_budget_table[], so prefer to use the same way > > for example, if there are two endpoints scheduled in the same u-frame index, > * ep1out = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0 > * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... } --> y0 > > (If this is possible allocation from this patch), > I guess xhci-mtk could have some problems on internal scheduling? Test it on dvt env. don't encounter issues; Thanks > > > } > > > > if (used) > > > -- > > 2.18.0 > >
Hi, On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote: > > Hi Chunfeng, > > > > On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > > > > > > Use @bw_budget_table[] to update fs bus bandwidth due to > > > not all microframes consume @bw_cost_per_microframe, see > > > setup_sch_info(). > > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > > --- > > > drivers/usb/host/xhci-mtk-sch.c | 17 +++++++---------- > > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c > > > index 0bb1a6295d64..10c0f0f6461f 100644 > > > --- a/drivers/usb/host/xhci-mtk-sch.c > > > +++ b/drivers/usb/host/xhci-mtk-sch.c > > > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset) > > > * Compared with hs bus, no matter what ep type, > > > * the hub will always delay one uframe to send data > > > */ > > > - for (j = 0; j < sch_ep->cs_count; j++) { > > > - tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe; > > > + for (j = 0; j < sch_ep->num_budget_microframes; j++) { > > > + tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j]; > > > if (tmp > FS_PAYLOAD_MAX) > > > return -ESCH_BW_OVERFLOW; > > > } > > > @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used) > > > { > > > struct mu3h_sch_tt *tt = sch_ep->sch_tt; > > > u32 base, num_esit; > > > - int bw_updated; > > > int i, j; > > > > > > num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit; > > > > > > - if (used) > > > - bw_updated = sch_ep->bw_cost_per_microframe; > > > - else > > > - bw_updated = -sch_ep->bw_cost_per_microframe; > > > - > > > for (i = 0; i < num_esit; i++) { > > > base = sch_ep->offset + i * sch_ep->esit; > > > > > > - for (j = 0; j < sch_ep->cs_count; j++) > > > - tt->fs_bus_bw[base + j] += bw_updated; > > > + for (j = 0; j < sch_ep->num_budget_microframes; j++) > > > + if (used) > > > + tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j]; > > > + else > > > + tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j]; > > > > I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth > > but I think this approach could trigger a problem. > See updat_bus_bw(), when add fs ep's bandwidth, it uses > bw_budget_table[], so prefer to use the same way > > > > > for example, if there are two endpoints scheduled in the same u-frame index, > > * ep1out = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0 > > * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... } --> y0 > > > > (If this is possible allocation from this patch), > > I guess xhci-mtk could have some problems on internal scheduling? > > Test it on dvt env. don't encounter issues; > As you can see In the above example, this patch starts to allow that allocation. Do you mean that we don't have to worry about such a case (on all MTK platforms)? thanks > Thanks > > > > > > } > > > > > > if (used) > > > > > -- > > > 2.18.0 > > > >
On Wed, 2021-08-04 at 22:06 +0800, Ikjoon Jang wrote: > Hi, > > On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun < > chunfeng.yun@mediatek.com> wrote: > > > > On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote: > > > Hi Chunfeng, > > > > > > On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun < > > > chunfeng.yun@mediatek.com> wrote: > > > > > > > > Use @bw_budget_table[] to update fs bus bandwidth due to > > > > not all microframes consume @bw_cost_per_microframe, see > > > > setup_sch_info(). > > > > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > > > --- > > > > drivers/usb/host/xhci-mtk-sch.c | 17 +++++++---------- > > > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/usb/host/xhci-mtk-sch.c > > > > b/drivers/usb/host/xhci-mtk-sch.c > > > > index 0bb1a6295d64..10c0f0f6461f 100644 > > > > --- a/drivers/usb/host/xhci-mtk-sch.c > > > > +++ b/drivers/usb/host/xhci-mtk-sch.c > > > > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct > > > > mu3h_sch_ep_info *sch_ep, int offset) > > > > * Compared with hs bus, no matter what ep > > > > type, > > > > * the hub will always delay one uframe to send > > > > data > > > > */ > > > > - for (j = 0; j < sch_ep->cs_count; j++) { > > > > - tmp = tt->fs_bus_bw[base + j] + sch_ep- > > > > >bw_cost_per_microframe; > > > > + for (j = 0; j < sch_ep->num_budget_microframes; > > > > j++) { > > > > + tmp = tt->fs_bus_bw[base + j] + sch_ep- > > > > >bw_budget_table[j]; > > > > if (tmp > FS_PAYLOAD_MAX) > > > > return -ESCH_BW_OVERFLOW; > > > > } > > > > @@ -534,21 +534,18 @@ static void update_sch_tt(struct > > > > mu3h_sch_ep_info *sch_ep, bool used) > > > > { > > > > struct mu3h_sch_tt *tt = sch_ep->sch_tt; > > > > u32 base, num_esit; > > > > - int bw_updated; > > > > int i, j; > > > > > > > > num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit; > > > > > > > > - if (used) > > > > - bw_updated = sch_ep->bw_cost_per_microframe; > > > > - else > > > > - bw_updated = -sch_ep->bw_cost_per_microframe; > > > > - > > > > for (i = 0; i < num_esit; i++) { > > > > base = sch_ep->offset + i * sch_ep->esit; > > > > > > > > - for (j = 0; j < sch_ep->cs_count; j++) > > > > - tt->fs_bus_bw[base + j] += bw_updated; > > > > + for (j = 0; j < sch_ep->num_budget_microframes; > > > > j++) > > > > + if (used) > > > > + tt->fs_bus_bw[base + j] += > > > > sch_ep->bw_budget_table[j]; > > > > + else > > > > + tt->fs_bus_bw[base + j] -= > > > > sch_ep->bw_budget_table[j]; > > > > > > I agree that xhci-mtk-sch still has more rooms for tt periodic > > > bandwidth > > > but I think this approach could trigger a problem. > > > > See updat_bus_bw(), when add fs ep's bandwidth, it uses > > bw_budget_table[], so prefer to use the same way > > > > > > > > for example, if there are two endpoints scheduled in the same u- > > > frame index, > > > * ep1out = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} > > > --> y0 > > > * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... } --> > > > y0 > > > > > > (If this is possible allocation from this patch), > > > I guess xhci-mtk could have some problems on internal scheduling? > > > > Test it on dvt env. don't encounter issues; > > > > As you can see In the above example, this patch starts to allow that > allocation. > Do you mean that we don't have to worry about such a case (on all MTK > platforms)? No, that is another question, when update bus_bw[] and fs_bus_bw[] for FS with TT should use the same bw_budget_table[] which is filled in setup_sch_info(). If the bw_budget_table[] is something wrong, we can prepare new patch to fix it. > > thanks > > > Thanks > > > > > > > > > } > > > > > > > > if (used) > > > > -- > > > > 2.18.0 > > > >
diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c index 0bb1a6295d64..10c0f0f6461f 100644 --- a/drivers/usb/host/xhci-mtk-sch.c +++ b/drivers/usb/host/xhci-mtk-sch.c @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset) * Compared with hs bus, no matter what ep type, * the hub will always delay one uframe to send data */ - for (j = 0; j < sch_ep->cs_count; j++) { - tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe; + for (j = 0; j < sch_ep->num_budget_microframes; j++) { + tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j]; if (tmp > FS_PAYLOAD_MAX) return -ESCH_BW_OVERFLOW; } @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used) { struct mu3h_sch_tt *tt = sch_ep->sch_tt; u32 base, num_esit; - int bw_updated; int i, j; num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit; - if (used) - bw_updated = sch_ep->bw_cost_per_microframe; - else - bw_updated = -sch_ep->bw_cost_per_microframe; - for (i = 0; i < num_esit; i++) { base = sch_ep->offset + i * sch_ep->esit; - for (j = 0; j < sch_ep->cs_count; j++) - tt->fs_bus_bw[base + j] += bw_updated; + for (j = 0; j < sch_ep->num_budget_microframes; j++) + if (used) + tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j]; + else + tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j]; } if (used)
Use @bw_budget_table[] to update fs bus bandwidth due to not all microframes consume @bw_cost_per_microframe, see setup_sch_info(). Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/usb/host/xhci-mtk-sch.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)