| Message ID | 44b1e6169ea672ddc6f6d035ef081bc96b1c1bec.1542336692.git.thinhn@synopsys.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 6abfa0f5bb7cce98c89e2c28fcea31c17200890e |
| Headers | show |
| Series | [v3,1/2] usb: gadget: Introduce frame_number to usb_request | expand |
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: > Implement the new frame_number API to report the isochronous interval > frame number. This patch checks and reports the interval in which the > isoc transfer was transmitted or received via the Isoc-First TRB SOF > number field. > > Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> > --- > Change in v3: > - Implement the change with the redefined frame_number meaning > Change in v2: > - Capture frame number at request cleanup > > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/gadget.c | 13 +++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index ed0359d1216d..2c9f7a93147a 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -777,6 +777,7 @@ enum dwc3_link_state { > #define DWC3_TRB_CTRL_ISP_IMI BIT(10) > #define DWC3_TRB_CTRL_IOC BIT(11) > #define DWC3_TRB_CTRL_SID_SOFN(n) (((n) & 0xffff) << 14) > +#define DWC3_TRB_CTRL_GET_SID_SOFN(n) (((n) & (0xffff << 14)) >> 14) > > #define DWC3_TRBCTL_TYPE(n) ((n) & (0x3f << 4)) > #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1) > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 679c12e14522..2de563124fc1 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, > if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) > trb->ctrl &= ~DWC3_TRB_CTRL_HWO; > > + /* > + * For isochronous transfers, the first TRB in a service interval must > + * have the Isoc-First type. Track and report its interval frame number. > + */ > + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && > + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { > + unsigned int frame_number; > + > + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); > + frame_number &= ~(dep->interval - 1); > + req->request.frame_number = frame_number; > + } could you refresh my memory on how this is going to be used?
Hi Felipe, On 12/5/2018 1:15 AM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >> Implement the new frame_number API to report the isochronous interval >> frame number. This patch checks and reports the interval in which the >> isoc transfer was transmitted or received via the Isoc-First TRB SOF >> number field. >> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> >> --- >> Change in v3: >> - Implement the change with the redefined frame_number meaning >> Change in v2: >> - Capture frame number at request cleanup >> >> drivers/usb/dwc3/core.h | 1 + >> drivers/usb/dwc3/gadget.c | 13 +++++++++++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index ed0359d1216d..2c9f7a93147a 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -777,6 +777,7 @@ enum dwc3_link_state { >> #define DWC3_TRB_CTRL_ISP_IMI BIT(10) >> #define DWC3_TRB_CTRL_IOC BIT(11) >> #define DWC3_TRB_CTRL_SID_SOFN(n) (((n) & 0xffff) << 14) >> +#define DWC3_TRB_CTRL_GET_SID_SOFN(n) (((n) & (0xffff << 14)) >> 14) >> >> #define DWC3_TRBCTL_TYPE(n) ((n) & (0x3f << 4)) >> #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1) >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 679c12e14522..2de563124fc1 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, >> if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) >> trb->ctrl &= ~DWC3_TRB_CTRL_HWO; >> >> + /* >> + * For isochronous transfers, the first TRB in a service interval must >> + * have the Isoc-First type. Track and report its interval frame number. >> + */ >> + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && >> + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { >> + unsigned int frame_number; >> + >> + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); >> + frame_number &= ~(dep->interval - 1); >> + req->request.frame_number = frame_number; >> + } > could you refresh my memory on how this is going to be used? > Sure. This informs the upper layer driver what interval the isoc request went out. The users can use this information for applications that require synchronization with the host. Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 679c12e14522..2de563124fc1 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, >>> if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) >>> trb->ctrl &= ~DWC3_TRB_CTRL_HWO; >>> >>> + /* >>> + * For isochronous transfers, the first TRB in a service interval must >>> + * have the Isoc-First type. Track and report its interval frame number. >>> + */ >>> + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && >>> + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { >>> + unsigned int frame_number; >>> + >>> + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); >>> + frame_number &= ~(dep->interval - 1); >>> + req->request.frame_number = frame_number; >>> + } >> could you refresh my memory on how this is going to be used? >> > > Sure. This informs the upper layer driver what interval the isoc request > went out. The users can use this information for applications that > require synchronization with the host. Thanks. Do you have an example of a gadget that would use it? Perhaps g_webcam can rely on this to improve its scheduling? I have this in my testing/next already, just looking for an actual user :-)
Hi Felipe, On 12/6/2018 10:01 PM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index 679c12e14522..2de563124fc1 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, >>>> if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) >>>> trb->ctrl &= ~DWC3_TRB_CTRL_HWO; >>>> >>>> + /* >>>> + * For isochronous transfers, the first TRB in a service interval must >>>> + * have the Isoc-First type. Track and report its interval frame number. >>>> + */ >>>> + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && >>>> + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { >>>> + unsigned int frame_number; >>>> + >>>> + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); >>>> + frame_number &= ~(dep->interval - 1); >>>> + req->request.frame_number = frame_number; >>>> + } >>> could you refresh my memory on how this is going to be used? >>> >> Sure. This informs the upper layer driver what interval the isoc request >> went out. The users can use this information for applications that >> require synchronization with the host. > Thanks. Do you have an example of a gadget that would use it? Perhaps > g_webcam can rely on this to improve its scheduling? I have this in my > testing/next already, just looking for an actual user :-) > One of our internal IP validation tools uses this to track the isoc data against the interval it should be in, and it has its custom protocol for synchronization with the host. But beside debugging purposes, I can also see that it can be useful for devices that are time-sensitive. I'm not familiar with g_webcam, but I can imagine that this option can be useful for it. When the user queues for a request, it now has the control of what data should come next. That is, when the user queues for a request, the user can get the current frame number by calling usb_gadget_frame_number(). Base on the current frame number and what interval the request went out, the user can determine what data should be queued next and discard any stale ones. Without this usb_request->frame_number, the user will not know how many intervals the request is delayed by. Thinh
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index ed0359d1216d..2c9f7a93147a 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -777,6 +777,7 @@ enum dwc3_link_state { #define DWC3_TRB_CTRL_ISP_IMI BIT(10) #define DWC3_TRB_CTRL_IOC BIT(11) #define DWC3_TRB_CTRL_SID_SOFN(n) (((n) & 0xffff) << 14) +#define DWC3_TRB_CTRL_GET_SID_SOFN(n) (((n) & (0xffff << 14)) >> 14) #define DWC3_TRBCTL_TYPE(n) ((n) & (0x3f << 4)) #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 679c12e14522..2de563124fc1 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + /* + * For isochronous transfers, the first TRB in a service interval must + * have the Isoc-First type. Track and report its interval frame number. + */ + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { + unsigned int frame_number; + + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); + frame_number &= ~(dep->interval - 1); + req->request.frame_number = frame_number; + } + /* * If we're dealing with unaligned size OUT transfer, we will be left * with one TRB pending in the ring. We need to manually clear HWO bit
Implement the new frame_number API to report the isochronous interval frame number. This patch checks and reports the interval in which the isoc transfer was transmitted or received via the Isoc-First TRB SOF number field. Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> --- Change in v3: - Implement the change with the redefined frame_number meaning Change in v2: - Capture frame number at request cleanup drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)