Performance device PM QoS (was: Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support)

2012-09-18 Thread Rafael J. Wysocki
On Tuesday, September 18, 2012, mark gross wrote:
> On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
> > On Monday, September 17, 2012, MyungJoo Ham wrote:

[...]

> > > > What QoS types do you think could be used here?
> > > 
> > > I don't think the devfreq core needs to care of it.
> > > Whatever the device driver wants could be available here.
> > 
> > That's a bit of a problem.  I don't want device drivers to use the global
> > QoS types, because they aren't sufficiently well defined (units are kind of
> > unknown in some cases, for example).
> > 
> > In my opinion it would be better to add performance device PM QoS in analogy
> > with the existing latency device PM QoS.  The unit might be percentage of 
> > full
> > performance (0 - 100).
> > 
> > Of course, that only would cover device performance, but there also is the
> > problem of interconnect throughput that may depend on what frequencies are
> > used by devices on it.
> >
> 
> Paul W. and I where talking about a boost interface I think may be worth
> considering.
> 
> We need to take a step back at this point.  What type of algebra is
> needed WRT the device pm_qos analogy?  do we need an ordered set or even
> partial ordering?  Do we need to be able to tell one state is more
> performing than another at all?
> 
> The use cases of these performance levels tend to be platform and device
> specific AFAICT so far.  The units of performance are not portable
> across ISA's and they're interpretation varies from device to device.

However, the approach used in the patchset being discussed in this thread
addresses this problem by mapping different "QoS" values to specific
device operating points with the help of an array.  Of course, the array
has to be provided by either the platform or the driver, but it allows
us to use "portable" QoS values in the core, at least in principle.

> What if we didn't think of it in terms of an ordered field of some sort?
> What if we had a per device boost hash who's meaning is defined by the
> board / device level module but, the interface and use is defined in the
> common code?  If the platform code didn't implement any then those are
> NOOPs if the platform code cares about that device qos then it
> implements and interprets the specific boost qos as needed.
> 
> So in practice when a driver or use case needed qos, it would request a
> qos hash from the platform code to use and that platform code would need
> to interpret that hash in a platform specific way.

That seems to be conceptually similar to what dev_pm_qos_expose_latency_limit()
does.  It essentially allows drivers to request that PM QoS latency constraints
be used for the devices they handle.

> This would remove the portability problem from drivers requested QoS
> levels from assorted devices.  the QoS levels will be a hash, to be
> interpreted by platform code.

I personally think that the QoS levels should be specified in a way allowing
user space to interpret them, so that they can be exposed to the actual user
in a comprehensible manner accross different systems (be it Android or a
random Linux distro).  There may be a mapping between this "portable"
representation and the platform-specific one (or even driver-specific one
if need be), but user space should be able to say what the particular setting
actually means.

> there are a lot of details to work out but I think something could be
> done along these lines.

Possibly, yes.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Performance device PM QoS (was: Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support)

2012-09-18 Thread Rafael J. Wysocki
On Tuesday, September 18, 2012, mark gross wrote:
 On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
  On Monday, September 17, 2012, MyungJoo Ham wrote:

[...]

What QoS types do you think could be used here?
   
   I don't think the devfreq core needs to care of it.
   Whatever the device driver wants could be available here.
  
  That's a bit of a problem.  I don't want device drivers to use the global
  QoS types, because they aren't sufficiently well defined (units are kind of
  unknown in some cases, for example).
  
  In my opinion it would be better to add performance device PM QoS in analogy
  with the existing latency device PM QoS.  The unit might be percentage of 
  full
  performance (0 - 100).
  
  Of course, that only would cover device performance, but there also is the
  problem of interconnect throughput that may depend on what frequencies are
  used by devices on it.
 
 
 Paul W. and I where talking about a boost interface I think may be worth
 considering.
 
 We need to take a step back at this point.  What type of algebra is
 needed WRT the device pm_qos analogy?  do we need an ordered set or even
 partial ordering?  Do we need to be able to tell one state is more
 performing than another at all?
 
 The use cases of these performance levels tend to be platform and device
 specific AFAICT so far.  The units of performance are not portable
 across ISA's and they're interpretation varies from device to device.

However, the approach used in the patchset being discussed in this thread
addresses this problem by mapping different QoS values to specific
device operating points with the help of an array.  Of course, the array
has to be provided by either the platform or the driver, but it allows
us to use portable QoS values in the core, at least in principle.

 What if we didn't think of it in terms of an ordered field of some sort?
 What if we had a per device boost hash who's meaning is defined by the
 board / device level module but, the interface and use is defined in the
 common code?  If the platform code didn't implement any then those are
 NOOPs if the platform code cares about that device qos then it
 implements and interprets the specific boost qos as needed.
 
 So in practice when a driver or use case needed qos, it would request a
 qos hash from the platform code to use and that platform code would need
 to interpret that hash in a platform specific way.

That seems to be conceptually similar to what dev_pm_qos_expose_latency_limit()
does.  It essentially allows drivers to request that PM QoS latency constraints
be used for the devices they handle.

 This would remove the portability problem from drivers requested QoS
 levels from assorted devices.  the QoS levels will be a hash, to be
 interpreted by platform code.

I personally think that the QoS levels should be specified in a way allowing
user space to interpret them, so that they can be exposed to the actual user
in a comprehensible manner accross different systems (be it Android or a
random Linux distro).  There may be a mapping between this portable
representation and the platform-specific one (or even driver-specific one
if need be), but user space should be able to say what the particular setting
actually means.

 there are a lot of details to work out but I think something could be
 done along these lines.

Possibly, yes.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread mark gross
On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, MyungJoo Ham wrote:
> > Sender : Rafael J. Wysocki
> > Date : 2012-09-09 07:20 (GMT+09:00)
> > Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
> > > On Thursday, August 30, 2012, MyungJoo Ham wrote:
> > > > Even if the performance of a device is controlled properly with devfreq,
> > > > sometimes, we still need to get PM-QoS inputs in order to meet the
> > > > required performance.
> > > > 
> > > > In our testbed of Exynos4412, which has on-chip various DMA devices, the
> > > > memory interface and system bus are controlled according to their
> > > > utilization by devfreq. However, in some multimedia applications
> > > > including video-playing with MFC (multi-function codec) H/W and
> > > > photo/video-capturing with FIMC H/W, we have observed issues due to
> > > > insufficient DMA throughput or latency.
> > > > 
> > > > In such applications, there are deadlines: less than 16.6ms with 60Hz.
> > > > With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
> > > > within a frame and we get missing frames and distorted pictures.
> > > > With longer polling intervals (20 ~ 100ms), the response time is not
> > > > sufficient and we get distorted or broken images. In other words,
> > > > regardless of polling interval, we get poor results with hard-deadline
> > > > H/Ws. They, in fact, have a preset requirement on DMA throughput.
> > > > 
> > > > Thus, we need PM-QoS capabilities in devfreq. (Note that for general
> > > > user applications, devfreq for bus/memory works fine. They are not so
> > > > sensitive to tens of ms in performance increasing responses in general.
> > > > 
> > > > In order to express how to handle QoS requests in devfreq devices,
> > > > the devfreq device drivers only need to express the mappings of
> > > > QoS value and frequency pairs with QoS class along with
> > > > devfreq_add_device() call.
> > > > 
> > > > As a side effect of the implementation, which happens to be positive,
> > > > min/max freq is now enforced regardless of governor implementation.
> > > 
> > > Can you please explain in a few words how this is supposed to work in
> > > practice?
> > 
> > Ah.. this "side effect" has been neutralized by the patch
> > 
> > ab5f299f51259fd2466cf35c89d79bd960e0fc32
> > PM / devfreq: add relation of recommended frequency.
> > 
> > I should've removed that comment.
> 
> OK
> 
> > > > Tested on Exynos4412 machines with memory/bus frequencies and multimedia
> > > > H/W blocks. (camera, video decoding, and video encoding)
> > > > 
> > > > Signed-off-by: MyungJoo Ham 
> > > > Signed-off-by: Kyungmin Park 
> > > 
> > > I'm not entirely convinced yet, but a few comments follow.
> > > 
> > > > ---
> > > > Changed from V2-resend
> > > > - Removed dependencies on global pm-qos class definitions
> > > > - Revised data structure handling pm-qos (being ready for dev-pm-qos)
> > > > 
> > > > Changes from V2
> > > > - Rebased
> > > > 
> > > > Changes from V1
> > > > - Error handling at devfreq_add_device()
> > > > - Handling pm_qos_max information
> > > > - Styly update
> > > > ---
> > []
> > > > @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
> > > >   * List from the highest proiority
> > > >   * max_freq (probably called by thermal when it's too hot)
> > > >   * min_freq
> > > > + * qos_min_freq
> > > >   */
> > > >  
> > > > + if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
> > > > + freq = devfreq->qos_min_freq;
> > > > + flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > > 
> > > What exactly is the purpose of this last line?
> > 
> > It says target callback to use "qos_min_freq" as its greatest lower bound;
> > use any values that are "qos_min_freq" or above.
> > And it can be overriden by min_freq and max_freq.
> 
> I see.
> 
> > > > + }
> > > >   if (devfreq->min_freq && freq < devfreq->min_freq) {
> > > >   freq = devfreq->min_freq;

Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread Rafael J. Wysocki
On Monday, September 17, 2012, MyungJoo Ham wrote:
> Sender : Rafael J. Wysocki
> Date : 2012-09-09 07:20 (GMT+09:00)
> Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
> > On Thursday, August 30, 2012, MyungJoo Ham wrote:
> > > Even if the performance of a device is controlled properly with devfreq,
> > > sometimes, we still need to get PM-QoS inputs in order to meet the
> > > required performance.
> > > 
> > > In our testbed of Exynos4412, which has on-chip various DMA devices, the
> > > memory interface and system bus are controlled according to their
> > > utilization by devfreq. However, in some multimedia applications
> > > including video-playing with MFC (multi-function codec) H/W and
> > > photo/video-capturing with FIMC H/W, we have observed issues due to
> > > insufficient DMA throughput or latency.
> > > 
> > > In such applications, there are deadlines: less than 16.6ms with 60Hz.
> > > With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
> > > within a frame and we get missing frames and distorted pictures.
> > > With longer polling intervals (20 ~ 100ms), the response time is not
> > > sufficient and we get distorted or broken images. In other words,
> > > regardless of polling interval, we get poor results with hard-deadline
> > > H/Ws. They, in fact, have a preset requirement on DMA throughput.
> > > 
> > > Thus, we need PM-QoS capabilities in devfreq. (Note that for general
> > > user applications, devfreq for bus/memory works fine. They are not so
> > > sensitive to tens of ms in performance increasing responses in general.
> > > 
> > > In order to express how to handle QoS requests in devfreq devices,
> > > the devfreq device drivers only need to express the mappings of
> > > QoS value and frequency pairs with QoS class along with
> > > devfreq_add_device() call.
> > > 
> > > As a side effect of the implementation, which happens to be positive,
> > > min/max freq is now enforced regardless of governor implementation.
> > 
> > Can you please explain in a few words how this is supposed to work in
> > practice?
> 
> Ah.. this "side effect" has been neutralized by the patch
> 
> ab5f299f51259fd2466cf35c89d79bd960e0fc32
> PM / devfreq: add relation of recommended frequency.
> 
> I should've removed that comment.

OK

> > > Tested on Exynos4412 machines with memory/bus frequencies and multimedia
> > > H/W blocks. (camera, video decoding, and video encoding)
> > > 
> > > Signed-off-by: MyungJoo Ham 
> > > Signed-off-by: Kyungmin Park 
> > 
> > I'm not entirely convinced yet, but a few comments follow.
> > 
> > > ---
> > > Changed from V2-resend
> > > - Removed dependencies on global pm-qos class definitions
> > > - Revised data structure handling pm-qos (being ready for dev-pm-qos)
> > > 
> > > Changes from V2
> > > - Rebased
> > > 
> > > Changes from V1
> > > - Error handling at devfreq_add_device()
> > > - Handling pm_qos_max information
> > > - Styly update
> > > ---
> []
> > > @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
> > >   * List from the highest proiority
> > >   * max_freq (probably called by thermal when it's too hot)
> > >   * min_freq
> > > + * qos_min_freq
> > >   */
> > >  
> > > + if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
> > > + freq = devfreq->qos_min_freq;
> > > + flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > 
> > What exactly is the purpose of this last line?
> 
> It says target callback to use "qos_min_freq" as its greatest lower bound;
> use any values that are "qos_min_freq" or above.
> And it can be overriden by min_freq and max_freq.

I see.

> > > + }
> > >   if (devfreq->min_freq && freq < devfreq->min_freq) {
> > >   freq = devfreq->min_freq;
> > >   flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > > @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
> > >   * devfreq_notifier_call() - Notify that the device frequency 
> > > requirements
> > >   *has been changed out of devfreq framework.
> > >   * @nb the notifier_block (supposed to be devfreq->nb)
> > > - * @type not used
> > > + * @val not used.
> > >   * @devp not used
> > >   *

Re: Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread MyungJoo Ham
Sender : Rafael J. Wysocki
Date : 2012-09-09 07:20 (GMT+09:00)
Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
> On Thursday, August 30, 2012, MyungJoo Ham wrote:
> > Even if the performance of a device is controlled properly with devfreq,
> > sometimes, we still need to get PM-QoS inputs in order to meet the
> > required performance.
> > 
> > In our testbed of Exynos4412, which has on-chip various DMA devices, the
> > memory interface and system bus are controlled according to their
> > utilization by devfreq. However, in some multimedia applications
> > including video-playing with MFC (multi-function codec) H/W and
> > photo/video-capturing with FIMC H/W, we have observed issues due to
> > insufficient DMA throughput or latency.
> > 
> > In such applications, there are deadlines: less than 16.6ms with 60Hz.
> > With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
> > within a frame and we get missing frames and distorted pictures.
> > With longer polling intervals (20 ~ 100ms), the response time is not
> > sufficient and we get distorted or broken images. In other words,
> > regardless of polling interval, we get poor results with hard-deadline
> > H/Ws. They, in fact, have a preset requirement on DMA throughput.
> > 
> > Thus, we need PM-QoS capabilities in devfreq. (Note that for general
> > user applications, devfreq for bus/memory works fine. They are not so
> > sensitive to tens of ms in performance increasing responses in general.
> > 
> > In order to express how to handle QoS requests in devfreq devices,
> > the devfreq device drivers only need to express the mappings of
> > QoS value and frequency pairs with QoS class along with
> > devfreq_add_device() call.
> > 
> > As a side effect of the implementation, which happens to be positive,
> > min/max freq is now enforced regardless of governor implementation.
> 
> Can you please explain in a few words how this is supposed to work in
> practice?

Ah.. this "side effect" has been neutralized by the patch

ab5f299f51259fd2466cf35c89d79bd960e0fc32
PM / devfreq: add relation of recommended frequency.

I should've removed that comment.

> 
> > Tested on Exynos4412 machines with memory/bus frequencies and multimedia
> > H/W blocks. (camera, video decoding, and video encoding)
> > 
> > Signed-off-by: MyungJoo Ham 
> > Signed-off-by: Kyungmin Park 
> 
> I'm not entirely convinced yet, but a few comments follow.
> 
> > ---
> > Changed from V2-resend
> > - Removed dependencies on global pm-qos class definitions
> > - Revised data structure handling pm-qos (being ready for dev-pm-qos)
> > 
> > Changes from V2
> > - Rebased
> > 
> > Changes from V1
> > - Error handling at devfreq_add_device()
> > - Handling pm_qos_max information
> > - Styly update
> > ---
[]
> > @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
> >   * List from the highest proiority
> >   * max_freq (probably called by thermal when it's too hot)
> >   * min_freq
> > + * qos_min_freq
> >   */
> >  
> > + if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
> > + freq = devfreq->qos_min_freq;
> > + flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> 
> What exactly is the purpose of this last line?

It says target callback to use "qos_min_freq" as its greatest lower bound;
use any values that are "qos_min_freq" or above.
And it can be overriden by min_freq and max_freq.

> 
> > + }
> >   if (devfreq->min_freq && freq < devfreq->min_freq) {
> >   freq = devfreq->min_freq;
> >   flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
> >   * devfreq_notifier_call() - Notify that the device frequency requirements
> >   *has been changed out of devfreq framework.
> >   * @nb the notifier_block (supposed to be devfreq->nb)
> > - * @type not used
> > + * @val not used.
> >   * @devp not used
> >   *
> >   * Called by a notifier that uses devfreq->nb.
> >   */
> > -static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
> > type,
> > +static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
> > val,
> >   void *devp)
> >  {
> >   struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> 
> The above change is only a cleanup unrelated to the rest of modifications.
> Please push it separately (if you _really_ think it'

Re: Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread MyungJoo Ham
Sender : Rafael J. Wysockir...@sisk.pl
Date : 2012-09-09 07:20 (GMT+09:00)
Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
 On Thursday, August 30, 2012, MyungJoo Ham wrote:
  Even if the performance of a device is controlled properly with devfreq,
  sometimes, we still need to get PM-QoS inputs in order to meet the
  required performance.
  
  In our testbed of Exynos4412, which has on-chip various DMA devices, the
  memory interface and system bus are controlled according to their
  utilization by devfreq. However, in some multimedia applications
  including video-playing with MFC (multi-function codec) H/W and
  photo/video-capturing with FIMC H/W, we have observed issues due to
  insufficient DMA throughput or latency.
  
  In such applications, there are deadlines: less than 16.6ms with 60Hz.
  With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
  within a frame and we get missing frames and distorted pictures.
  With longer polling intervals (20 ~ 100ms), the response time is not
  sufficient and we get distorted or broken images. In other words,
  regardless of polling interval, we get poor results with hard-deadline
  H/Ws. They, in fact, have a preset requirement on DMA throughput.
  
  Thus, we need PM-QoS capabilities in devfreq. (Note that for general
  user applications, devfreq for bus/memory works fine. They are not so
  sensitive to tens of ms in performance increasing responses in general.
  
  In order to express how to handle QoS requests in devfreq devices,
  the devfreq device drivers only need to express the mappings of
  QoS value and frequency pairs with QoS class along with
  devfreq_add_device() call.
  
  As a side effect of the implementation, which happens to be positive,
  min/max freq is now enforced regardless of governor implementation.
 
 Can you please explain in a few words how this is supposed to work in
 practice?

Ah.. this side effect has been neutralized by the patch

ab5f299f51259fd2466cf35c89d79bd960e0fc32
PM / devfreq: add relation of recommended frequency.

I should've removed that comment.

 
  Tested on Exynos4412 machines with memory/bus frequencies and multimedia
  H/W blocks. (camera, video decoding, and video encoding)
  
  Signed-off-by: MyungJoo Ham 
  Signed-off-by: Kyungmin Park 
 
 I'm not entirely convinced yet, but a few comments follow.
 
  ---
  Changed from V2-resend
  - Removed dependencies on global pm-qos class definitions
  - Revised data structure handling pm-qos (being ready for dev-pm-qos)
  
  Changes from V2
  - Rebased
  
  Changes from V1
  - Error handling at devfreq_add_device()
  - Handling pm_qos_max information
  - Styly update
  ---
[]
  @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
* List from the highest proiority
* max_freq (probably called by thermal when it's too hot)
* min_freq
  + * qos_min_freq
*/
   
  + if (devfreq-qos_min_freq  freq  devfreq-qos_min_freq) {
  + freq = devfreq-qos_min_freq;
  + flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
 
 What exactly is the purpose of this last line?

It says target callback to use qos_min_freq as its greatest lower bound;
use any values that are qos_min_freq or above.
And it can be overriden by min_freq and max_freq.

 
  + }
if (devfreq-min_freq  freq  devfreq-min_freq) {
freq = devfreq-min_freq;
flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
  @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
* devfreq_notifier_call() - Notify that the device frequency requirements
*has been changed out of devfreq framework.
* @nb the notifier_block (supposed to be devfreq-nb)
  - * @type not used
  + * @val not used.
* @devp not used
*
* Called by a notifier that uses devfreq-nb.
*/
  -static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
  type,
  +static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
  val,
void *devp)
   {
struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
 
 The above change is only a cleanup unrelated to the rest of modifications.
 Please push it separately (if you _really_ think it's necessary).

oops.. yes..

 
  @@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct notifier_block 
  *nb, unsigned long type,
   }
   
   /**
  + * devfreq_qos_notifier_call() -
 
 This looks like a missing kerneldoc comment?

yes..  I'd either remove the comment or fill it in.

 
  + */
  +static int devfreq_qos_notifier_call(struct notifier_block *nb,
  +  unsigned long value, void *devp)
  +{
  + struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
  + int ret;
  + int i;
  + s32 default_value = PM_QOS_DEFAULT_VALUE;
  + struct devfreq_pm_qos_table *qos_list = devfreq-profile-qos_list;
  + bool qos_use_max = devfreq-profile-qos_use_max;
  +
  + if (!qos_list)
  + return NOTIFY_DONE;
  +
  + mutex_lock(devfreq-lock);
  +
  + if (value == default_value

Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread Rafael J. Wysocki
On Monday, September 17, 2012, MyungJoo Ham wrote:
 Sender : Rafael J. Wysockir...@sisk.pl
 Date : 2012-09-09 07:20 (GMT+09:00)
 Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
  On Thursday, August 30, 2012, MyungJoo Ham wrote:
   Even if the performance of a device is controlled properly with devfreq,
   sometimes, we still need to get PM-QoS inputs in order to meet the
   required performance.
   
   In our testbed of Exynos4412, which has on-chip various DMA devices, the
   memory interface and system bus are controlled according to their
   utilization by devfreq. However, in some multimedia applications
   including video-playing with MFC (multi-function codec) H/W and
   photo/video-capturing with FIMC H/W, we have observed issues due to
   insufficient DMA throughput or latency.
   
   In such applications, there are deadlines: less than 16.6ms with 60Hz.
   With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
   within a frame and we get missing frames and distorted pictures.
   With longer polling intervals (20 ~ 100ms), the response time is not
   sufficient and we get distorted or broken images. In other words,
   regardless of polling interval, we get poor results with hard-deadline
   H/Ws. They, in fact, have a preset requirement on DMA throughput.
   
   Thus, we need PM-QoS capabilities in devfreq. (Note that for general
   user applications, devfreq for bus/memory works fine. They are not so
   sensitive to tens of ms in performance increasing responses in general.
   
   In order to express how to handle QoS requests in devfreq devices,
   the devfreq device drivers only need to express the mappings of
   QoS value and frequency pairs with QoS class along with
   devfreq_add_device() call.
   
   As a side effect of the implementation, which happens to be positive,
   min/max freq is now enforced regardless of governor implementation.
  
  Can you please explain in a few words how this is supposed to work in
  practice?
 
 Ah.. this side effect has been neutralized by the patch
 
 ab5f299f51259fd2466cf35c89d79bd960e0fc32
 PM / devfreq: add relation of recommended frequency.
 
 I should've removed that comment.

OK

   Tested on Exynos4412 machines with memory/bus frequencies and multimedia
   H/W blocks. (camera, video decoding, and video encoding)
   
   Signed-off-by: MyungJoo Ham 
   Signed-off-by: Kyungmin Park 
  
  I'm not entirely convinced yet, but a few comments follow.
  
   ---
   Changed from V2-resend
   - Removed dependencies on global pm-qos class definitions
   - Revised data structure handling pm-qos (being ready for dev-pm-qos)
   
   Changes from V2
   - Rebased
   
   Changes from V1
   - Error handling at devfreq_add_device()
   - Handling pm_qos_max information
   - Styly update
   ---
 []
   @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
 * List from the highest proiority
 * max_freq (probably called by thermal when it's too hot)
 * min_freq
   + * qos_min_freq
 */

   + if (devfreq-qos_min_freq  freq  devfreq-qos_min_freq) {
   + freq = devfreq-qos_min_freq;
   + flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
  
  What exactly is the purpose of this last line?
 
 It says target callback to use qos_min_freq as its greatest lower bound;
 use any values that are qos_min_freq or above.
 And it can be overriden by min_freq and max_freq.

I see.

   + }
 if (devfreq-min_freq  freq  devfreq-min_freq) {
 freq = devfreq-min_freq;
 flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
   @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
 * devfreq_notifier_call() - Notify that the device frequency 
   requirements
 *has been changed out of devfreq framework.
 * @nb the notifier_block (supposed to be devfreq-nb)
   - * @type not used
   + * @val not used.
 * @devp not used
 *
 * Called by a notifier that uses devfreq-nb.
 */
   -static int devfreq_notifier_call(struct notifier_block *nb, unsigned 
   long type,
   +static int devfreq_notifier_call(struct notifier_block *nb, unsigned 
   long val,
 void *devp)
{
 struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
  
  The above change is only a cleanup unrelated to the rest of modifications.
  Please push it separately (if you _really_ think it's necessary).
 
 oops.. yes..
 
  
   @@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct 
   notifier_block *nb, unsigned long type,
}

/**
   + * devfreq_qos_notifier_call() -
  
  This looks like a missing kerneldoc comment?
 
 yes..  I'd either remove the comment or fill it in.

Please add the comment.

   + */
   +static int devfreq_qos_notifier_call(struct notifier_block *nb,
   +  unsigned long value, void *devp)
   +{
   + struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
   + int ret;
   + int i;
   + s32 default_value = PM_QOS_DEFAULT_VALUE;
   + struct

Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-17 Thread mark gross
On Mon, Sep 17, 2012 at 11:10:09PM +0200, Rafael J. Wysocki wrote:
 On Monday, September 17, 2012, MyungJoo Ham wrote:
  Sender : Rafael J. Wysockir...@sisk.pl
  Date : 2012-09-09 07:20 (GMT+09:00)
  Title : Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support
   On Thursday, August 30, 2012, MyungJoo Ham wrote:
Even if the performance of a device is controlled properly with devfreq,
sometimes, we still need to get PM-QoS inputs in order to meet the
required performance.

In our testbed of Exynos4412, which has on-chip various DMA devices, the
memory interface and system bus are controlled according to their
utilization by devfreq. However, in some multimedia applications
including video-playing with MFC (multi-function codec) H/W and
photo/video-capturing with FIMC H/W, we have observed issues due to
insufficient DMA throughput or latency.

In such applications, there are deadlines: less than 16.6ms with 60Hz.
With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
within a frame and we get missing frames and distorted pictures.
With longer polling intervals (20 ~ 100ms), the response time is not
sufficient and we get distorted or broken images. In other words,
regardless of polling interval, we get poor results with hard-deadline
H/Ws. They, in fact, have a preset requirement on DMA throughput.

Thus, we need PM-QoS capabilities in devfreq. (Note that for general
user applications, devfreq for bus/memory works fine. They are not so
sensitive to tens of ms in performance increasing responses in general.

In order to express how to handle QoS requests in devfreq devices,
the devfreq device drivers only need to express the mappings of
QoS value and frequency pairs with QoS class along with
devfreq_add_device() call.

As a side effect of the implementation, which happens to be positive,
min/max freq is now enforced regardless of governor implementation.
   
   Can you please explain in a few words how this is supposed to work in
   practice?
  
  Ah.. this side effect has been neutralized by the patch
  
  ab5f299f51259fd2466cf35c89d79bd960e0fc32
  PM / devfreq: add relation of recommended frequency.
  
  I should've removed that comment.
 
 OK
 
Tested on Exynos4412 machines with memory/bus frequencies and multimedia
H/W blocks. (camera, video decoding, and video encoding)

Signed-off-by: MyungJoo Ham 
Signed-off-by: Kyungmin Park 
   
   I'm not entirely convinced yet, but a few comments follow.
   
---
Changed from V2-resend
- Removed dependencies on global pm-qos class definitions
- Revised data structure handling pm-qos (being ready for dev-pm-qos)

Changes from V2
- Rebased

Changes from V1
- Error handling at devfreq_add_device()
- Handling pm_qos_max information
- Styly update
---
  []
@@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
  * List from the highest proiority
  * max_freq (probably called by thermal when it's too hot)
  * min_freq
+ * qos_min_freq
  */
 
+ if (devfreq-qos_min_freq  freq  devfreq-qos_min_freq) {
+ freq = devfreq-qos_min_freq;
+ flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
   
   What exactly is the purpose of this last line?
  
  It says target callback to use qos_min_freq as its greatest lower bound;
  use any values that are qos_min_freq or above.
  And it can be overriden by min_freq and max_freq.
 
 I see.
 
+ }
  if (devfreq-min_freq  freq  devfreq-min_freq) {
  freq = devfreq-min_freq;
  flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
@@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
  * devfreq_notifier_call() - Notify that the device frequency 
requirements
  *has been changed out of devfreq framework.
  * @nb the notifier_block (supposed to be devfreq-nb)
- * @type not used
+ * @val not used.
  * @devp not used
  *
  * Called by a notifier that uses devfreq-nb.
  */
-static int devfreq_notifier_call(struct notifier_block *nb, unsigned 
long type,
+static int devfreq_notifier_call(struct notifier_block *nb, unsigned 
long val,
  void *devp)
 {
  struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
   
   The above change is only a cleanup unrelated to the rest of modifications.
   Please push it separately (if you _really_ think it's necessary).
  
  oops.. yes..
  
   
@@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct 
notifier_block *nb, unsigned long type,
 }
 
 /**
+ * devfreq_qos_notifier_call() -
   
   This looks like a missing kerneldoc comment?
  
  yes..  I'd either remove the comment or fill it in.
 
 Please add the comment.
 
+ */
+static int devfreq_qos_notifier_call(struct notifier_block *nb,
+  unsigned

Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-08 Thread Rafael J. Wysocki
On Thursday, August 30, 2012, MyungJoo Ham wrote:
> Even if the performance of a device is controlled properly with devfreq,
> sometimes, we still need to get PM-QoS inputs in order to meet the
> required performance.
> 
> In our testbed of Exynos4412, which has on-chip various DMA devices, the
> memory interface and system bus are controlled according to their
> utilization by devfreq. However, in some multimedia applications
> including video-playing with MFC (multi-function codec) H/W and
> photo/video-capturing with FIMC H/W, we have observed issues due to
> insufficient DMA throughput or latency.
> 
> In such applications, there are deadlines: less than 16.6ms with 60Hz.
> With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
> within a frame and we get missing frames and distorted pictures.
> With longer polling intervals (20 ~ 100ms), the response time is not
> sufficient and we get distorted or broken images. In other words,
> regardless of polling interval, we get poor results with hard-deadline
> H/Ws. They, in fact, have a preset requirement on DMA throughput.
> 
> Thus, we need PM-QoS capabilities in devfreq. (Note that for general
> user applications, devfreq for bus/memory works fine. They are not so
> sensitive to tens of ms in performance increasing responses in general.
> 
> In order to express how to handle QoS requests in devfreq devices,
> the devfreq device drivers only need to express the mappings of
> QoS value and frequency pairs with QoS class along with
> devfreq_add_device() call.
> 
> As a side effect of the implementation, which happens to be positive,
> min/max freq is now enforced regardless of governor implementation.

Can you please explain in a few words how this is supposed to work in
practice?

> Tested on Exynos4412 machines with memory/bus frequencies and multimedia
> H/W blocks. (camera, video decoding, and video encoding)
> 
> Signed-off-by: MyungJoo Ham 
> Signed-off-by: Kyungmin Park 

I'm not entirely convinced yet, but a few comments follow.

> ---
> Changed from V2-resend
> - Removed dependencies on global pm-qos class definitions
> - Revised data structure handling pm-qos (being ready for dev-pm-qos)
> 
> Changes from V2
> - Rebased
> 
> Changes from V1
> - Error handling at devfreq_add_device()
> - Handling pm_qos_max information
> - Styly update
> ---
>  drivers/devfreq/devfreq.c |  127 
> +++--
>  include/linux/devfreq.h   |   41 ++
>  2 files changed, 164 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 00e326c..d74b382 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "governor.h"
>  
>  static struct class *devfreq_class;
> @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
>* List from the highest proiority
>* max_freq (probably called by thermal when it's too hot)
>* min_freq
> +  * qos_min_freq
>*/
>  
> + if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
> + freq = devfreq->qos_min_freq;
> + flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */

What exactly is the purpose of this last line?

> + }
>   if (devfreq->min_freq && freq < devfreq->min_freq) {
>   freq = devfreq->min_freq;
>   flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
>   * devfreq_notifier_call() - Notify that the device frequency requirements
>   *  has been changed out of devfreq framework.
>   * @nb   the notifier_block (supposed to be devfreq->nb)
> - * @type not used
> + * @val  not used.
>   * @devp not used
>   *
>   * Called by a notifier that uses devfreq->nb.
>   */
> -static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
> type,
> +static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
> val,
>void *devp)
>  {
>   struct devfreq *devfreq = container_of(nb, struct devfreq, nb);

The above change is only a cleanup unrelated to the rest of modifications.
Please push it separately (if you _really_ think it's necessary).

> @@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct notifier_block 
> *nb, unsigned long type,
>  }
>  
>  /**
> + * devfreq_qos_notifier_call() -

This looks like a missing kerneldoc comment?

> + */
> +static int devfreq_qos_notifier_call(struct notifier_block *nb,
> +  unsigned long value, void *devp)
> +{
> + struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
> + int ret;
> + int i;
> + s32 default_value = PM_QOS_DEFAULT_VALUE;
> + struct devfreq_pm_qos_table *qos_list = devfreq->profile->qos_list;
> + bool qos_use_max 

Re: [PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-09-08 Thread Rafael J. Wysocki
On Thursday, August 30, 2012, MyungJoo Ham wrote:
 Even if the performance of a device is controlled properly with devfreq,
 sometimes, we still need to get PM-QoS inputs in order to meet the
 required performance.
 
 In our testbed of Exynos4412, which has on-chip various DMA devices, the
 memory interface and system bus are controlled according to their
 utilization by devfreq. However, in some multimedia applications
 including video-playing with MFC (multi-function codec) H/W and
 photo/video-capturing with FIMC H/W, we have observed issues due to
 insufficient DMA throughput or latency.
 
 In such applications, there are deadlines: less than 16.6ms with 60Hz.
 With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
 within a frame and we get missing frames and distorted pictures.
 With longer polling intervals (20 ~ 100ms), the response time is not
 sufficient and we get distorted or broken images. In other words,
 regardless of polling interval, we get poor results with hard-deadline
 H/Ws. They, in fact, have a preset requirement on DMA throughput.
 
 Thus, we need PM-QoS capabilities in devfreq. (Note that for general
 user applications, devfreq for bus/memory works fine. They are not so
 sensitive to tens of ms in performance increasing responses in general.
 
 In order to express how to handle QoS requests in devfreq devices,
 the devfreq device drivers only need to express the mappings of
 QoS value and frequency pairs with QoS class along with
 devfreq_add_device() call.
 
 As a side effect of the implementation, which happens to be positive,
 min/max freq is now enforced regardless of governor implementation.

Can you please explain in a few words how this is supposed to work in
practice?

 Tested on Exynos4412 machines with memory/bus frequencies and multimedia
 H/W blocks. (camera, video decoding, and video encoding)
 
 Signed-off-by: MyungJoo Ham myungjoo@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

I'm not entirely convinced yet, but a few comments follow.

 ---
 Changed from V2-resend
 - Removed dependencies on global pm-qos class definitions
 - Revised data structure handling pm-qos (being ready for dev-pm-qos)
 
 Changes from V2
 - Rebased
 
 Changes from V1
 - Error handling at devfreq_add_device()
 - Handling pm_qos_max information
 - Styly update
 ---
  drivers/devfreq/devfreq.c |  127 
 +++--
  include/linux/devfreq.h   |   41 ++
  2 files changed, 164 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
 index 00e326c..d74b382 100644
 --- a/drivers/devfreq/devfreq.c
 +++ b/drivers/devfreq/devfreq.c
 @@ -25,6 +25,7 @@
  #include linux/list.h
  #include linux/printk.h
  #include linux/hrtimer.h
 +#include linux/pm_qos.h
  #include governor.h
  
  static struct class *devfreq_class;
 @@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
* List from the highest proiority
* max_freq (probably called by thermal when it's too hot)
* min_freq
 +  * qos_min_freq
*/
  
 + if (devfreq-qos_min_freq  freq  devfreq-qos_min_freq) {
 + freq = devfreq-qos_min_freq;
 + flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */

What exactly is the purpose of this last line?

 + }
   if (devfreq-min_freq  freq  devfreq-min_freq) {
   freq = devfreq-min_freq;
   flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
 @@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
   * devfreq_notifier_call() - Notify that the device frequency requirements
   *  has been changed out of devfreq framework.
   * @nb   the notifier_block (supposed to be devfreq-nb)
 - * @type not used
 + * @val  not used.
   * @devp not used
   *
   * Called by a notifier that uses devfreq-nb.
   */
 -static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
 type,
 +static int devfreq_notifier_call(struct notifier_block *nb, unsigned long 
 val,
void *devp)
  {
   struct devfreq *devfreq = container_of(nb, struct devfreq, nb);

The above change is only a cleanup unrelated to the rest of modifications.
Please push it separately (if you _really_ think it's necessary).

 @@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct notifier_block 
 *nb, unsigned long type,
  }
  
  /**
 + * devfreq_qos_notifier_call() -

This looks like a missing kerneldoc comment?

 + */
 +static int devfreq_qos_notifier_call(struct notifier_block *nb,
 +  unsigned long value, void *devp)
 +{
 + struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
 + int ret;
 + int i;
 + s32 default_value = PM_QOS_DEFAULT_VALUE;
 + struct devfreq_pm_qos_table *qos_list = devfreq-profile-qos_list;
 + bool qos_use_max = 

[PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-08-30 Thread MyungJoo Ham
Even if the performance of a device is controlled properly with devfreq,
sometimes, we still need to get PM-QoS inputs in order to meet the
required performance.

In our testbed of Exynos4412, which has on-chip various DMA devices, the
memory interface and system bus are controlled according to their
utilization by devfreq. However, in some multimedia applications
including video-playing with MFC (multi-function codec) H/W and
photo/video-capturing with FIMC H/W, we have observed issues due to
insufficient DMA throughput or latency.

In such applications, there are deadlines: less than 16.6ms with 60Hz.
With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
within a frame and we get missing frames and distorted pictures.
With longer polling intervals (20 ~ 100ms), the response time is not
sufficient and we get distorted or broken images. In other words,
regardless of polling interval, we get poor results with hard-deadline
H/Ws. They, in fact, have a preset requirement on DMA throughput.

Thus, we need PM-QoS capabilities in devfreq. (Note that for general
user applications, devfreq for bus/memory works fine. They are not so
sensitive to tens of ms in performance increasing responses in general.

In order to express how to handle QoS requests in devfreq devices,
the devfreq device drivers only need to express the mappings of
QoS value and frequency pairs with QoS class along with
devfreq_add_device() call.

As a side effect of the implementation, which happens to be positive,
min/max freq is now enforced regardless of governor implementation.

Tested on Exynos4412 machines with memory/bus frequencies and multimedia
H/W blocks. (camera, video decoding, and video encoding)

Signed-off-by: MyungJoo Ham 
Signed-off-by: Kyungmin Park 

---
Changed from V2-resend
- Removed dependencies on global pm-qos class definitions
- Revised data structure handling pm-qos (being ready for dev-pm-qos)

Changes from V2
- Rebased

Changes from V1
- Error handling at devfreq_add_device()
- Handling pm_qos_max information
- Styly update
---
 drivers/devfreq/devfreq.c |  127 +++--
 include/linux/devfreq.h   |   41 ++
 2 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 00e326c..d74b382 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "governor.h"
 
 static struct class *devfreq_class;
@@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
 * List from the highest proiority
 * max_freq (probably called by thermal when it's too hot)
 * min_freq
+* qos_min_freq
 */
 
+   if (devfreq->qos_min_freq && freq < devfreq->qos_min_freq) {
+   freq = devfreq->qos_min_freq;
+   flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
+   }
if (devfreq->min_freq && freq < devfreq->min_freq) {
freq = devfreq->min_freq;
flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
@@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
  * devfreq_notifier_call() - Notify that the device frequency requirements
  *has been changed out of devfreq framework.
  * @nb the notifier_block (supposed to be devfreq->nb)
- * @type   not used
+ * @valnot used.
  * @devp   not used
  *
  * Called by a notifier that uses devfreq->nb.
  */
-static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
+static int devfreq_notifier_call(struct notifier_block *nb, unsigned long val,
 void *devp)
 {
struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
@@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct notifier_block 
*nb, unsigned long type,
 }
 
 /**
+ * devfreq_qos_notifier_call() -
+ */
+static int devfreq_qos_notifier_call(struct notifier_block *nb,
+unsigned long value, void *devp)
+{
+   struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
+   int ret;
+   int i;
+   s32 default_value = PM_QOS_DEFAULT_VALUE;
+   struct devfreq_pm_qos_table *qos_list = devfreq->profile->qos_list;
+   bool qos_use_max = devfreq->profile->qos_use_max;
+
+   if (!qos_list)
+   return NOTIFY_DONE;
+
+   mutex_lock(>lock);
+
+   if (value == default_value) {
+   devfreq->qos_min_freq = 0;
+   goto update;
+   }
+
+   for (i = 0; qos_list[i].freq; i++) {
+   /* QoS Met */
+   if ((qos_use_max && qos_list[i].qos_value >= value) ||
+   (!qos_use_max && qos_list[i].qos_value <= value)) {
+   devfreq->qos_min_freq = qos_list[i].freq;
+   goto update;
+   }
+   }
+
+   /* Use 

[PATCH v3 1/2] PM / devfreq: add global PM QoS support

2012-08-30 Thread MyungJoo Ham
Even if the performance of a device is controlled properly with devfreq,
sometimes, we still need to get PM-QoS inputs in order to meet the
required performance.

In our testbed of Exynos4412, which has on-chip various DMA devices, the
memory interface and system bus are controlled according to their
utilization by devfreq. However, in some multimedia applications
including video-playing with MFC (multi-function codec) H/W and
photo/video-capturing with FIMC H/W, we have observed issues due to
insufficient DMA throughput or latency.

In such applications, there are deadlines: less than 16.6ms with 60Hz.
With shorter polling intervals (5 ~ 15ms), the frequencies fluctuate
within a frame and we get missing frames and distorted pictures.
With longer polling intervals (20 ~ 100ms), the response time is not
sufficient and we get distorted or broken images. In other words,
regardless of polling interval, we get poor results with hard-deadline
H/Ws. They, in fact, have a preset requirement on DMA throughput.

Thus, we need PM-QoS capabilities in devfreq. (Note that for general
user applications, devfreq for bus/memory works fine. They are not so
sensitive to tens of ms in performance increasing responses in general.

In order to express how to handle QoS requests in devfreq devices,
the devfreq device drivers only need to express the mappings of
QoS value and frequency pairs with QoS class along with
devfreq_add_device() call.

As a side effect of the implementation, which happens to be positive,
min/max freq is now enforced regardless of governor implementation.

Tested on Exynos4412 machines with memory/bus frequencies and multimedia
H/W blocks. (camera, video decoding, and video encoding)

Signed-off-by: MyungJoo Ham myungjoo@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

---
Changed from V2-resend
- Removed dependencies on global pm-qos class definitions
- Revised data structure handling pm-qos (being ready for dev-pm-qos)

Changes from V2
- Rebased

Changes from V1
- Error handling at devfreq_add_device()
- Handling pm_qos_max information
- Styly update
---
 drivers/devfreq/devfreq.c |  127 +++--
 include/linux/devfreq.h   |   41 ++
 2 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 00e326c..d74b382 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -25,6 +25,7 @@
 #include linux/list.h
 #include linux/printk.h
 #include linux/hrtimer.h
+#include linux/pm_qos.h
 #include governor.h
 
 static struct class *devfreq_class;
@@ -136,8 +137,13 @@ int update_devfreq(struct devfreq *devfreq)
 * List from the highest proiority
 * max_freq (probably called by thermal when it's too hot)
 * min_freq
+* qos_min_freq
 */
 
+   if (devfreq-qos_min_freq  freq  devfreq-qos_min_freq) {
+   freq = devfreq-qos_min_freq;
+   flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
+   }
if (devfreq-min_freq  freq  devfreq-min_freq) {
freq = devfreq-min_freq;
flags = ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
@@ -164,12 +170,12 @@ int update_devfreq(struct devfreq *devfreq)
  * devfreq_notifier_call() - Notify that the device frequency requirements
  *has been changed out of devfreq framework.
  * @nb the notifier_block (supposed to be devfreq-nb)
- * @type   not used
+ * @valnot used.
  * @devp   not used
  *
  * Called by a notifier that uses devfreq-nb.
  */
-static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
+static int devfreq_notifier_call(struct notifier_block *nb, unsigned long val,
 void *devp)
 {
struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
@@ -183,6 +189,49 @@ static int devfreq_notifier_call(struct notifier_block 
*nb, unsigned long type,
 }
 
 /**
+ * devfreq_qos_notifier_call() -
+ */
+static int devfreq_qos_notifier_call(struct notifier_block *nb,
+unsigned long value, void *devp)
+{
+   struct devfreq *devfreq = container_of(nb, struct devfreq, qos_nb);
+   int ret;
+   int i;
+   s32 default_value = PM_QOS_DEFAULT_VALUE;
+   struct devfreq_pm_qos_table *qos_list = devfreq-profile-qos_list;
+   bool qos_use_max = devfreq-profile-qos_use_max;
+
+   if (!qos_list)
+   return NOTIFY_DONE;
+
+   mutex_lock(devfreq-lock);
+
+   if (value == default_value) {
+   devfreq-qos_min_freq = 0;
+   goto update;
+   }
+
+   for (i = 0; qos_list[i].freq; i++) {
+   /* QoS Met */
+   if ((qos_use_max  qos_list[i].qos_value = value) ||
+   (!qos_use_max  qos_list[i].qos_value = value)) {
+   devfreq-qos_min_freq = qos_list[i].freq;