From: Andrew Morton on
On Mon, 2 Nov 2009 17:50:02 +0900
InKi Dae <daeinki(a)gmail.com> wrote:

> This patch adds brightness feature to lcd class.
> (kernel/driver/video/backlight/lcd.c)
>
> In the past, most of the lcd panels for embedded system was TFT-LCD
> Panel needing backlight device.
> But now AMOLED LCD Panel appeared so we should consider brightness
> control for AMOLED Panel.
>
> For the time being, I used backlight fake driver for brightness
> control of AMOLED LCD Panel.
> But this way is not good, so I propose to add brightness feature to lcd class.
>
> For this, I attached patch file and if my proposal is approved
> Then I will send s6e63m0 and tl2796 AMOLED lcd panel driver based on
> lcd class modified soon.

Please send the s6e63m0 and tl2796 patches anyway. That way we have
some examples of how this new capability is used by drivers.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Richard Purdie on
On Mon, 2009-11-02 at 17:50 +0900, InKi Dae wrote:
> This patch adds brightness feature to lcd class.
> (kernel/driver/video/backlight/lcd.c)
>
> In the past, most of the lcd panels for embedded system was TFT-LCD
> Panel needing backlight device.
> But now AMOLED LCD Panel appeared so we should consider brightness
> control for AMOLED Panel.
>
> For the time being, I used backlight fake driver for brightness
> control of AMOLED LCD Panel.
> But this way is not good, so I propose to add brightness feature to lcd class.
>
> For this, I attached patch file and if my proposal is approved
> Then I will send s6e63m0 and tl2796 AMOLED lcd panel driver based on
> lcd class modified soon.

Pavel has a good point but let me try and explain it differently.

The point of the backlight class is to expose a backlight brightness
control to userspace in a consistent well defined way. Anyone wishing to
write a piece of software to control the brightness of a backlight then
only needs to support *one* interface.

It is entirely accepted and normal that multiple "class" devices may
appear in userspace for one piece of physical hardware.

Your patch duplicates a userspace API and means any backlight
application would have to look for *two* different interfaces. This is
unacceptable.

Why can't your driver just register a backlight interface and an LCD
interface? I'd imagine your backlight and LCD can be powered on/off
separately too.

Please also cc: the backlight/lcd maintainer (me) on backlight/lcd
patches in future.

Andrew: Can you drop that patch from -mm please as I'm not convinced we
need two backlight brightness interfaces around...

Cheers,

Richard



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: InKi Dae on
Thank you for your comments.

it is a good idea that lcd driver registers interfaces to lcd class
and backlight class.
but I think for AMOLED LCD Panel, backlight couldn't be real device so
power on/off
for backlight device doesn't make sense.

I had tried lcd driver registers interfaces to lcd class and backlight
class as you said.
and also backlight fake driver as Pavel said.

but I thought to use my patch is more simple and effective.
of course, this patch has a problem that it has different path from
backlight class
for controlling brightness.

how about creating symbolic link file for competiability?

Best Regards,
InKi Dae.

2009/11/10 Richard Purdie <rpurdie(a)linux.intel.com>:
> On Mon, 2009-11-02 at 17:50 +0900, InKi Dae wrote:
>> This patch adds brightness feature to lcd class.
>> (kernel/driver/video/backlight/lcd.c)
>>
>> In the past, most of the lcd panels for embedded system was TFT-LCD
>> Panel needing backlight device.
>> But now AMOLED LCD Panel appeared so we should consider brightness
>> control for AMOLED Panel.
>>
>> For the time being, I used backlight fake driver for brightness
>> control of AMOLED LCD Panel.
>> But this way is not good, so I propose to add brightness feature to lcd class.
>>
>> For this, I attached patch file and if my proposal is approved
>> Then I will send s6e63m0 and tl2796 AMOLED lcd panel driver based on
>> lcd class modified soon.
>
> Pavel has a good point but let me try and explain it differently.
>
> The point of the backlight class is to expose a backlight brightness
> control to userspace in a consistent well defined way. Anyone wishing to
> write a piece of software to control the brightness of a backlight then
> only needs to support *one* interface.
>
> It is entirely accepted and normal that multiple "class" devices may
> appear in userspace for one piece of physical hardware.
>
> Your patch duplicates a userspace API and means any backlight
> application would have to look for *two* different interfaces. This is
> unacceptable.
>
> Why can't your driver just register a backlight interface and an LCD
> interface? I'd imagine your backlight and LCD can be powered on/off
> separately too.
>
> Please also cc: the backlight/lcd maintainer (me) on backlight/lcd
> patches in future.
>
> Andrew: Can you drop that patch from -mm please as I'm not convinced we
> need two backlight brightness interfaces around...
>
> Cheers,
>
> Richard
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Richard Purdie on
On Tue, 2009-11-10 at 12:26 +0900, InKi Dae wrote:
> Thank you for your comments.
>
> it is a good idea that lcd driver registers interfaces to lcd class
> and backlight class.
> but I think for AMOLED LCD Panel, backlight couldn't be real device so
> power on/off
> for backlight device doesn't make sense.

The backlight power control can just turn the backlight level down to
its lowest setting (off)?

> I had tried lcd driver registers interfaces to lcd class and backlight
> class as you said.
> and also backlight fake driver as Pavel said.

So you've tried this, what was the problem? Perhaps post this driver
code to illustrate your problem?

> but I thought to use my patch is more simple and effective.
> of course, this patch has a problem that it has different path from
> backlight class
> for controlling brightness.
>
> how about creating symbolic link file for competiability?

Creating two ways to do something (with or without symlinks) is not
simple and effective for anything other than your driver.

Cheers,

Richard


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Matthew Garrett on
Is the confusion here just that LED displays aren't backlit in the
traditional sense of the term?

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/