Hi dengxian,
On 5/15/24 5:45 AM, dengxiang wrote:
> Hi Hans,
>
>> A couple of remarks / questions:
>
>> 1. Looking at the strings you match on this is not for a Lenovo X1 Carbon,
>> but rather for a Lenovo Kaitan model ? So it seems that the commit message
>> and the comment for the quirk need some work.
>
> ok, I will add DMI_PRODUCT_VERSION & DMI_BOARD_NAME to make a distinction between X1 Carbon and other kaitian models.
>
>> 2. I have never heard of a zx_backlight interface before and there certainly
>> is no upstream driver providing this. I believe you need to explain what
>> is going on in a bit more detail here and then we can see if this really is
>> the best way to fix this. It seems that these Lenovo Kaitan laptops are
>> using Zhaoxin Kaixian x86 processors with integrate graphics. I would expect
>> the zx_backlight interface to be provided by the driver for the Zhaoxin Kaixian
>> integrated graphics in this case. And if that is the case then the integrated
>> graphics driver should use BACKLIGHT_RAW (aka native) for the backlight type
>> and with that change this quirk should not be necessary .
>
> Yes, zx_backlight interface has been provided by the driver for the Zhaoxin Kaixian integrated graphics. Also use backlight_device_register("zx_backlight",...).
> Strangely enough, X1 Carbon laptops will generate two native acpi_video as belows:
>
> lrwxrwxrwx 1 root root 0 5月 14 16:20 acpi_video0 -> ../../devices/pci0000:00/0000:00:01.0/backlight/acpi_video0
> lrwxrwxrwx 1 root root 0 5月 14 16:20 acpi_video1 -> ../../devices/pci0000:00/0000:00:01.0/backlight/acpi_video1
>
> As you see, it will conflict with the same pci bus, then zx_blacklight interface can't be shown on the path /sys/class/backlight/.
> That is to say, zhaoxin driver contain key code as belows:
> #if DRM_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
> if(acpi_video_get_backlight_type() != acpi_backlight_vendor)
> {
> return ret;
> }
> #endif
>
> If i remove the key code, this laptops will generate two native acpi_video and zx_backlight on the sys backlight patch. Once add acpi_backlight=vendor parameter into kernel cmdline,
> just zx_backlight interface has been left on the sys path, which mean that both acpi_video0 and acpi_video1 interface can not be found.
Ok, so the way this is supposed to work is as follows, there is a single
acpi_video_get_backlight_type() function which all backlight drivers are
supposed to use (and all in tree drivers do use).
This looks like this (simplified a bit, see drivers/acpi/video_detect.c):
enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto_detect)
{
...
/* Use ACPI video if available, except when native should be preferred. */
if ((video_caps & ACPI_VIDEO_BACKLIGHT) &&
!(native_available && prefer_native_over_acpi_video()))
return acpi_backlight_video;
/* Use native if available */
if (native_available)
return acpi_backlight_native;
/* ... long comment explaining this ... */
if (acpi_osi_is_win8())
return acpi_backlight_none;
/* No ACPI video/native (old hw), use vendor specific fw methods. */
return acpi_backlight_vendor;
}
as you can see here acpi_backlight_video is only returned if available
(which it is in this case) *and* there is no native GPU backlight
driver or prefer_native_over_acpi_video() returns false.
Since there is no way for this function to know a native GPU driver
is supported it uses the native parameter passed to it for this,
so native backlight drivers, like the Zhaoxin Kaixian integrated graphics
driver must call a special helper, which internally calls the above
function with native=true. I think not calling that special helper
is why you see the acpi_video backlight devices, assuming you are
using a recent mainline kernel.
So that:
#if DRM_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
{
return ret;
}
#endif
block you quoted should look like this when using recent upstream
kernels:
if (!acpi_video_backlight_use_native())
{
return ret;
}
Although that return ret looks weird, maybe hardcode 0 for success
(not registering is on purpose, so success ?)
Or to keep things compatible with multiple kernel versions use:
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 1, 0)
if (!acpi_video_backlight_use_native())
{
return ret;
}
#elif DRM_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
{
return ret;
}
#endif
Please give this a try, I believe you will not need a quirk
when the Zhaoxin Kaixian integrated graphics driver does
the right thing.
Regards,
Hans
