From: Sean Young <sean@mess.org> To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: "Maíra Canal" <maira.canal@usp.br>, lkp@intel.com, mchehab@kernel.org, thierry.reding@gmail.com, lee.jones@linaro.org, llvm@lists.linux.dev, kbuild-all@lists.01.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org Subject: Re: [PATCH v4] media: rc: pwm-ir-tx: Switch to atomic PWM API Date: Fri, 29 Oct 2021 08:16:08 +0100 [thread overview] Message-ID: <20211029071608.GA28997@gofer.mess.org> (raw) In-Reply-To: <20211028180516.t2tpfbzztm7s6cqm@pengutronix.de> On Thu, Oct 28, 2021 at 08:05:16PM +0200, Uwe Kleine-König wrote: > On Thu, Oct 28, 2021 at 01:26:10PM +0100, Sean Young wrote: > > > bloat-o-meter reports (for an arm allmodconfig build) > > > > > > add/remove: 0/0 grow/shrink: 3/1 up/down: 644/-396 (248) > > > Function old new delta > > > pwm_ir_probe 372 676 +304 > > > pwm_ir_set_carrier 108 292 +184 > > > pwm_ir_set_duty_cycle 68 224 +156 > > > pwm_ir_tx 908 512 -396 > > > Total: Before=2302, After=2550, chg +10.77% > > > > So 248 bytes more after your changes. > > ack. This is because the compiler inlines the division which accounts > for > 100 bytes. I'm surprised it's that large. This is on 32 bit? > > > struct pwm_ir increases from 12 bytes to 40 bytes. > > > > > > The stack space required by pwm_ir_tx decreases from 60 to 36 > > > > > > I don't know exactly how kmalloc works internally. Maybe allocating a > > > structure of size 40 bytes doesn't need more memory than a structure of > > > size 12? > > > > > > I didn't check how runtimes change, but the size decrease of pwm_ir_tx() > > > is nice and might save a bit of runtime. > > > > I'm not following, how is this decreasing runtime? > > With my changes pwm_ir_tx got smaller and { pwm_ir_probe, > pwm_ir_set_carrier, pwm_ir_set_duty_cycle } got bigger. Now if for a > typical runtime pattern pwm_ir_probe and pwm_ir_set_carrier run once and > pwm_ir_set_duty_cycle 100 times and pwm_ir_tx 1000 times (no idea if > that is realistic) it might be a net win in sum. The two most common programs for sending IR are ir-ctl: https://git.linuxtv.org/v4l-utils.git/tree/utils/ir-ctl/ir-ctl.c#n1041 lircd: https://sourceforge.net/p/lirc/git/ci/master/tree/lib/transmit.c For each transmission, the carrier is set. If the duty cyle is specified, then that is set too. Then the transmit itself is done. Both of them set the carrier and duty cycle (if required) for every transmission: setting the carrier and duty cycle is a cheap operation, and it is device property which can be overriden by another process. This means with your changes, if the carrier and duty cycle are both set for each transmission, then we're doing more work. If only the carrier is set for each transmission, then there is no net gain/loss (I think), but the code size has increased. Thanks for prototyping this. Sean
WARNING: multiple messages have this Message-ID (diff)
From: Sean Young <sean@mess.org> To: kbuild-all@lists.01.org Subject: Re: [PATCH v4] media: rc: pwm-ir-tx: Switch to atomic PWM API Date: Fri, 29 Oct 2021 08:16:08 +0100 [thread overview] Message-ID: <20211029071608.GA28997@gofer.mess.org> (raw) In-Reply-To: <20211028180516.t2tpfbzztm7s6cqm@pengutronix.de> [-- Attachment #1: Type: text/plain, Size: 2581 bytes --] On Thu, Oct 28, 2021 at 08:05:16PM +0200, Uwe Kleine-König wrote: > On Thu, Oct 28, 2021 at 01:26:10PM +0100, Sean Young wrote: > > > bloat-o-meter reports (for an arm allmodconfig build) > > > > > > add/remove: 0/0 grow/shrink: 3/1 up/down: 644/-396 (248) > > > Function old new delta > > > pwm_ir_probe 372 676 +304 > > > pwm_ir_set_carrier 108 292 +184 > > > pwm_ir_set_duty_cycle 68 224 +156 > > > pwm_ir_tx 908 512 -396 > > > Total: Before=2302, After=2550, chg +10.77% > > > > So 248 bytes more after your changes. > > ack. This is because the compiler inlines the division which accounts > for > 100 bytes. I'm surprised it's that large. This is on 32 bit? > > > struct pwm_ir increases from 12 bytes to 40 bytes. > > > > > > The stack space required by pwm_ir_tx decreases from 60 to 36 > > > > > > I don't know exactly how kmalloc works internally. Maybe allocating a > > > structure of size 40 bytes doesn't need more memory than a structure of > > > size 12? > > > > > > I didn't check how runtimes change, but the size decrease of pwm_ir_tx() > > > is nice and might save a bit of runtime. > > > > I'm not following, how is this decreasing runtime? > > With my changes pwm_ir_tx got smaller and { pwm_ir_probe, > pwm_ir_set_carrier, pwm_ir_set_duty_cycle } got bigger. Now if for a > typical runtime pattern pwm_ir_probe and pwm_ir_set_carrier run once and > pwm_ir_set_duty_cycle 100 times and pwm_ir_tx 1000 times (no idea if > that is realistic) it might be a net win in sum. The two most common programs for sending IR are ir-ctl: https://git.linuxtv.org/v4l-utils.git/tree/utils/ir-ctl/ir-ctl.c#n1041 lircd: https://sourceforge.net/p/lirc/git/ci/master/tree/lib/transmit.c For each transmission, the carrier is set. If the duty cyle is specified, then that is set too. Then the transmit itself is done. Both of them set the carrier and duty cycle (if required) for every transmission: setting the carrier and duty cycle is a cheap operation, and it is device property which can be overriden by another process. This means with your changes, if the carrier and duty cycle are both set for each transmission, then we're doing more work. If only the carrier is set for each transmission, then there is no net gain/loss (I think), but the code size has increased. Thanks for prototyping this. Sean
next prev parent reply other threads:[~2021-10-29 7:16 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-27 15:34 [PATCH v4] media: rc: pwm-ir-tx: Switch to atomic PWM API Maíra Canal 2021-10-27 15:34 ` Maíra Canal 2021-10-28 6:45 ` Uwe Kleine-König 2021-10-28 6:45 ` Uwe Kleine-König 2021-10-28 9:14 ` Sean Young 2021-10-28 9:14 ` Sean Young 2021-10-28 11:15 ` Uwe Kleine-König 2021-10-28 11:15 ` Uwe Kleine-König 2021-10-28 12:26 ` Sean Young 2021-10-28 12:26 ` Sean Young 2021-10-28 18:05 ` Uwe Kleine-König 2021-10-28 18:05 ` Uwe Kleine-König 2021-10-29 7:16 ` Sean Young [this message] 2021-10-29 7:16 ` Sean Young 2021-10-29 11:06 ` Uwe Kleine-König 2021-10-29 11:06 ` Uwe Kleine-König 2021-10-29 11:54 ` Sean Young 2021-10-29 11:54 ` Sean Young 2021-10-29 12:08 ` Maíra Canal 2021-10-29 12:08 ` Maíra Canal 2021-10-29 15:18 ` Uwe Kleine-König 2021-10-29 15:18 ` Uwe Kleine-König 2021-10-30 9:21 ` Sean Young 2021-10-30 9:21 ` Sean Young 2021-10-31 10:39 ` Sean Young 2021-10-31 17:40 ` Uwe Kleine-König
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211029071608.GA28997@gofer.mess.org \ --to=sean@mess.org \ --cc=kbuild-all@lists.01.org \ --cc=lee.jones@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-pwm@vger.kernel.org \ --cc=lkp@intel.com \ --cc=llvm@lists.linux.dev \ --cc=maira.canal@usp.br \ --cc=mchehab@kernel.org \ --cc=thierry.reding@gmail.com \ --cc=u.kleine-koenig@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.