Re: [PATCH 1/3] si2157: change command for sleep
Hi Antti, Em Thu, 18 Sep 2014 15:00:59 +0300 Antti Palosaari escreveu: > On 09/18/2014 02:50 PM, Mauro Carvalho Chehab wrote: > > Sorry, I misunderstood this patch. > > > > There are actually two different things, each requiring a different PM > > setting: > > > > 1) to put the tuner to sleep while it is not in usage; > > > > 2) put the machine to suspend. > > > > This patch is for (1). That's FINE. I'll apply it. > > > > Yet, for (2), assuming a suspend to ram, the best is to save more > > power. > > > > In the past, the DVB core didn't make any distinction between those > > two, but we recently added a hook for suspend there. > > > > So, it makes sense to keep the tuner powerdown mode for suspend. > > > I think old deep sleep should be used for suspend, whilst on warm state > that new is OK. Agreed. > I2C driver has suspend/resume support and it is one thing I am going to > study in near future. The problem with an I2C core driven suspend/resume call is that the suspend/resume order will be random. This will likely break support on complex devices where resuming require a certain order to happen, especially if the device is streaming. > Currently I have 128GB SSD and no swap at all, so > I cannot test it now. I have already bought 256GB SSD and I am just > waiting for Fedora 21 alpha version, which I will install with proper > swap. Unfortunately they have delayed many times already Fedora 21 alpha... Fedora 21 alpha will likely be too experimental for my taste ;) The changes on Fedora next means that too much needs to be touched, so, I'll likely wait for a few months after Fedora 21 release to migrate most of my machines. > I am not even sure if DVB frontend needs any special suspend/resume > handling, isn't it possible to use standard kernel PM here? (I already > added some initial PM to DVB FE ~2 years ago, but IIRC those will only > kill threads and so which are not allowed when suspend). Well, dvb uses the standard Kernel PM. The way PM works on most places is that the subsystem provides their own PM methods that are called by the PM subsystem. > So is there some reason DVB FE needs suspend/resume hooks? Yes. See above. Basically, at suspend, the tuner and demod should be stopped (including all kthreads), and they need to be restarted and firmware may need to be reloaded at resume time. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] si2157: change command for sleep
On 09/18/2014 02:50 PM, Mauro Carvalho Chehab wrote: Sorry, I misunderstood this patch. There are actually two different things, each requiring a different PM setting: 1) to put the tuner to sleep while it is not in usage; 2) put the machine to suspend. This patch is for (1). That's FINE. I'll apply it. Yet, for (2), assuming a suspend to ram, the best is to save more power. In the past, the DVB core didn't make any distinction between those two, but we recently added a hook for suspend there. So, it makes sense to keep the tuner powerdown mode for suspend. I think old deep sleep should be used for suspend, whilst on warm state that new is OK. I2C driver has suspend/resume support and it is one thing I am going to study in near future. Currently I have 128GB SSD and no swap at all, so I cannot test it now. I have already bought 256GB SSD and I am just waiting for Fedora 21 alpha version, which I will install with proper swap. Unfortunately they have delayed many times already Fedora 21 alpha... I am not even sure if DVB frontend needs any special suspend/resume handling, isn't it possible to use standard kernel PM here? (I already added some initial PM to DVB FE ~2 years ago, but IIRC those will only kill threads and so which are not allowed when suspend). So is there some reason DVB FE needs suspend/resume hooks? regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] si2157: change command for sleep
Em Thu, 18 Sep 2014 08:22:33 -0300 Mauro Carvalho Chehab escreveu: > Em Fri, 05 Sep 2014 11:34:01 +0300 > Antti Palosaari escreveu: > > > Moikka Olli > > > > I ran some PM tests for that patch set, using PCTV 292e, which is em28xx > > + Si2168 B40 + Si2157 A30. Here is results: > > > > current impementation > > - > > cold plugin 40 mA > > streaming 235 mA > > sleeping42 mA > > > > si2157: change command for sleep > > - > > cold plugin 40 mA > > streaming 235 mA > > sleeping60 mA > > > > So it increases sleep power usage surprisingly much, almost 20mA from > > the USB, nominal 5V. > > > > It is also funny that you will not lose firmware for Si2168 when sleep > > with command 13, but that Si2157 tuner behaves differently. > > > > I think I will still apply that, it is just firmware download time vs. > > current use in sleep. > > IMHO, the best is to keep it saving more power. Ok, it will take more > time to wake up but so what? If someone is putting the machine to sleep, > it is because he/she wants to save power. > > So, IMHO, we should keep the default behavior as-is. Nothing prevents > that we would add a modprobe parameter or to use some other method that > would allow the user to choose between those two different ways. Sorry, I misunderstood this patch. There are actually two different things, each requiring a different PM setting: 1) to put the tuner to sleep while it is not in usage; 2) put the machine to suspend. This patch is for (1). That's FINE. I'll apply it. Yet, for (2), assuming a suspend to ram, the best is to save more power. In the past, the DVB core didn't make any distinction between those two, but we recently added a hook for suspend there. So, it makes sense to keep the tuner powerdown mode for suspend. Regards, Mauro. > > Regards, > Mauro > > > > > Antti > > > > > > On 08/25/2014 09:07 PM, Olli Salonen wrote: > > > Instead of sending command 13 to the tuner, send command 16 when > > > sleeping. This > > > behaviour is observed when using manufacturer provided binary-only Linux > > > driver > > > for TechnoTrend CT2-4400 (Windows driver does not do power management). > > > > > > The issue with command 13 is that firmware loading is necessary after > > > that. > > > This is not an issue with tuners that do not require firmware, but > > > resuming > > > from sleep on an Si2158 takes noticeable time as firmware is loaded on > > > resume. > > > > > > Signed-off-by: Olli Salonen > > > --- > > > drivers/media/tuners/si2157.c | 7 --- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c > > > index efb5cce..c84f7b8 100644 > > > --- a/drivers/media/tuners/si2157.c > > > +++ b/drivers/media/tuners/si2157.c > > > @@ -197,9 +197,10 @@ static int si2157_sleep(struct dvb_frontend *fe) > > > > > > s->active = false; > > > > > > - memcpy(cmd.args, "\x13", 1); > > > - cmd.wlen = 1; > > > - cmd.rlen = 0; > > > + /* standby */ > > > + memcpy(cmd.args, "\x16\x00", 2); > > > + cmd.wlen = 2; > > > + cmd.rlen = 1; > > > ret = si2157_cmd_execute(s, &cmd); > > > if (ret) > > > goto err; > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] si2157: change command for sleep
Em Fri, 05 Sep 2014 11:34:01 +0300 Antti Palosaari escreveu: > Moikka Olli > > I ran some PM tests for that patch set, using PCTV 292e, which is em28xx > + Si2168 B40 + Si2157 A30. Here is results: > > current impementation > - > cold plugin 40 mA > streaming 235 mA > sleeping42 mA > > si2157: change command for sleep > - > cold plugin 40 mA > streaming 235 mA > sleeping60 mA > > So it increases sleep power usage surprisingly much, almost 20mA from > the USB, nominal 5V. > > It is also funny that you will not lose firmware for Si2168 when sleep > with command 13, but that Si2157 tuner behaves differently. > > I think I will still apply that, it is just firmware download time vs. > current use in sleep. IMHO, the best is to keep it saving more power. Ok, it will take more time to wake up but so what? If someone is putting the machine to sleep, it is because he/she wants to save power. So, IMHO, we should keep the default behavior as-is. Nothing prevents that we would add a modprobe parameter or to use some other method that would allow the user to choose between those two different ways. Regards, Mauro > > Antti > > > On 08/25/2014 09:07 PM, Olli Salonen wrote: > > Instead of sending command 13 to the tuner, send command 16 when sleeping. > > This > > behaviour is observed when using manufacturer provided binary-only Linux > > driver > > for TechnoTrend CT2-4400 (Windows driver does not do power management). > > > > The issue with command 13 is that firmware loading is necessary after that. > > This is not an issue with tuners that do not require firmware, but resuming > > from sleep on an Si2158 takes noticeable time as firmware is loaded on > > resume. > > > > Signed-off-by: Olli Salonen > > --- > > drivers/media/tuners/si2157.c | 7 --- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c > > index efb5cce..c84f7b8 100644 > > --- a/drivers/media/tuners/si2157.c > > +++ b/drivers/media/tuners/si2157.c > > @@ -197,9 +197,10 @@ static int si2157_sleep(struct dvb_frontend *fe) > > > > s->active = false; > > > > - memcpy(cmd.args, "\x13", 1); > > - cmd.wlen = 1; > > - cmd.rlen = 0; > > + /* standby */ > > + memcpy(cmd.args, "\x16\x00", 2); > > + cmd.wlen = 2; > > + cmd.rlen = 1; > > ret = si2157_cmd_execute(s, &cmd); > > if (ret) > > goto err; > > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] si2157: change command for sleep
Moikka Olli I ran some PM tests for that patch set, using PCTV 292e, which is em28xx + Si2168 B40 + Si2157 A30. Here is results: current impementation - cold plugin 40 mA streaming 235 mA sleeping42 mA si2157: change command for sleep - cold plugin 40 mA streaming 235 mA sleeping60 mA So it increases sleep power usage surprisingly much, almost 20mA from the USB, nominal 5V. It is also funny that you will not lose firmware for Si2168 when sleep with command 13, but that Si2157 tuner behaves differently. I think I will still apply that, it is just firmware download time vs. current use in sleep. Antti On 08/25/2014 09:07 PM, Olli Salonen wrote: Instead of sending command 13 to the tuner, send command 16 when sleeping. This behaviour is observed when using manufacturer provided binary-only Linux driver for TechnoTrend CT2-4400 (Windows driver does not do power management). The issue with command 13 is that firmware loading is necessary after that. This is not an issue with tuners that do not require firmware, but resuming from sleep on an Si2158 takes noticeable time as firmware is loaded on resume. Signed-off-by: Olli Salonen --- drivers/media/tuners/si2157.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index efb5cce..c84f7b8 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -197,9 +197,10 @@ static int si2157_sleep(struct dvb_frontend *fe) s->active = false; - memcpy(cmd.args, "\x13", 1); - cmd.wlen = 1; - cmd.rlen = 0; + /* standby */ + memcpy(cmd.args, "\x16\x00", 2); + cmd.wlen = 2; + cmd.rlen = 1; ret = si2157_cmd_execute(s, &cmd); if (ret) goto err; -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] si2157: change command for sleep
Instead of sending command 13 to the tuner, send command 16 when sleeping. This behaviour is observed when using manufacturer provided binary-only Linux driver for TechnoTrend CT2-4400 (Windows driver does not do power management). The issue with command 13 is that firmware loading is necessary after that. This is not an issue with tuners that do not require firmware, but resuming from sleep on an Si2158 takes noticeable time as firmware is loaded on resume. Signed-off-by: Olli Salonen --- drivers/media/tuners/si2157.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index efb5cce..c84f7b8 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -197,9 +197,10 @@ static int si2157_sleep(struct dvb_frontend *fe) s->active = false; - memcpy(cmd.args, "\x13", 1); - cmd.wlen = 1; - cmd.rlen = 0; + /* standby */ + memcpy(cmd.args, "\x16\x00", 2); + cmd.wlen = 2; + cmd.rlen = 1; ret = si2157_cmd_execute(s, &cmd); if (ret) goto err; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html