Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

["Pali Rohár" <pali.rohar@gmail.com>]

On Thursday 11 June 2015 11:29:24 Hans de Goede wrote:
> Hi,
> 
> On 11-06-15 11:19, Pali Rohár wrote:
> >On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
> >>Currently we have 2 kernel commandline options + dmi-quirks in 3 places all
> >>interacting (in interesting ways) to select which which backlight interface
> >>to use. On the commandline we've acpi_backlight=[video|vendor] and
> >>video.use_native_backlight=[0|1]. DMI quirks we have in
> >>acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
> >>
> >>This commit is the first step to cleaning this up, replacing the 2 cmdline
> >>options with just acpi_video.backlight=[video|vendor|native|none], and
> >>adding a new API to video_detect.c to reflect this.
> >>
> >>Follow up commits will also move other related code, like unregistering the
> >>acpi_video backlight interface if it was registered before other drivers
> >>which take priority over it are loaded, to video_detect.c where this
> >>logic really belongs.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> >Hello,
> >
> >lot of people are using acpi_backlight parameter in kernel cmdline to
> >fix some of problems. I would like to see this parameter still working
> >and to not break existing configuration. E.g acpi_backlight=vendor to
> >use dell-laptop.ko or thinkpad_acpi.ko backlight.
> 
> I would have like to keep acpi_backlight= for that exact same reason,
> but that is not possible while keeping acpi_video as a module.
> 
> Thinking more about this, this is not strictly true, we could make
> some other (core) part of the acpi code use __setup to get the
> acpi_backlight= value and have that part export the value for use
> in the acpi_video module. This is not really pretty, but I think it
> may be the best way to solve this.
> 

This solution should work. It is not nice, but current mess of backlight
drivers, acpi, etc, is not nice too. It will help users who already know
(or read on internet/forums/discussions) acpi_backlight= parameter...

> >It is still nightmare to get laptop panel backlight working on different
> >(broken) laptops
> 
> Well one reason it is a nightmare is because there are too many
> backlight drivers fighting for control and there is not an easy
> way to tell the kernel only register this one, this is exactly
> what this patch-set is trying to address :)  People may still
> need to use a cmdline option but now there is only one option to
> play with.
> 

It is not only that. Sometimes there are three possible ways how to
control backlight (vendor driver, acpi driver, gpu driver). And all
those driver can work, but some of them works "better".

E.g in my case on Dell Latitude E6440 all three drivers can register
backlight interface and export it to userspace via sysfs.

But the best option for *me* is to use one from dell-laptop.ko because
it has good granularity of levels for controlling backlight. Intel one
has too many levels and does not fit some linear distribution (which I
would like to see). ACPI video.ko sometimes does not work correctly with
KDE4 setting -- sometimes I need to press brightness key more times to
take effect. I do not know if this is problem of KDE or ACPI or
whatever. For me for my current desktop & kernel setup is the best
solution to use dell-laptop.ko with acpi_backlight=vendor param. I do
not have to debug or change anything, just add one param to cmdline.

And I think other users (without programming knowledge) who need
immediately working laptop will use same option as me.

> > and people learnt to try to use acpi_backlight
> > parameter for quit/hot fixing these problems.
> 
> People who need to pass a kernel commandline option really should
> report a bug once they have figured out what option to use.
> 

This is good assumption, but do not forget that there are people who:

1) do not have time to trying find out where to report bug and how
2) do not know how to report bug
3) even do not know that they can do something (like for MS systems)

> Fedora users are getting pretty good at this as the Fedora kernel
> maintainers punt all such bug reports to me and I properly deal
> with them verifying the users solution is sane and then submitting
> a patch with a dmi based quirk for the users laptop upstream.
> 
> > Upgrading kernel (if you
> >remove acpi_backlight parameter) just break it again.
> 
> I think that is actually (partially) a good thing, as said people
> who rely on cmdline workarounds should file bugs, so that we can
> add a quirk. Had the users done so, the quirk would be long in
> place and the changing of the cmdline option name would not be
> an issue for them.
> 
> I realize that this knife cuts both ways, and that this will
> cause unhappy users, as said if it had been possible to not change
> the cmdline option name in a clean way I would have done so.
> 
> If everyone agrees with the solution I've outlined above, we
> can actually make it so as to not break things for users
> who's setup relies on acpi_backlight=foo
> 
> Regards,
> 
> Hans

-- 
Pali Rohár
pali.rohar@gmail.com