Hi,

On 18-12-15 08:10, Michał Kępień wrote:
>>> To clear things up, here is the current state of affairs:
>>>
>>>    * acpi_backlight=native solves the flickering issue, but doesn't help
>>>      with key event "lagging" after WMI is enabled,
>>>
>>>    * using the patch provided by Hans (or a similar one), the "lagged"
>>>      events can be filtered, leaving key event generation to dell-wmi,
>>>
>>>    * dell-wmi won't generate events unless acpi_backlight=vendor, which
>>>      in turn breaks brightness control due to faulty ASLE requests.
>>>
>>> To break out of this loop, I suggest that:
>>>
>>>    * acpi_backlight should default to "native" for Vostro V131, based on
>>>      a DMI check,
>>
>> Ack.
>>
>>>    * brightness key event generation performed by the ACPI video driver
>>>      should always be suppressed on Vostro V131, because it's faulty due
>>>      to firmware bugs (and correct events will be reported anyway by
>>>      either i8042 or WMI),
>>
>> s/should always be suppressed/should be suppressed by default/ otherwise
>> ack. We can easily do this the same way we currently deal with the
>> disable_backlight_sysfs_if option in acpi_video.c
>>
>>>    * yet another quirk should be added to dell-wmi, so that brightness
>>>      key events are generated not only when acpi_backlight=vendor, but
>>>      also when acpi_backlight=native.
>>
>> Nack, the real problem here is that checking if acpi_backlight!=vendor
>> is the wrong way to check if key event generation should be suppressed.
>
> Well, I'm sure you know better, given that you wrote the code that this
> new patch series fixes :)
>
>> This actually reminds me that I've the following item on my
>> todo list for a while but I've not gotten around to it yet:
>>
>>   -Add an acpi_video_handles_key_presses() helper, and use this where
>>    appropriate (dell-wmi and others).
>>
>> The mean reason for that item on my todo list was to make code doing
>> brigthness key-event suppression more readable. But we can also use
>> it for this case, if we check the new video.report_key_events parameter
>> in the acpi_video_handles_key_presses() helper, and switch dell-wmi
>> over to this helper, things will just work without needing yet another
>> quirk in dell-wmi :)
>>
>> I've written a patch-set implementing this (attached), this obsoletes
>> my previous patch. As before, please test this with:
>>
>> acpi_backlight=native video.report_key_events=1
>>
>> On the kernel cmdline, we can add a patch adding dmi quirks to make
>> those the default later.
>
> I tested the patch series you submitted and it seems to work fine for
> me, given that proper kernel parameters are provided.
>
>> For that patch I'm going to need the dmi strings for your laptop,
>> can you please do:
>>
>> for i in /sys/class/dmi/id/*_*; do echo $i; cat $i; done
>>
>> And then include the output in your next mail ? Feel free to leave
>> out the serial numbers, asset tags and uuids, those are potentially
>> privacy sensitive and I don't need them.
>
> /sys/class/dmi/id/bios_date
> 06/12/2014
> /sys/class/dmi/id/bios_vendor
> Dell Inc.
> /sys/class/dmi/id/bios_version
> A04
> /sys/class/dmi/id/board_name
> 0X3GJK
> /sys/class/dmi/id/board_vendor
> Dell Inc.
> /sys/class/dmi/id/board_version
> A04
> /sys/class/dmi/id/chassis_type
> 8
> /sys/class/dmi/id/chassis_vendor
> Dell Inc.
> /sys/class/dmi/id/chassis_version
> Not Specified
> /sys/class/dmi/id/product_name
> Vostro V131
> /sys/class/dmi/id/product_version
> Not Specified
> /sys/class/dmi/id/sys_vendor
> Dell Inc.

Thanks for both the testing and the dmi info, attached is a patch
which applies on top of the other 4 which should automatically do the
right thing, can you please test this patch with an "empty" kernel
cmdline, once that bit works I'll post the entire set upstream for
merging.

Regards,

Hans