On Mon, Jun 1, 2015 at 10:25 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> acpi_video_unregister() not only unregisters the acpi-video backlight
> interface but also unregisters the acpi video bus event listener, causing
> e.g. brightness hotkey presses to no longer generate keypress events.
>
> The unregistering of the acpi video bus event listener usually is
> undesirable, which by itself is a good reason to switch to
> acpi_video_unregister_backlight().
>
> Another problem with using acpi_video_unregister() rather then using
> acpi_video_unregister_backlight() is that on systems with an intel video
> opregion (most systems) and a broken_acpi_video quirk, whether or not
> the acpi video bus event listener actually gets unregistered depends on
> module load ordering:
>
> Scenario a:
> 1) acpi/video.ko gets loaded (*), does not do acpi_video_register as there
> is an intel opregion.
> 2) intel.ko gets loaded, calls acpi_video_register() which registers both
> the listener and the acpi backlight interface
> 3) samsung-laptop.ko gets loaded, calls acpi_video_unregister() causing
> both the listener and the acpi backlight interface to unregister
>
> Scenario b:
> 1) acpi/video.ko gets loaded (*), does not do acpi_video_register as there
> is an intel opregion.
> 2) samsung-laptop.ko gets loaded, calls acpi_video_dmi_promote_vendor(),
> calls acpi_video_unregister(), which is a nop since acpi_video_register
> has not yet been called
> 2) intel.ko gets loaded, calls acpi_video_register() which registers
> the listener, but does not register the acpi backlight interface due to
> the call to the preciding call to acpi_video_dmi_promote_vendor()
>
> *) acpi/video.ko always loads first as both other modules depend on it.
>
> So we end up with or without an acpi video bus event listener depending
> on module load ordering, not good.
>
> Switching to using acpi_video_unregister_backlight() means that independ
> of ordering we will always have an acpi video bus event listener fixing
> this.
>
> Note that this commit means that systems without an intel video opregion,
> and systems which were hitting scenario a wrt module load ordering, are
> now getting an acpi video bus event listener while before they were not!
>
> On some systems this may cause the brightness hotkeys to start generating
> keypresses while before they were not (good), while on other systems this
> may cause the brightness hotkeys to generate multiple keypress events for
> a single press (not so good). Since on most systems the acpi video bus is
> the canonical source for brightness events I believe that the latter case
> will needs to be handled on a case by case basis by filtering out the
> duplicate keypresses at the other source for them.
>
> Cc: Corentin Chary <corentin.chary@xxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/platform/x86/samsung-laptop.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index 9e701b2..0df03e2 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -1730,14 +1730,14 @@ static int __init samsung_init(void)
> samsung->handle_backlight = false;
> } else if (samsung->quirks->broken_acpi_video) {
> pr_info("Disabling ACPI video driver\n");
> - acpi_video_unregister();
> + acpi_video_unregister_backlight();
> }
>
> if (samsung->quirks->use_native_backlight) {
> pr_info("Using native backlight driver\n");
> /* Tell acpi-video to not handle the backlight */
> acpi_video_dmi_promote_vendor();
> - acpi_video_unregister();
> + acpi_video_unregister_backlight();
> /* And also do not handle it ourselves */
> samsung->handle_backlight = false;
> }
> --
> 2.4.2
>
Acked-by: Corentin Chary <corentin.chary@xxxxxxxxx)
--
Corentin Chary
http://xf.iksaif.net
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
