From patchwork Tue Nov 17 00:51:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 7631611 Return-Path: X-Original-To: patchwork-linux-rockchip@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 9137AC05CA for ; Tue, 17 Nov 2015 00:52:26 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 22953203DF for ; Tue, 17 Nov 2015 00:52:25 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 56C732053E for ; Tue, 17 Nov 2015 00:52:23 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZyUVS-0002Je-UT; Tue, 17 Nov 2015 00:52:22 +0000 Received: from mail-pa0-x233.google.com ([2607:f8b0:400e:c03::233]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZyUVF-00026i-VZ for linux-rockchip@lists.infradead.org; Tue, 17 Nov 2015 00:52:15 +0000 Received: by pacej9 with SMTP id ej9so85138791pac.2 for ; Mon, 16 Nov 2015 16:51:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=2O9mB2JzQ5+irt5rJLKCxRjwasNRv+CCWovC89M+j18=; b=JvLRO0Ik+dCBkw5wOtC4xaZs10mm+HTLOggaMp4tP0d4u7zFRjnVmxhT/Vugx5m9qT xeQjKKozCQSLsNHV4k5wUE2WO7xwARantyXC/+o9+yaQCT42TPwuf+J4ge4gOYoJm54C /zjNkhP3KluMlVqS6vTyT1bR8wNk0KjV4xxhk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=2O9mB2JzQ5+irt5rJLKCxRjwasNRv+CCWovC89M+j18=; b=VaUzfAnz6XRBlbcIZHU3+2fSIRYKEftiwHEZmUbr44/0bJNrkfg04EwF34kGl8mYhU yznmSmeyp/gKafCBeXxvQTAkFAouIF8FG8I445BEfwRv9lSOa+AMkwtJor3cAXgOg8Iq pPA3sTmfhantOIIK8zuwnj4cVVAbP6M9hriPsZXNzoJgVwBYp9Azj81bseiP/UjsiMpF qbKCpRVcUM+5geeVb7UsUcOxCeb55pKQn/SMTUq86KV6ViI3XE731pckFaJ3OVVtTbHJ 5jLtMCnileZIcmGZqBwMGsG6r+b7TGFhwqexSd0AottK5b8jkfujwO0qJZUd5dld7cx6 VOFA== X-Gm-Message-State: ALoCoQk0yp+MWeNXvr8IlC/tAxLA+r76aN3MjMLkBM/JVqkqCcyhXvptNH2ZoM4Ex0cicULLDtrF X-Received: by 10.67.23.229 with SMTP id id5mr59734746pad.64.1447721509310; Mon, 16 Nov 2015 16:51:49 -0800 (PST) Received: from tictac.mtv.corp.google.com ([172.22.65.76]) by smtp.gmail.com with ESMTPSA id hg2sm16316243pbb.7.2015.11.16.16.51.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 16 Nov 2015 16:51:48 -0800 (PST) From: Douglas Anderson To: John Youn , balbi@ti.com Subject: [PATCH v3 7/8] usb: dwc2: host: Add a delay before releasing periodic bandwidth Date: Mon, 16 Nov 2015 16:51:23 -0800 Message-Id: <1447721484-22548-8-git-send-email-dianders@chromium.org> X-Mailer: git-send-email 2.6.0.rc2.230.g3dd15c0 In-Reply-To: <1447721484-22548-1-git-send-email-dianders@chromium.org> References: <1447721484-22548-1-git-send-email-dianders@chromium.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151116_165210_309565_927892DC X-CRM114-Status: GOOD ( 28.10 ) X-Spam-Score: -2.7 (--) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gregory.herrero@intel.com, =?UTF-8?q?Heiko=20St=C3=BCbner?= , johnyoun@synopsys.com, gregkh@linuxfoundation.org, ming.lei@canonical.com, linux-usb@vger.kernel.org, Douglas Anderson , linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, yousaf.kaukab@intel.com, stern@rowland.harvard.edu, Yunzhi Li , Julius Werner , dinguyen@opensource.altera.com MIME-Version: 1.0 Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We'd like to be able to use HCD_BH in order to speed up the dwc2 host interrupt handler quite a bit. However, according to the kernel doc for usb_submit_urb() (specifically the part about "Reserved Bandwidth Transfers"), we need to keep a reservation active as long as a device driver keeps submitting. That was easy to do when we gave back the URB in the interrupt context: we just looked at when our queue was empty and released the reserved bandwidth then. ...but now we need a little more complexity. We'll follow EHCI's lead in commit 9118f9eb4f1e ("USB: EHCI: improve interrupt qh unlink") and add a 5ms delay. Since we don't have a whole timer infrastructure in dwc2, we'll just add a timer per QH. The overhead for this is very small. Signed-off-by: Douglas Anderson --- Changes in v3: - Moved periodic bandwidth release delay patch later in the series. Changes in v2: - Periodic bandwidth release delay new for V2 drivers/usb/dwc2/hcd.h | 6 ++ drivers/usb/dwc2/hcd_queue.c | 252 ++++++++++++++++++++++++++++++++----------- 2 files changed, 193 insertions(+), 65 deletions(-) diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h index ff99cb44c89d..2d9bb45c1777 100644 --- a/drivers/usb/dwc2/hcd.h +++ b/drivers/usb/dwc2/hcd.h @@ -211,6 +211,7 @@ enum dwc2_transaction_type { /** * struct dwc2_qh - Software queue head structure * + * @hsotg: The HCD state structure for the DWC OTG controller * @ep_type: Endpoint type. One of the following values: * - USB_ENDPOINT_XFER_CONTROL * - USB_ENDPOINT_XFER_BULK @@ -250,13 +251,16 @@ enum dwc2_transaction_type { * @n_bytes: Xfer Bytes array. Each element corresponds to a transfer * descriptor and indicates original XferSize value for the * descriptor + * @unreserve_timer: Timer for releasing periodic reservation. * @tt_buffer_dirty True if clear_tt_buffer_complete is pending + * @unreserve_pending: True if we planned to unreserve but haven't yet. * * A Queue Head (QH) holds the static characteristics of an endpoint and * maintains a list of transfers (QTDs) for that endpoint. A QH structure may * be entered in either the non-periodic or periodic schedule. */ struct dwc2_qh { + struct dwc2_hsotg *hsotg; u8 ep_type; u8 ep_is_in; u16 maxp; @@ -279,7 +283,9 @@ struct dwc2_qh { struct dwc2_hcd_dma_desc *desc_list; dma_addr_t desc_list_dma; u32 *n_bytes; + struct timer_list unreserve_timer; unsigned tt_buffer_dirty:1; + unsigned unreserve_pending:1; }; /** diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c index c5a2edb04bec..a287e52f9a63 100644 --- a/drivers/usb/dwc2/hcd_queue.c +++ b/drivers/usb/dwc2/hcd_queue.c @@ -53,6 +53,101 @@ #include "core.h" #include "hcd.h" +/* Wait this long before releasing periodic reservation */ +#define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5)) + +/** + * dwc2_do_unreserve() - Actually release the periodic reservation + * + * This function actually releases the periodic bandwidth that was reserved + * by the given qh. + * + * @hsotg: The HCD state structure for the DWC OTG controller + * @qh: QH for the periodic transfer. + */ +static void dwc2_do_unreserve(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) +{ + int start = qh->start_usecs; + int utime = qh->usecs; + int i; + + assert_spin_locked(&hsotg->lock); + + WARN_ON(!qh->unreserve_pending); + + /* No more unreserve pending--we're doing it */ + qh->unreserve_pending = false; + + if (WARN_ON(!list_empty(&qh->qh_list_entry))) + list_del_init(&qh->qh_list_entry); + + /* Update claimed usecs per (micro)frame */ + hsotg->periodic_usecs -= qh->usecs; + + /* Release periodic channel reservation */ + if (hsotg->core_params->uframe_sched <= 0) { + hsotg->periodic_channels--; + return; + } + + bitmap_clear(hsotg->periodic_bitmap, start, utime); + + if (qh->do_split) { + for (i = start / EARLY_FRAME_USEC; + i < DIV_ROUND_UP(start + utime - 1, EARLY_FRAME_USEC); + i++) + hsotg->has_split[i] = false; + } +} + +/** + * dwc2_unreserve_timer_fn() - Timer function to release periodic reservation + * + * According to the kernel doc for usb_submit_urb() (specifically the part about + * "Reserved Bandwidth Transfers"), we need to keep a reservation active as + * long as a device driver keeps submitting. Since we're using HCD_BH to give + * back the URB we need to give the driver a little bit of time before we + * release the reservation. This worker is called after the appropriate + * delay. + * + * @work: Pointer to a qh unreserve_work. + */ +static void dwc2_unreserve_timer_fn(unsigned long data) +{ + struct dwc2_qh *qh = (struct dwc2_qh *)data; + struct dwc2_hsotg *hsotg = qh->hsotg; + unsigned long flags; + + /* + * Wait for the lock, or for us to be scheduled again. We + * could be scheduled again if: + * - We started executing but didn't get the lock yet. + * - A new reservation came in, but cancel didn't take effect + * because we already started executing. + * - The timer has been kicked again. + * In that case cancel and wait for the next call. + */ + while (!spin_trylock_irqsave(&hsotg->lock, flags)) { + if (timer_pending(&qh->unreserve_timer)) + return; + } + + /* + * Might be no more unreserve pending if: + * - We started executing but didn't get the lock yet. + * - A new reservation came in, but cancel didn't take effect + * because we already started executing. + * + * We can't put this in the loop above because unreserve_pending needs + * to be accessed under lock, so we can only check it once we got the + * lock. + */ + if (qh->unreserve_pending) + dwc2_do_unreserve(hsotg, qh); + + spin_unlock_irqrestore(&hsotg->lock, flags); +} + /** * dwc2_qh_init() - Initializes a QH structure * @@ -71,6 +166,9 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh, dev_vdbg(hsotg->dev, "%s()\n", __func__); /* Initialize QH */ + qh->hsotg = hsotg; + setup_timer(&qh->unreserve_timer, dwc2_unreserve_timer_fn, + (unsigned long)qh); qh->ep_type = dwc2_hcd_get_pipe_type(&urb->pipe_info); qh->ep_is_in = dwc2_hcd_is_pipe_in(&urb->pipe_info) ? 1 : 0; @@ -240,6 +338,15 @@ struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg, */ void dwc2_hcd_qh_free(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) { + /* Make sure any unreserve work is finished. */ + if (del_timer_sync(&qh->unreserve_timer)) { + unsigned long flags; + + spin_lock_irqsave(&hsotg->lock, flags); + dwc2_do_unreserve(hsotg, qh); + spin_unlock_irqrestore(&hsotg->lock, flags); + } + if (hsotg->core_params->dma_desc_enable > 0) dwc2_hcd_qh_free_ddma(hsotg, qh); kfree(qh); @@ -483,55 +590,76 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) { int status; - if (hsotg->core_params->uframe_sched > 0) { - int frame = -1; + status = dwc2_check_max_xfer_size(hsotg, qh); + if (status) { + dev_dbg(hsotg->dev, + "%s: Channel max transfer size too small for periodic transfer\n", + __func__); + return status; + } - status = dwc2_find_uframe(hsotg, qh); - if (status == 0) - frame = 7; - else if (status > 0) - frame = status - 1; + /* Cancel pending unreserve; if canceled OK, unreserve was pending */ + if (del_timer(&qh->unreserve_timer)) + WARN_ON(!qh->unreserve_pending); - /* Set the new frame up */ - if (frame >= 0) { - qh->sched_uframe = frame; + /* + * Only need to reserve if there's not an unreserve pending, since if an + * unreserve is pending then by definition our old reservation is still + * valid. Unreserve might still be pending even if we didn't cancel if + * dwc2_unreserve_timer_fn() already started. Code in the timer handles + * that case. + */ + if (!qh->unreserve_pending) { + if (hsotg->core_params->uframe_sched > 0) { + int frame = -1; + + status = dwc2_find_uframe(hsotg, qh); + if (status == 0) + frame = 7; + else if (status > 0) + frame = status - 1; + + /* Set the new frame up */ + if (frame >= 0) { + qh->sched_uframe = frame; + qh->sched_frame = ((qh->sched_frame + 7) & ~7) | + frame; + dwc2_sch_dbg(hsotg, + "sched_p %p sch=%04x, uf=%d, us=%d\n", + qh, qh->sched_frame, frame, + qh->start_usecs); + } - /* The next frame that matches our scheduled uframe */ - qh->sched_frame = ((qh->sched_frame + 7) & ~7) | frame; - dwc2_sch_dbg(hsotg, - "sched_p %p sch=%04x, uf=%d, us=%d\n", - qh, qh->sched_frame, frame, - qh->start_usecs); + if (status > 0) + status = 0; + } else { + status = dwc2_periodic_channel_available(hsotg); + if (status) { + dev_info(hsotg->dev, + "%s: No host channel available for periodic transfer\n", + __func__); + return status; + } + + status = dwc2_check_periodic_bandwidth(hsotg, qh); } - if (status > 0) - status = 0; - } else { - status = dwc2_periodic_channel_available(hsotg); if (status) { - dev_info(hsotg->dev, - "%s: No host channel available for periodic transfer\n", - __func__); + dev_dbg(hsotg->dev, + "%s: Insufficient periodic bandwidth for periodic transfer\n", + __func__); return status; } - status = dwc2_check_periodic_bandwidth(hsotg, qh); - } + if (hsotg->core_params->uframe_sched <= 0) + /* Reserve periodic channel */ + hsotg->periodic_channels++; - if (status) { - dev_dbg(hsotg->dev, - "%s: Insufficient periodic bandwidth for periodic transfer\n", - __func__); - return status; + /* Update claimed usecs per (micro)frame */ + hsotg->periodic_usecs += qh->usecs; } - status = dwc2_check_max_xfer_size(hsotg, qh); - if (status) { - dev_dbg(hsotg->dev, - "%s: Channel max transfer size too small for periodic transfer\n", - __func__); - return status; - } + qh->unreserve_pending = 0; if (hsotg->core_params->dma_desc_enable > 0) /* Don't rely on SOF and start in ready schedule */ @@ -541,13 +669,6 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) list_add_tail(&qh->qh_list_entry, &hsotg->periodic_sched_inactive); - if (hsotg->core_params->uframe_sched <= 0) - /* Reserve periodic channel */ - hsotg->periodic_channels++; - - /* Update claimed usecs per (micro)frame */ - hsotg->periodic_usecs += qh->usecs; - return status; } @@ -561,29 +682,31 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) static void dwc2_deschedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) { - int start = qh->start_usecs; - int utime = qh->usecs; - int i; + bool did_modify; - list_del_init(&qh->qh_list_entry); + assert_spin_locked(&hsotg->lock); - /* Update claimed usecs per (micro)frame */ - hsotg->periodic_usecs -= qh->usecs; - - if (hsotg->core_params->uframe_sched <= 0) { - /* Release periodic channel reservation */ - hsotg->periodic_channels--; - return; - } - - bitmap_clear(hsotg->periodic_bitmap, start, utime); + /* + * Schedule the unreserve to happen in a little bit. Cases here: + * - Unreserve worker might be sitting there waiting to grab the lock. + * In this case it will notice it's been schedule again and will + * quit. + * - Unreserve worker might not be scheduled. + * + * We should never already be scheduled since dwc2_schedule_periodic() + * should have canceled the scheduled unreserve timer (hence the + * warning on did_modify). + * + * We add + 1 to the timer to guarantee that at least 1 jiffy has + * passed (otherwise if the jiffy counter might tick right after we + * read it and we'll get no delay). + */ + did_modify = mod_timer(&qh->unreserve_timer, + jiffies + DWC2_UNRESERVE_DELAY + 1); + WARN_ON(did_modify); + qh->unreserve_pending = 1; - if (qh->do_split) { - for (i = start / EARLY_FRAME_USEC; - i < DIV_ROUND_UP(start + utime - 1, EARLY_FRAME_USEC); - i++) - hsotg->has_split[i] = false; - } + list_del_init(&qh->qh_list_entry); } /** @@ -710,7 +833,6 @@ static void dwc2_sched_periodic_split(struct dwc2_hsotg *hsotg, qh->sched_frame = frame_number; if (hsotg->core_params->uframe_sched > 0) - /* The next frame that matches our scheduled uframe */ qh->sched_frame = ((qh->sched_frame + 7) & ~7) | qh->sched_uframe; else