Message ID | 1462634508-24961-6-git-send-email-ivo.g.dimitrov.75@gmail.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1az45b-0002ka-Ij; Sat, 07 May 2016 15:24:19 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-8) with esmtp id 1az45Z-0001Lo-l4; Sat, 07 May 2016 17:24:19 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751373AbcEGPXw (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sat, 7 May 2016 11:23:52 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34276 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750886AbcEGPW0 (ORCPT <rfc822; linux-media@vger.kernel.org>); Sat, 7 May 2016 11:22:26 -0400 Received: by mail-wm0-f68.google.com with SMTP id n129so13306352wmn.1; Sat, 07 May 2016 08:22:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=O5KY0dX3icRMF7fOYcVNw5UJFH9R7a/Gg1JVsW0+jdA=; b=RsjxTNjEU6s6OI7n+xN9R6Tzlyav1oqKtpxFhuiMMpk/iiRVYHEjR31G0otrRbCDGj F6JfHGQjvCxtXKkcpCjsVSJj8i8EBx3QEjDuRkYTjFdJpI87x3f0oKpbCPxXCbO3abpR BZSlYYxknx+mX3Rtn7ew5UFpU6G2reLQignoCUIB4JujOhIg5RPM1dWd2Csh2HLYNzr6 dedQ4CUq0TO8iwsWHZSil7/qDIYpATvYIVl1f9mQUhI/mJa+0avqvM+SfwnLU8Sb6OGn pTuWbu57upfYvJQA9jcmQXWMAC+JnAmcLUQoclOx0lrUF1f/rHV4gH+dI5a2EYPNRjTd cHXg== 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=O5KY0dX3icRMF7fOYcVNw5UJFH9R7a/Gg1JVsW0+jdA=; b=PJtpsAYH0LZbs+l7coiO9myWT2z/eiOMOfpS0KapXUgqGI7B8hDD7lQPRaVHvlDp/s 1lHc98eI3Sxqb0+mLIn1rEvFILJQQ0oWCGra3cX6KJvInA97fvD07h9+cvhbQJGa7H59 4rn4XygohSdNkSh1Uo8AqlnFQ+Ht2chx7eZKBNWxlV2gA8WvdzPpU8hzgcHUpVzGjYQ0 mgEBearSeMWvuG7WDCf7X5NHTDTtW/4qFUaoW5XshSx5yfkMm6gypLZ9IXPifBqfF+sk KY6AHH/rmaPQQhAxIZhsNYRQ08d8q7QAp39FZQiOPuIiuIqLID6bBWXlCAkBuN0xeGIO 6qyw== X-Gm-Message-State: AOPr4FUNxHeJ8OuQ2/vdZgxtyvyjeeAa5yWqlDJQRD8M4BgPcQH0SkWAqCY4y8kiawMKZw== X-Received: by 10.28.126.145 with SMTP id z139mr2649080wmc.81.1462634544557; Sat, 07 May 2016 08:22:24 -0700 (PDT) Received: from localhost.localdomain ([46.249.74.23]) by smtp.gmail.com with ESMTPSA id jq1sm20737835wjc.28.2016.05.07.08.22.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 07 May 2016 08:22:24 -0700 (PDT) From: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> To: robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, thierry.reding@gmail.com, bcousson@baylibre.com, tony@atomide.com, linux@arm.linux.org.uk, mchehab@osg.samsung.com Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, sre@kernel.org, pali.rohar@gmail.com, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> Subject: [PATCH 5/7] ARM: OMAP: dmtimer: Do not call PM runtime functions when not needed. Date: Sat, 7 May 2016 18:21:46 +0300 Message-Id: <1462634508-24961-6-git-send-email-ivo.g.dimitrov.75@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1462634508-24961-1-git-send-email-ivo.g.dimitrov.75@gmail.com> References: <1462634508-24961-1-git-send-email-ivo.g.dimitrov.75@gmail.com> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.5.7.151815 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DKIM_SIGNATURE 0, NO_URI_HTTPS 0, REFERENCES 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __DATE_TZ_RU 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_RCPTS_TO_X5 0, __PHISH_SPEAR_STRUCTURE_1 0, __REFERENCES 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0, __YOUTUBE_RCVD 0' |
Commit Message
Ivaylo Dimitrov
May 7, 2016, 3:21 p.m. UTC
once omap_dm_timer_start() is called, which calls omap_dm_timer_enable()
and thus pm_runtime_get_sync(), it doesn't make sense to call PM runtime
functions again before omap_dm_timer_stop is called(). Otherwise PM runtime
functions called in omap_dm_timer_enable/disable lead to long and unneeded
delays.
Fix that by implementing an "enabled" counter, so the PM runtime functions
get called only when really needed.
Without that patch Nokia N900 IR TX driver (ir-rx51) does not function.
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
arch/arm/plat-omap/dmtimer.c | 9 ++++++++-
arch/arm/plat-omap/include/plat/dmtimer.h | 1 +
2 files changed, 9 insertions(+), 1 deletion(-)
Comments
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160507 08:24]: > once omap_dm_timer_start() is called, which calls omap_dm_timer_enable() > and thus pm_runtime_get_sync(), it doesn't make sense to call PM runtime > functions again before omap_dm_timer_stop is called(). Otherwise PM runtime > functions called in omap_dm_timer_enable/disable lead to long and unneeded > delays. > > Fix that by implementing an "enabled" counter, so the PM runtime functions > get called only when really needed. > > Without that patch Nokia N900 IR TX driver (ir-rx51) does not function. We should use pm_runtime for the refcounting though and call PM runtime unconditionally. Can you try to follow the standard PM runtime usage like this: init: pm_runtime_use_autosuspend(&timer->pdev->dev); pm_runtime_set_autosuspend_delay(&timer->pdev->dev, 200); pm_runtime_enable(&timer->pdev->dev); ... enable: pm_runtime_get_sync(&timer->pdev->dev); ... disable: pm_runtime_mark_last_busy(&timer->pdev->dev); pm_runtime_put_autosuspend(&timer->pdev->dev); ... exit: pm_runtime_dont_use_autosuspend(&timer->pdev->dev); pm_runtime_put_sync(&timer->pdev->dev); pm_runtime_disable(&timer->pdev->dev); No idea what the timeout should be, maybe less than 200 ms. Also we need to test that off idle still works with timer1, that might need special handling. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 9.05.2016 22:36, Tony Lindgren wrote: > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160507 08:24]: >> once omap_dm_timer_start() is called, which calls omap_dm_timer_enable() >> and thus pm_runtime_get_sync(), it doesn't make sense to call PM runtime >> functions again before omap_dm_timer_stop is called(). Otherwise PM runtime >> functions called in omap_dm_timer_enable/disable lead to long and unneeded >> delays. >> >> Fix that by implementing an "enabled" counter, so the PM runtime functions >> get called only when really needed. >> >> Without that patch Nokia N900 IR TX driver (ir-rx51) does not function. > Well, I just tested again, with the $subject patch reverted and contradictory to my own words, it worked just fine. I believe the reason is that I did hrtimer "migration" after I did the $subject patch. I was thinking the reason for the slow transmission was PWM dmtimer, but now it turns out it has been the "pulse" dmtimer. So, I think the $subject patch should be dropped. > We should use pm_runtime for the refcounting though and call PM runtime > unconditionally. Can you try to follow the standard PM runtime usage > like this: > It works without that, but on the other hand, I finally have some reference on how PM runtime API should be called :). > init: > pm_runtime_use_autosuspend(&timer->pdev->dev); > pm_runtime_set_autosuspend_delay(&timer->pdev->dev, 200); > pm_runtime_enable(&timer->pdev->dev); > ... > enable: > pm_runtime_get_sync(&timer->pdev->dev); > ... > disable: > pm_runtime_mark_last_busy(&timer->pdev->dev); > pm_runtime_put_autosuspend(&timer->pdev->dev); > ... > exit: > pm_runtime_dont_use_autosuspend(&timer->pdev->dev); > pm_runtime_put_sync(&timer->pdev->dev); > pm_runtime_disable(&timer->pdev->dev); > > No idea what the timeout should be, maybe less than 200 ms. Also we need > to test that off idle still works with timer1, that might need special > handling. > Thanks, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160509 13:52]: > Hi, > > On 9.05.2016 22:36, Tony Lindgren wrote: > > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160507 08:24]: > > > once omap_dm_timer_start() is called, which calls omap_dm_timer_enable() > > > and thus pm_runtime_get_sync(), it doesn't make sense to call PM runtime > > > functions again before omap_dm_timer_stop is called(). Otherwise PM runtime > > > functions called in omap_dm_timer_enable/disable lead to long and unneeded > > > delays. > > > > > > Fix that by implementing an "enabled" counter, so the PM runtime functions > > > get called only when really needed. > > > > > > Without that patch Nokia N900 IR TX driver (ir-rx51) does not function. > > > > Well, I just tested again, with the $subject patch reverted and > contradictory to my own words, it worked just fine. I believe the reason is > that I did hrtimer "migration" after I did the $subject patch. I was > thinking the reason for the slow transmission was PWM dmtimer, but now it > turns out it has been the "pulse" dmtimer. So, I think the $subject patch > should be dropped. > > > We should use pm_runtime for the refcounting though and call PM runtime > > unconditionally. Can you try to follow the standard PM runtime usage > > like this: > > > > It works without that, but on the other hand, I finally have some reference > on how PM runtime API should be called :). OK. And I just applied the related dts changes. Please repost the driver changes and DT binding doc with Rob's ack to the driver maintainers to apply. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10.06.2016 13:22, Tony Lindgren wrote: > > OK. And I just applied the related dts changes. Please repost the driver > changes and DT binding doc with Rob's ack to the driver maintainers to > apply. > Already did, see https://lkml.org/lkml/2016/5/16/429 Shall I do anything else? Thanks, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160610 14:23]: > > On 10.06.2016 13:22, Tony Lindgren wrote: > > > > OK. And I just applied the related dts changes. Please repost the driver > > changes and DT binding doc with Rob's ack to the driver maintainers to > > apply. > > > > Already did, see https://lkml.org/lkml/2016/5/16/429 > > Shall I do anything else? Probably good idea to repost just the driver changes to the subsystem maintainers. With v4.7 out any pre v4.7 patchsets easily get forgotten. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tony Lindgren <tony@atomide.com> [160613 00:10]: > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160610 14:23]: > > > > On 10.06.2016 13:22, Tony Lindgren wrote: > > > > > > OK. And I just applied the related dts changes. Please repost the driver > > > changes and DT binding doc with Rob's ack to the driver maintainers to > > > apply. > > > > > > > Already did, see https://lkml.org/lkml/2016/5/16/429 > > > > Shall I do anything else? > > Probably good idea to repost just the driver changes to the > subsystem maintainers. With v4.7 out any pre v4.7 patchsets > easily get forgotten. Heh s/v4.7/v4.6/ :) Tony -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 13.06.2016 10:10, Tony Lindgren wrote: > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160610 14:23]: >> >> On 10.06.2016 13:22, Tony Lindgren wrote: >>> >>> OK. And I just applied the related dts changes. Please repost the driver >>> changes and DT binding doc with Rob's ack to the driver maintainers to >>> apply. >>> >> >> Already did, see https://lkml.org/lkml/2016/5/16/429 >> >> Shall I do anything else? > > Probably good idea to repost just the driver changes to the > subsystem maintainers. With v4.7 out any pre v4.7 patchsets > easily get forgotten. > Sorry for the maybe stupid question, but does this mean that I should send separate patches instead of series? Or the series without what you've already applied? Thanks, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160613 12:01]: > Hi, > > On 13.06.2016 10:10, Tony Lindgren wrote: > > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160610 14:23]: > > > > > > On 10.06.2016 13:22, Tony Lindgren wrote: > > > > > > > > OK. And I just applied the related dts changes. Please repost the driver > > > > changes and DT binding doc with Rob's ack to the driver maintainers to > > > > apply. > > > > > > > > > > Already did, see https://lkml.org/lkml/2016/5/16/429 > > > > > > Shall I do anything else? > > > > Probably good idea to repost just the driver changes to the > > subsystem maintainers. With v4.7 out any pre v4.7 patchsets > > easily get forgotten. > > > > Sorry for the maybe stupid question, but does this mean that I should send > separate patches instead of series? Or the series without what you've > already applied? Always leave out the patches that have been already applied. Otherwise people will get confused. Just mention it in the cover letter saying "patch xyz has been already applied into foo tree, these patches are safe to apply separately into bar tree" or something similar :) Tony -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c index 7a327bd..f35a78c 100644 --- a/arch/arm/plat-omap/dmtimer.c +++ b/arch/arm/plat-omap/dmtimer.c @@ -365,6 +365,9 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer) { int c; + if (timer->enabled++) + return; + pm_runtime_get_sync(&timer->pdev->dev); if (!(timer->capability & OMAP_TIMER_ALWON)) { @@ -383,7 +386,11 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_enable); void omap_dm_timer_disable(struct omap_dm_timer *timer) { - pm_runtime_put_sync(&timer->pdev->dev); + if (timer->enabled == 1) + pm_runtime_put_sync(&timer->pdev->dev); + + if (timer->enabled) + timer->enabled--; } EXPORT_SYMBOL_GPL(omap_dm_timer_disable); diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h index dd79f30..fc984e1 100644 --- a/arch/arm/plat-omap/include/plat/dmtimer.h +++ b/arch/arm/plat-omap/include/plat/dmtimer.h @@ -114,6 +114,7 @@ struct omap_dm_timer { unsigned long rate; unsigned reserved:1; unsigned posted:1; + u32 enabled; struct timer_regs context; int (*get_context_loss_count)(struct device *); int ctx_loss_count;