From: Hans de Goede <hdegoede@redhat.com>
To: dengxiang <dengxiang@nfschina.com>
Cc: lenb@kernel.org, linux-acpi@vger.kernel.org,
linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
platform-driver-x86@vger.kernel.org, prime.zeng@hisilicon.com,
rafael@kernel.org, tony.luck@intel.com, wanghuiqiang@huawei.com
Subject: Re: [PATCH] ACPI: video: Use vendor backlight on Lenovo X1 Carbon.
Date: Wed, 15 May 2024 19:29:16 +0200 [thread overview]
Message-ID: <04271fb5-cca3-4bca-b98e-3fab8a78c724@redhat.com> (raw)
In-Reply-To: <20240515034501.12772-1-dengxiang@nfschina.com>
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
prev parent reply other threads:[~2024-05-15 17:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-13 3:38 [PATCH] ACPI: video: Use vendor backlight on Lenovo X1 Carbon dengxiang
2024-05-13 8:39 ` Hans de Goede
2024-05-15 3:45 ` dengxiang
2024-05-15 17:29 ` Hans de Goede [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=04271fb5-cca3-4bca-b98e-3fab8a78c724@redhat.com \
--to=hdegoede@redhat.com \
--cc=dengxiang@nfschina.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=prime.zeng@hisilicon.com \
--cc=rafael@kernel.org \
--cc=tony.luck@intel.com \
--cc=wanghuiqiang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).