Re: [PATCH v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Jani Nikula 
On Thu, 10 Oct 2013, Aaron Lu  wrote:
> On 10/10/2013 12:29 PM, Jani Nikula wrote:
>> On Thu, 10 Oct 2013, Aaron Lu  wrote:
>>> On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
> +bool backlight_device_registered(enum backlight_type type)
> +{
> + bool found = false;
> + struct backlight_device *bd;
> +
> + mutex_lock(_list_mutex);
> + list_for_each_entry(bd, _list_head, entry) {
> + if (bd->props.type == type) {
> + found = true;
> + break;
> + }
> + }

 Isn't it useful to be able to register more than one backlight device of 
 the
 same type sometimes?
>>>
>>> I think so for some kind of computers. OTOH, the above function should
>>> be enough for the problem we are solving here, if someday we need to
>>> differentiate, we can enhance the code then.
>> 
>> Since both Baytrail and Haswell already have two backlight PWMs, this
>> may be needed sooner than you think. But we shouldn't let that block
>
> Do we need to differentiate which backlight PWM is registered to decide
> if ACPI video backlight interface should be skipped? My understanding is
> no.

That's correct. If things change, we can fix it then.

Jani.


>
> Thanks,
> Aaron
>
>> fixing the more urgent issue we have now. So I'm fine with this. It
>> doesn't prevent one from registering more than one device of the same
>> type anyway.
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>

-- 
Jani Nikula, Intel Open Source Technology Center
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Aaron Lu 
On 10/10/2013 12:29 PM, Jani Nikula wrote:
> On Thu, 10 Oct 2013, Aaron Lu  wrote:
>> On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
>>> On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 +bool backlight_device_registered(enum backlight_type type)
 +{
 +  bool found = false;
 +  struct backlight_device *bd;
 +
 +  mutex_lock(_list_mutex);
 +  list_for_each_entry(bd, _list_head, entry) {
 +  if (bd->props.type == type) {
 +  found = true;
 +  break;
 +  }
 +  }
>>>
>>> Isn't it useful to be able to register more than one backlight device of the
>>> same type sometimes?
>>
>> I think so for some kind of computers. OTOH, the above function should
>> be enough for the problem we are solving here, if someday we need to
>> differentiate, we can enhance the code then.
> 
> Since both Baytrail and Haswell already have two backlight PWMs, this
> may be needed sooner than you think. But we shouldn't let that block

Do we need to differentiate which backlight PWM is registered to decide
if ACPI video backlight interface should be skipped? My understanding is
no.

Thanks,
Aaron

> fixing the more urgent issue we have now. So I'm fine with this. It
> doesn't prevent one from registering more than one device of the same
> type anyway.
> 
> BR,
> Jani.
> 
> 
> 

--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Jani Nikula 
On Thu, 10 Oct 2013, Aaron Lu  wrote:
> On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
>> On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
>>> +bool backlight_device_registered(enum backlight_type type)
>>> +{
>>> +   bool found = false;
>>> +   struct backlight_device *bd;
>>> +
>>> +   mutex_lock(_list_mutex);
>>> +   list_for_each_entry(bd, _list_head, entry) {
>>> +   if (bd->props.type == type) {
>>> +   found = true;
>>> +   break;
>>> +   }
>>> +   }
>> 
>> Isn't it useful to be able to register more than one backlight device of the
>> same type sometimes?
>
> I think so for some kind of computers. OTOH, the above function should
> be enough for the problem we are solving here, if someday we need to
> differentiate, we can enhance the code then.

Since both Baytrail and Haswell already have two backlight PWMs, this
may be needed sooner than you think. But we shouldn't let that block
fixing the more urgent issue we have now. So I'm fine with this. It
doesn't prevent one from registering more than one device of the same
type anyway.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Rafael J. Wysocki 
On Thursday, October 10, 2013 08:54:29 AM Aaron Lu wrote:
> On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
> > On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
> >> Introduce a new API for modules to query if a specific type of backlight
> >> device has been registered. This is useful for some backlight device
> >> provider module(e.g. ACPI video) to know if a native control
> >> interface(e.g. the interface created by i915) is available and then do
> >> things accordingly(e.g. avoid register its own on Win8 systems).
> >>
> >> Signed-off-by: Aaron Lu 
> >> Tested-by: Igor Gnatenko 
> >> Tested-by: Yves-Alexis Perez 
> >> Tested-by: Mika Westerberg 
> >> ---
> >>  drivers/video/backlight/backlight.c | 31 +++
> >>  include/linux/backlight.h   |  4 
> >>  2 files changed, 35 insertions(+)
> >>
> >> diff --git a/drivers/video/backlight/backlight.c 
> >> b/drivers/video/backlight/backlight.c
> >> index 94a403a..bf2d71d 100644
> >> --- a/drivers/video/backlight/backlight.c
> >> +++ b/drivers/video/backlight/backlight.c
> >> @@ -21,6 +21,9 @@
> >>  #include 
> >>  #endif
> >>  
> >> +static struct list_head bd_list_head;
> >> +static struct mutex bd_list_mutex;
> > 
> > I'd prefer these two things to be called backlight_dev_list and
> > backlight_dev_list_mutex, respectively.
> 
> OK.
> 
> > 
> >> +
> >>  static const char *const backlight_types[] = {
> >>[BACKLIGHT_RAW] = "raw",
> >>[BACKLIGHT_PLATFORM] = "platform",
> >> @@ -349,10 +352,32 @@ struct backlight_device 
> >> *backlight_device_register(const char *name,
> >>mutex_unlock(_backlight_mutex);
> >>  #endif
> >>  
> >> +  mutex_lock(_list_mutex);
> >> +  list_add(_bd->entry, _list_head);
> >> +  mutex_unlock(_list_mutex);
> >> +
> >>return new_bd;
> >>  }
> >>  EXPORT_SYMBOL(backlight_device_register);
> >>  
> >> +bool backlight_device_registered(enum backlight_type type)
> >> +{
> >> +  bool found = false;
> >> +  struct backlight_device *bd;
> >> +
> >> +  mutex_lock(_list_mutex);
> >> +  list_for_each_entry(bd, _list_head, entry) {
> >> +  if (bd->props.type == type) {
> >> +  found = true;
> >> +  break;
> >> +  }
> >> +  }
> > 
> > Isn't it useful to be able to register more than one backlight device of the
> > same type sometimes?
> 
> I think so for some kind of computers. OTOH, the above function should
> be enough for the problem we are solving here, if someday we need to
> differentiate, we can enhance the code then.

OK

--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Aaron Lu 
On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
> On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
>> Introduce a new API for modules to query if a specific type of backlight
>> device has been registered. This is useful for some backlight device
>> provider module(e.g. ACPI video) to know if a native control
>> interface(e.g. the interface created by i915) is available and then do
>> things accordingly(e.g. avoid register its own on Win8 systems).
>>
>> Signed-off-by: Aaron Lu 
>> Tested-by: Igor Gnatenko 
>> Tested-by: Yves-Alexis Perez 
>> Tested-by: Mika Westerberg 
>> ---
>>  drivers/video/backlight/backlight.c | 31 +++
>>  include/linux/backlight.h   |  4 
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c 
>> b/drivers/video/backlight/backlight.c
>> index 94a403a..bf2d71d 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -21,6 +21,9 @@
>>  #include 
>>  #endif
>>  
>> +static struct list_head bd_list_head;
>> +static struct mutex bd_list_mutex;
> 
> I'd prefer these two things to be called backlight_dev_list and
> backlight_dev_list_mutex, respectively.

OK.

> 
>> +
>>  static const char *const backlight_types[] = {
>>  [BACKLIGHT_RAW] = "raw",
>>  [BACKLIGHT_PLATFORM] = "platform",
>> @@ -349,10 +352,32 @@ struct backlight_device 
>> *backlight_device_register(const char *name,
>>  mutex_unlock(_backlight_mutex);
>>  #endif
>>  
>> +mutex_lock(_list_mutex);
>> +list_add(_bd->entry, _list_head);
>> +mutex_unlock(_list_mutex);
>> +
>>  return new_bd;
>>  }
>>  EXPORT_SYMBOL(backlight_device_register);
>>  
>> +bool backlight_device_registered(enum backlight_type type)
>> +{
>> +bool found = false;
>> +struct backlight_device *bd;
>> +
>> +mutex_lock(_list_mutex);
>> +list_for_each_entry(bd, _list_head, entry) {
>> +if (bd->props.type == type) {
>> +found = true;
>> +break;
>> +}
>> +}
> 
> Isn't it useful to be able to register more than one backlight device of the
> same type sometimes?

I think so for some kind of computers. OTOH, the above function should
be enough for the problem we are solving here, if someday we need to
differentiate, we can enhance the code then.

> 
>> +mutex_unlock(_list_mutex);
>> +
>> +return found;
>> +}
>> +EXPORT_SYMBOL(backlight_device_registered);
>> +
>>  /**
>>   * backlight_device_unregister - unregisters a backlight device object.
>>   * @bd: the backlight device object to be unregistered and freed.
>> @@ -364,6 +389,10 @@ void backlight_device_unregister(struct 
>> backlight_device *bd)
>>  if (!bd)
>>  return;
>>  
>> +mutex_lock(_list_mutex);
>> +list_del(>entry);
>> +mutex_unlock(_list_mutex);
>> +
>>  #ifdef CONFIG_PMAC_BACKLIGHT
>>  mutex_lock(_backlight_mutex);
>>  if (pmac_backlight == bd)
>> @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
>>  
>>  backlight_class->dev_groups = bl_device_groups;
>>  backlight_class->pm = _class_dev_pm_ops;
>> +INIT_LIST_HEAD(_list_head);
>> +mutex_init(_list_mutex);
>>  return 0;
>>  }
>>  
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 53b7794..5f9cd96 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -100,6 +100,9 @@ struct backlight_device {
>>  /* The framebuffer notifier block */
>>  struct notifier_block fb_notif;
>>  
>> +/* list entry of all registered backlight devices */
>> +struct list_head entry;
>> +
>>  struct device dev;
>>  };
>>  
>> @@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct 
>> device *dev,
>>  struct backlight_device *bd);
>>  extern void backlight_force_update(struct backlight_device *bd,
>> enum backlight_update_reason reason);
>> +extern bool backlight_device_registered(enum backlight_type type);
>>  
>>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
>> dev)
> 
> Thanks!

Thanks for the review,
-Aaron
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Rafael J. Wysocki 
On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
> Introduce a new API for modules to query if a specific type of backlight
> device has been registered. This is useful for some backlight device
> provider module(e.g. ACPI video) to know if a native control
> interface(e.g. the interface created by i915) is available and then do
> things accordingly(e.g. avoid register its own on Win8 systems).
> 
> Signed-off-by: Aaron Lu 
> Tested-by: Igor Gnatenko 
> Tested-by: Yves-Alexis Perez 
> Tested-by: Mika Westerberg 
> ---
>  drivers/video/backlight/backlight.c | 31 +++
>  include/linux/backlight.h   |  4 
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c 
> b/drivers/video/backlight/backlight.c
> index 94a403a..bf2d71d 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -21,6 +21,9 @@
>  #include 
>  #endif
>  
> +static struct list_head bd_list_head;
> +static struct mutex bd_list_mutex;

I'd prefer these two things to be called backlight_dev_list and
backlight_dev_list_mutex, respectively.

> +
>  static const char *const backlight_types[] = {
>   [BACKLIGHT_RAW] = "raw",
>   [BACKLIGHT_PLATFORM] = "platform",
> @@ -349,10 +352,32 @@ struct backlight_device 
> *backlight_device_register(const char *name,
>   mutex_unlock(_backlight_mutex);
>  #endif
>  
> + mutex_lock(_list_mutex);
> + list_add(_bd->entry, _list_head);
> + mutex_unlock(_list_mutex);
> +
>   return new_bd;
>  }
>  EXPORT_SYMBOL(backlight_device_register);
>  
> +bool backlight_device_registered(enum backlight_type type)
> +{
> + bool found = false;
> + struct backlight_device *bd;
> +
> + mutex_lock(_list_mutex);
> + list_for_each_entry(bd, _list_head, entry) {
> + if (bd->props.type == type) {
> + found = true;
> + break;
> + }
> + }

Isn't it useful to be able to register more than one backlight device of the
same type sometimes?

> + mutex_unlock(_list_mutex);
> +
> + return found;
> +}
> +EXPORT_SYMBOL(backlight_device_registered);
> +
>  /**
>   * backlight_device_unregister - unregisters a backlight device object.
>   * @bd: the backlight device object to be unregistered and freed.
> @@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device 
> *bd)
>   if (!bd)
>   return;
>  
> + mutex_lock(_list_mutex);
> + list_del(>entry);
> + mutex_unlock(_list_mutex);
> +
>  #ifdef CONFIG_PMAC_BACKLIGHT
>   mutex_lock(_backlight_mutex);
>   if (pmac_backlight == bd)
> @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
>  
>   backlight_class->dev_groups = bl_device_groups;
>   backlight_class->pm = _class_dev_pm_ops;
> + INIT_LIST_HEAD(_list_head);
> + mutex_init(_list_mutex);
>   return 0;
>  }
>  
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 53b7794..5f9cd96 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -100,6 +100,9 @@ struct backlight_device {
>   /* The framebuffer notifier block */
>   struct notifier_block fb_notif;
>  
> + /* list entry of all registered backlight devices */
> + struct list_head entry;
> +
>   struct device dev;
>  };
>  
> @@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct 
> device *dev,
>   struct backlight_device *bd);
>  extern void backlight_force_update(struct backlight_device *bd,
>  enum backlight_update_reason reason);
> +extern bool backlight_device_registered(enum backlight_type type);
>  
>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
> dev)

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Rafael J. Wysocki 
On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 Introduce a new API for modules to query if a specific type of backlight
 device has been registered. This is useful for some backlight device
 provider module(e.g. ACPI video) to know if a native control
 interface(e.g. the interface created by i915) is available and then do
 things accordingly(e.g. avoid register its own on Win8 systems).
 
 Signed-off-by: Aaron Lu aaron...@intel.com
 Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
 Tested-by: Yves-Alexis Perez cor...@debian.org
 Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
 ---
  drivers/video/backlight/backlight.c | 31 +++
  include/linux/backlight.h   |  4 
  2 files changed, 35 insertions(+)
 
 diff --git a/drivers/video/backlight/backlight.c 
 b/drivers/video/backlight/backlight.c
 index 94a403a..bf2d71d 100644
 --- a/drivers/video/backlight/backlight.c
 +++ b/drivers/video/backlight/backlight.c
 @@ -21,6 +21,9 @@
  #include asm/backlight.h
  #endif
  
 +static struct list_head bd_list_head;
 +static struct mutex bd_list_mutex;

I'd prefer these two things to be called backlight_dev_list and
backlight_dev_list_mutex, respectively.

 +
  static const char *const backlight_types[] = {
   [BACKLIGHT_RAW] = raw,
   [BACKLIGHT_PLATFORM] = platform,
 @@ -349,10 +352,32 @@ struct backlight_device 
 *backlight_device_register(const char *name,
   mutex_unlock(pmac_backlight_mutex);
  #endif
  
 + mutex_lock(bd_list_mutex);
 + list_add(new_bd-entry, bd_list_head);
 + mutex_unlock(bd_list_mutex);
 +
   return new_bd;
  }
  EXPORT_SYMBOL(backlight_device_register);
  
 +bool backlight_device_registered(enum backlight_type type)
 +{
 + bool found = false;
 + struct backlight_device *bd;
 +
 + mutex_lock(bd_list_mutex);
 + list_for_each_entry(bd, bd_list_head, entry) {
 + if (bd-props.type == type) {
 + found = true;
 + break;
 + }
 + }

Isn't it useful to be able to register more than one backlight device of the
same type sometimes?

 + mutex_unlock(bd_list_mutex);
 +
 + return found;
 +}
 +EXPORT_SYMBOL(backlight_device_registered);
 +
  /**
   * backlight_device_unregister - unregisters a backlight device object.
   * @bd: the backlight device object to be unregistered and freed.
 @@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device 
 *bd)
   if (!bd)
   return;
  
 + mutex_lock(bd_list_mutex);
 + list_del(bd-entry);
 + mutex_unlock(bd_list_mutex);
 +
  #ifdef CONFIG_PMAC_BACKLIGHT
   mutex_lock(pmac_backlight_mutex);
   if (pmac_backlight == bd)
 @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
  
   backlight_class-dev_groups = bl_device_groups;
   backlight_class-pm = backlight_class_dev_pm_ops;
 + INIT_LIST_HEAD(bd_list_head);
 + mutex_init(bd_list_mutex);
   return 0;
  }
  
 diff --git a/include/linux/backlight.h b/include/linux/backlight.h
 index 53b7794..5f9cd96 100644
 --- a/include/linux/backlight.h
 +++ b/include/linux/backlight.h
 @@ -100,6 +100,9 @@ struct backlight_device {
   /* The framebuffer notifier block */
   struct notifier_block fb_notif;
  
 + /* list entry of all registered backlight devices */
 + struct list_head entry;
 +
   struct device dev;
  };
  
 @@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct 
 device *dev,
   struct backlight_device *bd);
  extern void backlight_force_update(struct backlight_device *bd,
  enum backlight_update_reason reason);
 +extern bool backlight_device_registered(enum backlight_type type);
  
  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
 dev)

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Aaron Lu 
On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 Introduce a new API for modules to query if a specific type of backlight
 device has been registered. This is useful for some backlight device
 provider module(e.g. ACPI video) to know if a native control
 interface(e.g. the interface created by i915) is available and then do
 things accordingly(e.g. avoid register its own on Win8 systems).

 Signed-off-by: Aaron Lu aaron...@intel.com
 Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
 Tested-by: Yves-Alexis Perez cor...@debian.org
 Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
 ---
  drivers/video/backlight/backlight.c | 31 +++
  include/linux/backlight.h   |  4 
  2 files changed, 35 insertions(+)

 diff --git a/drivers/video/backlight/backlight.c 
 b/drivers/video/backlight/backlight.c
 index 94a403a..bf2d71d 100644
 --- a/drivers/video/backlight/backlight.c
 +++ b/drivers/video/backlight/backlight.c
 @@ -21,6 +21,9 @@
  #include asm/backlight.h
  #endif
  
 +static struct list_head bd_list_head;
 +static struct mutex bd_list_mutex;
 
 I'd prefer these two things to be called backlight_dev_list and
 backlight_dev_list_mutex, respectively.

OK.

 
 +
  static const char *const backlight_types[] = {
  [BACKLIGHT_RAW] = raw,
  [BACKLIGHT_PLATFORM] = platform,
 @@ -349,10 +352,32 @@ struct backlight_device 
 *backlight_device_register(const char *name,
  mutex_unlock(pmac_backlight_mutex);
  #endif
  
 +mutex_lock(bd_list_mutex);
 +list_add(new_bd-entry, bd_list_head);
 +mutex_unlock(bd_list_mutex);
 +
  return new_bd;
  }
  EXPORT_SYMBOL(backlight_device_register);
  
 +bool backlight_device_registered(enum backlight_type type)
 +{
 +bool found = false;
 +struct backlight_device *bd;
 +
 +mutex_lock(bd_list_mutex);
 +list_for_each_entry(bd, bd_list_head, entry) {
 +if (bd-props.type == type) {
 +found = true;
 +break;
 +}
 +}
 
 Isn't it useful to be able to register more than one backlight device of the
 same type sometimes?

I think so for some kind of computers. OTOH, the above function should
be enough for the problem we are solving here, if someday we need to
differentiate, we can enhance the code then.

 
 +mutex_unlock(bd_list_mutex);
 +
 +return found;
 +}
 +EXPORT_SYMBOL(backlight_device_registered);
 +
  /**
   * backlight_device_unregister - unregisters a backlight device object.
   * @bd: the backlight device object to be unregistered and freed.
 @@ -364,6 +389,10 @@ void backlight_device_unregister(struct 
 backlight_device *bd)
  if (!bd)
  return;
  
 +mutex_lock(bd_list_mutex);
 +list_del(bd-entry);
 +mutex_unlock(bd_list_mutex);
 +
  #ifdef CONFIG_PMAC_BACKLIGHT
  mutex_lock(pmac_backlight_mutex);
  if (pmac_backlight == bd)
 @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
  
  backlight_class-dev_groups = bl_device_groups;
  backlight_class-pm = backlight_class_dev_pm_ops;
 +INIT_LIST_HEAD(bd_list_head);
 +mutex_init(bd_list_mutex);
  return 0;
  }
  
 diff --git a/include/linux/backlight.h b/include/linux/backlight.h
 index 53b7794..5f9cd96 100644
 --- a/include/linux/backlight.h
 +++ b/include/linux/backlight.h
 @@ -100,6 +100,9 @@ struct backlight_device {
  /* The framebuffer notifier block */
  struct notifier_block fb_notif;
  
 +/* list entry of all registered backlight devices */
 +struct list_head entry;
 +
  struct device dev;
  };
  
 @@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct 
 device *dev,
  struct backlight_device *bd);
  extern void backlight_force_update(struct backlight_device *bd,
 enum backlight_update_reason reason);
 +extern bool backlight_device_registered(enum backlight_type type);
  
  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
 dev)
 
 Thanks!

Thanks for the review,
-Aaron
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Rafael J. Wysocki 
On Thursday, October 10, 2013 08:54:29 AM Aaron Lu wrote:
 On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
  On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
  Introduce a new API for modules to query if a specific type of backlight
  device has been registered. This is useful for some backlight device
  provider module(e.g. ACPI video) to know if a native control
  interface(e.g. the interface created by i915) is available and then do
  things accordingly(e.g. avoid register its own on Win8 systems).
 
  Signed-off-by: Aaron Lu aaron...@intel.com
  Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
  Tested-by: Yves-Alexis Perez cor...@debian.org
  Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
  ---
   drivers/video/backlight/backlight.c | 31 +++
   include/linux/backlight.h   |  4 
   2 files changed, 35 insertions(+)
 
  diff --git a/drivers/video/backlight/backlight.c 
  b/drivers/video/backlight/backlight.c
  index 94a403a..bf2d71d 100644
  --- a/drivers/video/backlight/backlight.c
  +++ b/drivers/video/backlight/backlight.c
  @@ -21,6 +21,9 @@
   #include asm/backlight.h
   #endif
   
  +static struct list_head bd_list_head;
  +static struct mutex bd_list_mutex;
  
  I'd prefer these two things to be called backlight_dev_list and
  backlight_dev_list_mutex, respectively.
 
 OK.
 
  
  +
   static const char *const backlight_types[] = {
 [BACKLIGHT_RAW] = raw,
 [BACKLIGHT_PLATFORM] = platform,
  @@ -349,10 +352,32 @@ struct backlight_device 
  *backlight_device_register(const char *name,
 mutex_unlock(pmac_backlight_mutex);
   #endif
   
  +  mutex_lock(bd_list_mutex);
  +  list_add(new_bd-entry, bd_list_head);
  +  mutex_unlock(bd_list_mutex);
  +
 return new_bd;
   }
   EXPORT_SYMBOL(backlight_device_register);
   
  +bool backlight_device_registered(enum backlight_type type)
  +{
  +  bool found = false;
  +  struct backlight_device *bd;
  +
  +  mutex_lock(bd_list_mutex);
  +  list_for_each_entry(bd, bd_list_head, entry) {
  +  if (bd-props.type == type) {
  +  found = true;
  +  break;
  +  }
  +  }
  
  Isn't it useful to be able to register more than one backlight device of the
  same type sometimes?
 
 I think so for some kind of computers. OTOH, the above function should
 be enough for the problem we are solving here, if someday we need to
 differentiate, we can enhance the code then.

OK

--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Jani Nikula 
On Thu, 10 Oct 2013, Aaron Lu aaron...@intel.com wrote:
 On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 +bool backlight_device_registered(enum backlight_type type)
 +{
 +   bool found = false;
 +   struct backlight_device *bd;
 +
 +   mutex_lock(bd_list_mutex);
 +   list_for_each_entry(bd, bd_list_head, entry) {
 +   if (bd-props.type == type) {
 +   found = true;
 +   break;
 +   }
 +   }
 
 Isn't it useful to be able to register more than one backlight device of the
 same type sometimes?

 I think so for some kind of computers. OTOH, the above function should
 be enough for the problem we are solving here, if someday we need to
 differentiate, we can enhance the code then.

Since both Baytrail and Haswell already have two backlight PWMs, this
may be needed sooner than you think. But we shouldn't let that block
fixing the more urgent issue we have now. So I'm fine with this. It
doesn't prevent one from registering more than one device of the same
type anyway.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Aaron Lu 
On 10/10/2013 12:29 PM, Jani Nikula wrote:
 On Thu, 10 Oct 2013, Aaron Lu aaron...@intel.com wrote:
 On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 +bool backlight_device_registered(enum backlight_type type)
 +{
 +  bool found = false;
 +  struct backlight_device *bd;
 +
 +  mutex_lock(bd_list_mutex);
 +  list_for_each_entry(bd, bd_list_head, entry) {
 +  if (bd-props.type == type) {
 +  found = true;
 +  break;
 +  }
 +  }

 Isn't it useful to be able to register more than one backlight device of the
 same type sometimes?

 I think so for some kind of computers. OTOH, the above function should
 be enough for the problem we are solving here, if someday we need to
 differentiate, we can enhance the code then.
 
 Since both Baytrail and Haswell already have two backlight PWMs, this
 may be needed sooner than you think. But we shouldn't let that block

Do we need to differentiate which backlight PWM is registered to decide
if ACPI video backlight interface should be skipped? My understanding is
no.

Thanks,
Aaron

 fixing the more urgent issue we have now. So I'm fine with this. It
 doesn't prevent one from registering more than one device of the same
 type anyway.
 
 BR,
 Jani.
 
 
 

--
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 v4 1/4] backlight: introduce backlight_device_registered

2013-10-09 Thread Jani Nikula 
On Thu, 10 Oct 2013, Aaron Lu aaron...@intel.com wrote:
 On 10/10/2013 12:29 PM, Jani Nikula wrote:
 On Thu, 10 Oct 2013, Aaron Lu aaron...@intel.com wrote:
 On 10/10/2013 08:25 AM, Rafael J. Wysocki wrote:
 On Tuesday, October 08, 2013 02:39:58 PM Aaron Lu wrote:
 +bool backlight_device_registered(enum backlight_type type)
 +{
 + bool found = false;
 + struct backlight_device *bd;
 +
 + mutex_lock(bd_list_mutex);
 + list_for_each_entry(bd, bd_list_head, entry) {
 + if (bd-props.type == type) {
 + found = true;
 + break;
 + }
 + }

 Isn't it useful to be able to register more than one backlight device of 
 the
 same type sometimes?

 I think so for some kind of computers. OTOH, the above function should
 be enough for the problem we are solving here, if someday we need to
 differentiate, we can enhance the code then.
 
 Since both Baytrail and Haswell already have two backlight PWMs, this
 may be needed sooner than you think. But we shouldn't let that block

 Do we need to differentiate which backlight PWM is registered to decide
 if ACPI video backlight interface should be skipped? My understanding is
 no.

That's correct. If things change, we can fix it then.

Jani.



 Thanks,
 Aaron

 fixing the more urgent issue we have now. So I'm fine with this. It
 doesn't prevent one from registering more than one device of the same
 type anyway.
 
 BR,
 Jani.
 
 
 


-- 
Jani Nikula, Intel Open Source Technology Center
--
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/

[PATCH v4 1/4] backlight: introduce backlight_device_registered

2013-10-08 Thread Aaron Lu 
Introduce a new API for modules to query if a specific type of backlight
device has been registered. This is useful for some backlight device
provider module(e.g. ACPI video) to know if a native control
interface(e.g. the interface created by i915) is available and then do
things accordingly(e.g. avoid register its own on Win8 systems).

Signed-off-by: Aaron Lu 
Tested-by: Igor Gnatenko 
Tested-by: Yves-Alexis Perez 
Tested-by: Mika Westerberg 
---
 drivers/video/backlight/backlight.c | 31 +++
 include/linux/backlight.h   |  4 
 2 files changed, 35 insertions(+)

diff --git a/drivers/video/backlight/backlight.c 
b/drivers/video/backlight/backlight.c
index 94a403a..bf2d71d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
 #include 
 #endif
 
+static struct list_head bd_list_head;
+static struct mutex bd_list_mutex;
+
 static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
[BACKLIGHT_PLATFORM] = "platform",
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const 
char *name,
mutex_unlock(_backlight_mutex);
 #endif
 
+   mutex_lock(_list_mutex);
+   list_add(_bd->entry, _list_head);
+   mutex_unlock(_list_mutex);
+
return new_bd;
 }
 EXPORT_SYMBOL(backlight_device_register);
 
+bool backlight_device_registered(enum backlight_type type)
+{
+   bool found = false;
+   struct backlight_device *bd;
+
+   mutex_lock(_list_mutex);
+   list_for_each_entry(bd, _list_head, entry) {
+   if (bd->props.type == type) {
+   found = true;
+   break;
+   }
+   }
+   mutex_unlock(_list_mutex);
+
+   return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
 /**
  * backlight_device_unregister - unregisters a backlight device object.
  * @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device 
*bd)
if (!bd)
return;
 
+   mutex_lock(_list_mutex);
+   list_del(>entry);
+   mutex_unlock(_list_mutex);
+
 #ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
 
backlight_class->dev_groups = bl_device_groups;
backlight_class->pm = _class_dev_pm_ops;
+   INIT_LIST_HEAD(_list_head);
+   mutex_init(_list_mutex);
return 0;
 }
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
 
+   /* list entry of all registered backlight devices */
+   struct list_head entry;
+
struct device dev;
 };
 
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device 
*dev,
struct backlight_device *bd);
 extern void backlight_force_update(struct backlight_device *bd,
   enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
 
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
dev)
 
-- 
1.8.4.12.g2ea3df6

--
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/

[PATCH v4 1/4] backlight: introduce backlight_device_registered

2013-10-08 Thread Aaron Lu 
Introduce a new API for modules to query if a specific type of backlight
device has been registered. This is useful for some backlight device
provider module(e.g. ACPI video) to know if a native control
interface(e.g. the interface created by i915) is available and then do
things accordingly(e.g. avoid register its own on Win8 systems).

Signed-off-by: Aaron Lu aaron...@intel.com
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Tested-by: Yves-Alexis Perez cor...@debian.org
Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
---
 drivers/video/backlight/backlight.c | 31 +++
 include/linux/backlight.h   |  4 
 2 files changed, 35 insertions(+)

diff --git a/drivers/video/backlight/backlight.c 
b/drivers/video/backlight/backlight.c
index 94a403a..bf2d71d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -21,6 +21,9 @@
 #include asm/backlight.h
 #endif
 
+static struct list_head bd_list_head;
+static struct mutex bd_list_mutex;
+
 static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = raw,
[BACKLIGHT_PLATFORM] = platform,
@@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const 
char *name,
mutex_unlock(pmac_backlight_mutex);
 #endif
 
+   mutex_lock(bd_list_mutex);
+   list_add(new_bd-entry, bd_list_head);
+   mutex_unlock(bd_list_mutex);
+
return new_bd;
 }
 EXPORT_SYMBOL(backlight_device_register);
 
+bool backlight_device_registered(enum backlight_type type)
+{
+   bool found = false;
+   struct backlight_device *bd;
+
+   mutex_lock(bd_list_mutex);
+   list_for_each_entry(bd, bd_list_head, entry) {
+   if (bd-props.type == type) {
+   found = true;
+   break;
+   }
+   }
+   mutex_unlock(bd_list_mutex);
+
+   return found;
+}
+EXPORT_SYMBOL(backlight_device_registered);
+
 /**
  * backlight_device_unregister - unregisters a backlight device object.
  * @bd: the backlight device object to be unregistered and freed.
@@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device 
*bd)
if (!bd)
return;
 
+   mutex_lock(bd_list_mutex);
+   list_del(bd-entry);
+   mutex_unlock(bd_list_mutex);
+
 #ifdef CONFIG_PMAC_BACKLIGHT
mutex_lock(pmac_backlight_mutex);
if (pmac_backlight == bd)
@@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
 
backlight_class-dev_groups = bl_device_groups;
backlight_class-pm = backlight_class_dev_pm_ops;
+   INIT_LIST_HEAD(bd_list_head);
+   mutex_init(bd_list_mutex);
return 0;
 }
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 53b7794..5f9cd96 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -100,6 +100,9 @@ struct backlight_device {
/* The framebuffer notifier block */
struct notifier_block fb_notif;
 
+   /* list entry of all registered backlight devices */
+   struct list_head entry;
+
struct device dev;
 };
 
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device 
*dev,
struct backlight_device *bd);
 extern void backlight_force_update(struct backlight_device *bd,
   enum backlight_update_reason reason);
+extern bool backlight_device_registered(enum backlight_type type);
 
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
dev)
 
-- 
1.8.4.12.g2ea3df6

--
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/