Re: [PATCH 2/2] usb: xhci-mtk: relax periodic TT bandwidth checking

[Chunfeng Yun]

On Tue, 2021-03-30 at 16:06 +0800, Ikjoon Jang wrote:
> Software bandwidth checking logics used by xhci-mtk puts
> a quite heavy constraints to TT periodic endpoint allocations.
> 
> This patch provides a relaxed bandwidth calculation by
> - Allowing multiple periodic transactions in a same microframe
>   for a device with multiple interrupt endpoints.
> - Using best case budget instead of maximum number of
>   complete-split when calculating byte budgets on lower speed bus
> 
> Without this patch, a typical full speed audio headset with
> 3 periodic endpoints (audio isoc-in/out, input int-in) cannot be
> configured with xhci-mtk.
> 
> Signed-off-by: Ikjoon Jang <ikjn@xxxxxxxxxxxx>
> ---
cc Yaqii Wu <Yaqii.Wu@xxxxxxxxxxxx>

I'll test it, thanks

> 
>  drivers/usb/host/xhci-mtk-sch.c | 68 ++++++++++-----------------------
>  drivers/usb/host/xhci-mtk.h     |  2 -
>  2 files changed, 20 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> index 0cb41007ec65..76827e48049a 100644
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -388,13 +388,17 @@ static void setup_sch_info(struct xhci_ep_ctx *ep_ctx,
>  		} else { /* INT_IN_EP or ISOC_IN_EP */
>  			bwb_table[0] = 0; /* start split */
>  			bwb_table[1] = 0; /* idle */
> +
> +			sch_ep->num_budget_microframes += 2;
> +			if (sch_ep->num_budget_microframes > sch_ep->esit)
> +				sch_ep->num_budget_microframes = sch_ep->esit;
>  			/*
>  			 * due to cs_count will be updated according to cs
>  			 * position, assign all remainder budget array
>  			 * elements as @bw_cost_per_microframe, but only first
>  			 * @num_budget_microframes elements will be used later
>  			 */
> -			for (i = 2; i < TT_MICROFRAMES_MAX; i++)
> +			for (i = 2; i < sch_ep->num_budget_microframes; i++)
>  				bwb_table[i] =	sch_ep->bw_cost_per_microframe;
>  		}
>  	}
> @@ -449,20 +453,17 @@ static void update_bus_bw(struct mu3h_sch_bw_info *sch_bw,
>  static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
>  {
>  	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> -	u32 num_esit, tmp;
> -	int base;
>  	int i, j;
> +	const int nr_lower_uframes =
> +		DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
>  
> -	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> -	for (i = 0; i < num_esit; i++) {
> -		base = offset + i * sch_ep->esit;
> -
> +	for (i = offset; i < XHCI_MTK_MAX_ESIT; i += sch_ep->esit) {
>  		/*
>  		 * 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 < nr_lower_uframes; j++) {
> +			u32 tmp = tt->fs_bus_bw[i + j + 1] + sch_ep->bw_cost_per_microframe;
>  			if (tmp > FS_PAYLOAD_MAX)
>  				return -ESCH_BW_OVERFLOW;
>  		}
> @@ -473,11 +474,9 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
>  
>  static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
>  {
> -	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
>  	u32 extra_cs_count;
>  	u32 start_ss, last_ss;
>  	u32 start_cs, last_cs;
> -	int i;
>  
>  	if (!sch_ep->sch_tt)
>  		return 0;
> @@ -494,10 +493,6 @@ static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
>  		if (!(start_ss == 7 || last_ss < 6))
>  			return -ESCH_SS_Y6;
>  
> -		for (i = 0; i < sch_ep->cs_count; i++)
> -			if (test_bit(offset + i, tt->ss_bit_map))
> -				return -ESCH_SS_OVERLAP;
> -
>  	} else {
>  		u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
>  
> @@ -524,19 +519,7 @@ static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
>  		if (cs_count > 7)
>  			cs_count = 7; /* HW limit */
>  
> -		if (test_bit(offset, tt->ss_bit_map))
> -			return -ESCH_SS_OVERLAP;
> -
>  		sch_ep->cs_count = cs_count;
> -		/* one for ss, the other for idle */
> -		sch_ep->num_budget_microframes = cs_count + 2;
> -
> -		/*
> -		 * if interval=1, maxp >752, num_budge_micoframe is larger
> -		 * than sch_ep->esit, will overstep boundary
> -		 */
> -		if (sch_ep->num_budget_microframes > sch_ep->esit)
> -			sch_ep->num_budget_microframes = sch_ep->esit;
>  	}
>  
>  	return check_fs_bus_bw(sch_ep, offset);
> @@ -545,31 +528,18 @@ static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
>  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 bits;
> -	int i, j;
> -
> -	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> -	bits = (sch_ep->ep_type == ISOC_OUT_EP) ? sch_ep->cs_count : 1;
> +	int i, j, bw_updated;
> +	const int nr_lower_uframes =
> +		DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
>  
>  	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 < bits; j++) {
> -			if (used)
> -				set_bit(base + j, tt->ss_bit_map);
> -			else
> -				clear_bit(base + j, tt->ss_bit_map);
> -		}
> -
> -		for (j = 0; j < sch_ep->cs_count; j++)
> -			tt->fs_bus_bw[base + j] += bw_updated;
> +	for (i = sch_ep->offset; i < XHCI_MTK_MAX_ESIT; i += sch_ep->esit) {
> +		for (j = 0; j < nr_lower_uframes; j++)
> +			tt->fs_bus_bw[i+ j + 1] += bw_updated;
>  	}
>  
>  	if (used)
> @@ -634,9 +604,11 @@ static int check_sch_bw(struct mu3h_sch_bw_info *sch_bw,
>  		if (min_bw > worst_bw) {
>  			min_bw = worst_bw;
>  			found = i;
> +			/* fastpath: bandwidth contributions to host is low
> +			 * when it's fs/ls */
> +			if (sch_ep->sch_tt || min_bw == 0)
> +				break;
>  		}
> -		if (min_bw == 0)
> -			break;
>  	}
>  
>  	/* check bandwidth */
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index 621ec1a85009..8a879f99ae1c 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -20,12 +20,10 @@
>  #define XHCI_MTK_MAX_ESIT	64
>  
>  /**
> - * @ss_bit_map: used to avoid start split microframes overlay
>   * @fs_bus_bw: array to keep track of bandwidth already used for FS
>   * @ep_list: Endpoints using this TT
>   */
>  struct mu3h_sch_tt {
> -	DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
>  	u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
>  	struct list_head ep_list;
>  };