[PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[Andy Shevchenko]

There are a couple of users that open code functionality of matching
a given handle against ACPI video device IDs. The current approach
duplicates ID table along with the matching code. Consolidate it
under the acpi_match_video_device_handle() helper's hood.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/utils.c | 19 +++++++++++++++++++
 include/linux/acpi.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 3a9773a09e19..4800aba3b99c 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -929,6 +929,25 @@ static int __init acpi_backlight(char *str)
 }
 __setup("acpi_backlight=", acpi_backlight);
 
+static const struct acpi_device_id video_device_ids[] = {
+	{ACPI_VIDEO_HID, 0},
+	{}
+};
+
+/**
+ * acpi_match_video_device_handle - match handle against ACPI video device IDs
+ * @handle: ACPI handle to match
+ *
+ * Return: true when matches, otherwise false.
+ */
+bool acpi_match_video_device_handle(acpi_handle handle)
+{
+	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
+
+	return adev && !acpi_match_device_ids(adev, video_device_ids);
+}
+EXPORT_SYMBOL(acpi_match_video_device_handle);
+
 /**
  * acpi_match_platform_list - Check if the system matches with a given list
  * @plat: pointer to acpi_platform_list table terminated by a NULL entry
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 7b96a8bff6d2..c48e8a0df0cc 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -439,6 +439,8 @@ extern char *wmi_get_acpi_device_uid(const char *guid);
 
 extern char acpi_video_backlight_string[];
 extern long acpi_is_video_device(acpi_handle handle);
+extern bool acpi_match_video_device_handle(acpi_handle handle);
+
 extern int acpi_blacklisted(void);
 extern void acpi_osi_setup(char *str);
 extern bool acpi_osi_is_win8(void);
-- 
2.35.1

[PATCH v1 2/5] ACPI: video: Convert to use acpi_match_video_device_handle() helper

[Andy Shevchenko]

Replace open coded variant of acpi_match_video_device_handle() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/video_detect.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index becc198e4c22..14ddbd821a46 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -59,16 +59,10 @@ static void acpi_video_parse_cmdline(void)
 static acpi_status
 find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
-	struct acpi_device *acpi_dev = acpi_fetch_acpi_dev(handle);
 	long *cap = context;
 	struct pci_dev *dev;
 
-	static const struct acpi_device_id video_ids[] = {
-		{ACPI_VIDEO_HID, 0},
-		{"", 0},
-	};
-
-	if (acpi_dev && !acpi_match_device_ids(acpi_dev, video_ids)) {
+	if (acpi_match_video_device_handle(handle)) {
 		dev = acpi_get_pci_dev(handle);
 		if (!dev)
 			return AE_OK;
-- 
2.35.1

[PATCH v1 3/5] i2c: acpi: Convert to use acpi_match_video_device_handle() helper

[Andy Shevchenko]

Replace open coded variant of acpi_match_video_device_handle() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/i2c-core-acpi.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 08b561f0709d..e6de54dec010 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -128,15 +128,6 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 	return 1;
 }
 
-static const struct acpi_device_id i2c_acpi_ignored_device_ids[] = {
-	/*
-	 * ACPI video acpi_devices, which are handled by the acpi-video driver
-	 * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore these.
-	 */
-	{ ACPI_VIDEO_HID, 0 },
-	{}
-};
-
 static int i2c_acpi_do_lookup(struct acpi_device *adev,
 			      struct i2c_acpi_lookup *lookup)
 {
@@ -150,7 +141,11 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
 	if (!acpi_dev_ready_for_enumeration(adev))
 		return -ENODEV;
 
-	if (acpi_match_device_ids(adev, i2c_acpi_ignored_device_ids) == 0)
+	/*
+	 * ACPI video devices, which are handled by the acpi-video driver,
+	 * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore these.
+	 */
+	if (acpi_match_video_device_handle(adev->handle))
 		return -ENODEV;
 
 	memset(info, 0, sizeof(*info));
-- 
2.35.1

Re: [PATCH v1 3/5] i2c: acpi: Convert to use acpi_match_video_device_handle() helper

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

On Fri, Jul 01, 2022 at 12:28:17AM +0300, Andy Shevchenko wrote:
> Replace open coded variant of acpi_match_video_device_handle() helper.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks like a good improvement to me:

Acked-by: Wolfram Sang <wsa@kernel.org>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v1 4/5] PCI/VGA: Convert to use acpi_match_video_device_handle() helper

[Andy Shevchenko]

Replace open coded variant of acpi_match_video_device_handle() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/vgaarb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index f80b6ec88dc3..47058f70c2c9 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -585,9 +585,7 @@ static bool vga_is_firmware_default(struct pci_dev *pdev)
 static bool vga_arb_integrated_gpu(struct device *dev)
 {
 #if defined(CONFIG_ACPI)
-	struct acpi_device *adev = ACPI_COMPANION(dev);
-
-	return adev && !strcmp(acpi_device_hid(adev), ACPI_VIDEO_HID);
+	return acpi_match_video_device_handle(ACPI_HANDLE(dev));
 #else
 	return false;
 #endif
-- 
2.35.1

[PATCH v1 5/5] platform/x86: thinkpad_acpi: Convert to use acpi_match_video_device_handle() helper

[Andy Shevchenko]

Replace open coded variant of acpi_match_video_device_handle() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 22d4e8633e30..624cb9436522 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -748,9 +748,7 @@ static acpi_status __init tpacpi_acpi_handle_locate_callback(acpi_handle handle,
 			u32 level, void *context, void **return_value)
 {
 	if (!strcmp(context, "video")) {
-		struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
-
-		if (!dev || strcmp(ACPI_VIDEO_HID, acpi_device_hid(dev)))
+		if (!acpi_match_video_device_handle(handle))
 			return AE_OK;
 	}
 
-- 
2.35.1

Re: [PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[Mika Westerberg]

Hi Andy,

On Fri, Jul 01, 2022 at 12:28:15AM +0300, Andy Shevchenko wrote:
>  extern long acpi_is_video_device(acpi_handle handle);
> +extern bool acpi_match_video_device_handle(acpi_handle handle);

I think we can do slightly better here. The only caller of
acpi_is_video_device() is in drivers/acpi/video_detect.c so we can move
it there and make it static (is_video_device()).

Then we can name this one acpi_is_video_device() instead and in addition
make it take struct acpi_device as parameter instead of acpi_handle (I
think we should not use acpi_handles in drivers if possible).

Re: [PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[Rafael J. Wysocki]

On Fri, Jul 1, 2022 at 7:47 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi Andy,
>
> On Fri, Jul 01, 2022 at 12:28:15AM +0300, Andy Shevchenko wrote:
> >  extern long acpi_is_video_device(acpi_handle handle);
> > +extern bool acpi_match_video_device_handle(acpi_handle handle);
>
> I think we can do slightly better here. The only caller of
> acpi_is_video_device() is in drivers/acpi/video_detect.c so we can move
> it there and make it static (is_video_device()).
>
> Then we can name this one acpi_is_video_device() instead and in addition
> make it take struct acpi_device as parameter instead of acpi_handle (I
> think we should not use acpi_handles in drivers if possible).

Agreed.

Re: [PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[Andy Shevchenko]

On Tue, Jul 05, 2022 at 08:40:30PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jul 1, 2022 at 7:47 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Fri, Jul 01, 2022 at 12:28:15AM +0300, Andy Shevchenko wrote:
> > >  extern long acpi_is_video_device(acpi_handle handle);
> > > +extern bool acpi_match_video_device_handle(acpi_handle handle);
> >
> > I think we can do slightly better here. The only caller of
> > acpi_is_video_device() is in drivers/acpi/video_detect.c so we can move
> > it there and make it static (is_video_device()).

AFAICS, the scan.c is user of it as well.

> > Then we can name this one acpi_is_video_device() instead and in addition
> > make it take struct acpi_device as parameter instead of acpi_handle (I
> > think we should not use acpi_handles in drivers if possible).
> 
> Agreed.

Not sure it will help to have acpi device since most of the callers asks for
handle to be checked.

Taking into account above, what we can do is to rename it to

	acpi_handle_is_video_device()

which should be clearer than initial proposal.

Thoughts?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/5] ACPI: utils: Introduce acpi_match_video_device_handle() helper

[Andy Shevchenko]

On Sat, Jul 09, 2022 at 12:27:09AM +0300, Andy Shevchenko wrote:
> On Tue, Jul 05, 2022 at 08:40:30PM +0200, Rafael J. Wysocki wrote:
> > On Fri, Jul 1, 2022 at 7:47 AM Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > On Fri, Jul 01, 2022 at 12:28:15AM +0300, Andy Shevchenko wrote:

...

> > > >  extern long acpi_is_video_device(acpi_handle handle);
> > > > +extern bool acpi_match_video_device_handle(acpi_handle handle);

> > > I think we can do slightly better here. The only caller of
> > > acpi_is_video_device() is in drivers/acpi/video_detect.c so we can move
> > > it there and make it static (is_video_device()).
> 
> AFAICS, the scan.c is user of it as well.
> 
> > > Then we can name this one acpi_is_video_device() instead and in addition
> > > make it take struct acpi_device as parameter instead of acpi_handle (I
> > > think we should not use acpi_handles in drivers if possible).
> > 
> > Agreed.
> 
> Not sure it will help to have acpi device since most of the callers asks for
> handle to be checked.

Actually it's 2:2 now. I'm fine with ACPI device and name like

	acpi_dev_is_video_device()

> Taking into account above, what we can do is to rename it to
> 
> 	acpi_handle_is_video_device()
> 
> which should be clearer than initial proposal.
> 
> Thoughts?

-- 
With Best Regards,
Andy Shevchenko