Received-SPF: pass (google.com: domain of linux-kernel+bounces-177354-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <2db5beba-500b-458e-9fc3-f05bb982172c@redhat.com>
Date: Mon, 13 May 2024 10:39:17 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] ACPI: video: Use vendor backlight on Lenovo X1 Carbon.
To: dengxiang <dengxiang@nfschina.com>, lenb@kernel.org, rafael@kernel.org,
 linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-edac@vger.kernel.org, platform-driver-x86@vger.kernel.org,
 wanghuiqiang@huawei.com, prime.zeng@hisilicon.com, tony.luck@intel.com
References: <20240513033834.9174-1-dengxiang@nfschina.com>
Content-Language: en-US
From: Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20240513033834.9174-1-dengxiang@nfschina.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi dengxiang,


On 5/13/24 5:38 AM, dengxiang wrote:
> Lenovo X1 Carbon advertises both native and vendor backlight
> control interfaces.But Linux defaults to picking the native ACPI
> video backlight interface, which will make that the vendor
> zx_backlight interface to not work.

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.
	
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 .

3. Vendor specific backlight interfaces are normally only found on really
old laptops. Since Windows XP laptops typically use the ACPI backlight
interface and since Windows 8 they typically use the GPU's native
backlight driver. So adding a quirk to use a vendor interface in 2024 is
weird. Again can you explain in a lot more detail what is going on here,
but I guess the backlight class device is provided by the driver for the
integrated graphics and in that case the fix is to simply change the type
of the backlight device registered by the igfx driver to BACKLIGHT_RAW.

4. You posted the same patch twice ?

Regards,

Hans


> 
> Add a DMI quirk to force use of the vendor interface.
> 
> Signed-off-by: dengxiang <dengxiang@nfschina.com>
> ---
>  drivers/acpi/video_detect.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 2cc3821b2b16..e647186b4e83 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -875,6 +875,14 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
>  		},
>  	},
> +	{
> +	 .callback = video_detect_force_vendor,
> +	 /* Lenovo X1 Carbon */
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "KaiTian"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "KaiTian X1 G1d"),
> +		},
> +	},
>  	{ },
>  };
>