Re: [PATCH v1] ACPI: video: Switch over to auxiliary bus type

["Nirujogi, Pratap" <pnirujog@amd.com>]

On 3/9/2026 12:01 PM, Rafael J. Wysocki wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Monday, March 9, 2026 1:23:35 PM CET Rafael J. Wysocki wrote:
>> On Mon, Mar 9, 2026 at 12:58 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>
>>> On Mon, Mar 9, 2026 at 9:11 AM Du, Bin <bin.du@amd.com> wrote:
>>>>
>>>>
>>>>
>>>> On 3/9/2026 11:52 AM, Nirujogi, Pratap wrote:
>>>>>
>>>>>
>>>>> On 3/6/2026 7:45 AM, Mario Limonciello wrote:
>>>>>> Caution: This message originated from an External Source. Use proper
>>>>>> caution when opening attachments, clicking links, or responding.
>>>>>>
>>>>>>
>>>>>> On 3/6/26 6:01 AM, Rafael J. Wysocki wrote:
>>>>>>> On Fri, Mar 6, 2026 at 1:35 AM Mario Limonciello (AMD) (kernel.org)
>>>>>>> <superm1@kernel.org> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 3/5/2026 5:11 PM, Nirujogi, Pratap wrote:
>>>>>>>>> Hi Rafael,
>>>>>>>>>
>>>>>>>>> In kernel version 7.0-rc2, the AMDISP device reports the following
>>>>>>>>> errors when created via mfd_add_hotplug_devices.
>>>>>>>>>
>>>>>>>>> [    5.236645] ACPI: video: Video Device [GFX0] (multi-head: yes  rom:
>>>>>>>>> no  post: no)
>>>>>>>>> [    5.236744] input: Video Bus as /devices/
>>>>>>>>> pci0000:00/0000:00:08.1/0000:c3:00.0/amd_isp_capture.1.auto/input/
>>>>>>>>> input21
>>>>>>>>> [    5.236779] acpi device:14: Error installing notify handler
>>>>>>>>> [    5.236782] acpi device:15: Error installing notify handler
>>>>>>>>> [    5.236783] acpi device:16: Error installing notify handler
>>>>>>>>> [    5.236784] acpi device:17: Error installing notify handler
>>>>>>>>> [    5.236785] acpi device:18: Error installing notify handler
>>>>>>>>> [    5.236786] acpi device:19: Error installing notify handler
>>>>>>>>> [    5.236786] acpi device:1a: Error installing notify handler
>>>>>>>>> [    5.236787] acpi device:1b: Error installing notify handler
>>>>>>>>> [    5.236788] acpi device:1c: Error installing notify handler
>>>>>>>>>
>>>>>>>>> These failures indicate AMDISP device is incorrectly detected as ACPI
>>>>>>>>> Video device while it is not.
>>>>>>>>>
>>>>>>>>> The seems like a regression caused by the change that converts the
>>>>>>>>> ACPI
>>>>>>>>> video device to a platform device (commit: 02c057ddefef5).
>>>>>>>>>
>>>>>>>>> Issue is not observed in 6.19-rc6, and also when this change is
>>>>>>>>> reverted
>>>>>>>>> in 7.0-rc2.
>>>>>>>>>
>>>>>>>>> I really appreciate your inputs on addressing this issue and
>>>>>>>>> helping us
>>>>>>>>> make progress on 7.0 rc2.
>>>>>>>>>
>>>>>>>>> Steps followed to reproduce the issue:
>>>>>>>>>
>>>>>>>>> 1. Apply AMDISP v9 patch series [1] on top of kernel v7.0-rc2
>>>>>>>>> 2. Add NULL check for “dev->type” in isp_genpd_add_device() [2] (to
>>>>>>>>> avoid kernel panic found in v7.0-rc2)
>>>>>>>>> 3. Build kernel with:
>>>>>>>>>        - CONFIG_AMD_ISP_PLATFORM=y
>>>>>>>>>        - CONFIG_VIDEO_AMD_ISP4_CAPTURE=m
>>>>>>>>> 4. Install kernel on AMDISP supported system (HP ZBook Ultra G1a)
>>>>>>>>> 5. Boot system to see the failures
>>>>>>>>>
>>>>>>>>> [1] https://lore.kernel.org/all/20260302073020.148277-1-
>>>>>>>>> Bin.Du@amd.com/
>>>>>>>>> [2] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/amd/
>>>>>>>>> amdgpu/isp_v4_1_1.c#L132
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Pratap
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> It's a bit weird to see it even probe in this path in the first place.
>>>>>>>> acpi_video_bus_probe() getting called with the mfd device doesn't seem
>>>>>>>> right to me.
>>>>>>>>
>>>>>>>> I wonder if it's because ISP is an MFD device of GPU (and thus LNXVIDEO
>>>>>>>> ends up matching).
>>>>>>>>
>>>>>>>> Would a sensible solution be to reject mfd device types in
>>>>>>>> acpi_video_bus_probe()?
>>>>>>>
>>>>>>> Every platform device with LNXVIDEO in the device ID list will be
>>>>>>> matched and so probed.
>>>>>>>
>>>>>>> I'm wondering how those devices get that ID though.
>>>>>>
>>>>>> Yeah me too.  I was surprised an MFD device got it.
>>>>>>
>>>>>> Pratap - can you figure this out before we go too far in this path?
>>>>>>
>>>>> yes, MFD child devices in this case have the device ID as that of parent
>>>>> (GPU) i.e. LNXVIDEO. Its because when no acpi_match is specified, the
>>>>> MFD child devices are inheriting the parent's ACPI companion device and
>>>>> this is resulting in having the same parent's ACPI device ID.
>>>>>
>>>>> device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
>>>>> https://github.com/torvalds/linux/blob/master/drivers/mfd/mfd-core.c#L91
>>>>>
>>>>>> If they really shouldn't have LNXVIDEO fixing that may be a better
>>>>>> solution.
>>>>>>
>>>>> AMD ISP related MFD devices are using the same LNXVIDEO device ID even
>>>>> on 6.19-rc4, no issues observed earlier. I can confirm automatic AMD ISP
>>>>> device probe works and also camera works on 7.0-rc2 if I avoid
>>>>> inheriting ACPI companion of the parent (GPU) in the mfd-core.c
>>>>>
>>>>> // device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
>>>>> https://github.com/torvalds/linux/blob/master/drivers/mfd/mfd-core.c#L91
>>>>>
>>>>> But why this is an issue on 7.0-rc2 while it works on 6.19-rc4 needs to
>>>>> be root caused.
>>>>>
>>>>
>>>> Possible cause may be.
>>>> 1. On 6.x (without commit 02c057ddefef5), the ACPI video driver was
>>>> registered as an acpi_driver (static struct acpi_driver acpi_video_bus)
>>>> ,  it lives on the ACPI bus (acpi_bus_type). It only binds to struct
>>>> acpi_device objects in the ACPI namespace. MFD children of GFX
>>>> (amd_isp_capture, amd_isp_i2c_designware, amdisp-pinctrl) are platform
>>>> devices on the platform bus, they are completely invisible to
>>>> acpi_driver, so there is no chance of the ACPI video driver matching an
>>>> MFD child.
>>>> 2. On 7.0 (with commit 02c057ddefef5), the ACPI video driver was
>>>> converted to a platform_driver (static struct platform_driver
>>>> acpi_video_bus), it lives on the platform bus. When the kernel registers
>>>> a new platform device, it checks ALL registered platform drivers to see
>>>> if any match. The matching logic for acpi_match_table works like this:
>>>> - Get the platform device's ACPI companion (ACPI_COMPANION(dev))
>>>> - Check if the companion's HID/CID matches any entry in acpi_match_table
>>>> - If yes, the driver probes the device
>>>
>>> It does, but then the probe fails.  This is the problem that has been
>>> reported to start with.
>>
>> I guess I know what's going on.
>>
>> Every device that shares an ACPI companion with the ACPI video bus
>> device advertises itself as "LNXVIDEO", so udev looks for a module
>> matching that device ID and since the ACPI video driver is loaded, it
>> will not attempt to load anything else.
>>
>> It may be viable to use an auxiliary device for ACPI video bus device
>> representation, let me have a look at that.
> 
> So appended is a patch that works for me.
> 
> Please test it and let me know if it helps.
> 
Thanks Rafael. With your latest patch, acpi_video_bus_probe() is no 
longer called for ISP MFD child devices, which is good.

Tested-by: pratap.nirujogi@amd.com

However, automatic modprobe of ISP MFD devices is still an issue. This 
requires additional changes outside acpi driver (either in amdgpu or 
mfd-core) to fix it. We'll submit a separate patch to address this issue.

Thanks,
Pratap

> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH v1] ACPI: video: Switch over to auxiliary bus type
> 
> Commit 02c057ddefef ("ACPI: video: Convert the driver to a platform one")
> switched over the ACPI video bus driver from an ACPI driver to a platform
> driver, but that change introduced an unwanted and unexpected side effect.
> Namely, on some systems, the ACPI device object of the ACPI video bus
> device is an ACPI companion of multiple platform devices and, after
> adding video_device_ids[] as an acpi_match_table to the acpi_video_bus
> platform driver, all of those devices started to match that driver and
> its probe callback is invoked for all of them (it fails, but it leaves
> confusing message in the log).  Moreover, the MODULE_DEVICE_TABLE()
> of the ACPI video driver module matches all of the devices sharing the
> ACPI companion with the ACPI video bus device, so registering them does
> not cause any other modules to be loaded, so they are only probed
> against the ACPI video bus platform driver.
> 
> To address this, make the core ACPI device enumeration code create an
> auxiliary device for the ACPI video bus device object instead of a
> platform device and switch over the ACPI video bus driver (once again)
> to an auxiliary driver.
> 
> Auxiliary driver generally is a better match for ACPI video bus than
> platform driver, among other things because the ACPI video bus device
> does not require any resources to be allocated for it during
> enumeration.  It also allows the ACPI video bus driver to stop abusing
> device matching based on ACPI device IDs and it allows a special case
> to be dropped from acpi_create_platform_device() because that function
> need not worry about the ACPI video bus device any more.
> 
> Fixes: 02c057ddefef ("ACPI: video: Convert the driver to a platform one")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/acpi_platform.c |    2 -
>   drivers/acpi/acpi_video.c    |   45 ++++++++++++++++++++---------------------
>   drivers/acpi/scan.c          |   47 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 70 insertions(+), 24 deletions(-)
> 
> --- a/drivers/acpi/acpi_platform.c
> +++ b/drivers/acpi/acpi_platform.c
> @@ -135,7 +135,7 @@ struct platform_device *acpi_create_plat
>                  }
>          }
> 
> -       if (adev->device_type == ACPI_BUS_TYPE_DEVICE && !adev->pnp.type.backlight) {
> +       if (adev->device_type == ACPI_BUS_TYPE_DEVICE) {
>                  LIST_HEAD(resource_list);
> 
>                  count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -9,6 +9,7 @@
> 
>   #define pr_fmt(fmt) "ACPI: video: " fmt
> 
> +#include <linux/auxiliary_bus.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/init.h>
> @@ -21,7 +22,6 @@
>   #include <linux/sort.h>
>   #include <linux/pci.h>
>   #include <linux/pci_ids.h>
> -#include <linux/platform_device.h>
>   #include <linux/slab.h>
>   #include <linux/dmi.h>
>   #include <linux/suspend.h>
> @@ -77,8 +77,9 @@ static int register_count;
>   static DEFINE_MUTEX(register_count_mutex);
>   static DEFINE_MUTEX(video_list_lock);
>   static LIST_HEAD(video_bus_head);
> -static int acpi_video_bus_probe(struct platform_device *pdev);
> -static void acpi_video_bus_remove(struct platform_device *pdev);
> +static int acpi_video_bus_probe(struct auxiliary_device *aux_dev,
> +                               const struct auxiliary_device_id *id);
> +static void acpi_video_bus_remove(struct auxiliary_device *aux);
>   static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
> 
>   /*
> @@ -93,19 +94,16 @@ enum acpi_video_level_idx {
>          ACPI_VIDEO_FIRST_LEVEL,         /* actual supported levels begin here */
>   };
> 
> -static const struct acpi_device_id video_device_ids[] = {
> -       {ACPI_VIDEO_HID, 0},
> -       {"", 0},
> +static const struct auxiliary_device_id video_bus_auxiliary_id_table[] = {
> +       { .name = "acpi.video_bus" },
> +       {},
>   };
> -MODULE_DEVICE_TABLE(acpi, video_device_ids);
> +MODULE_DEVICE_TABLE(auxiliary, video_bus_auxiliary_id_table);
> 
> -static struct platform_driver acpi_video_bus = {
> +static struct auxiliary_driver acpi_video_bus = {
>          .probe = acpi_video_bus_probe,
>          .remove = acpi_video_bus_remove,
> -       .driver = {
> -               .name = "acpi-video",
> -               .acpi_match_table = video_device_ids,
> -       },
> +       .id_table = video_bus_auxiliary_id_table,
>   };
> 
>   struct acpi_video_bus_flags {
> @@ -1885,7 +1883,7 @@ static void acpi_video_dev_add_notify_ha
>   }
> 
>   static int acpi_video_bus_add_notify_handler(struct acpi_video_bus *video,
> -                                            struct platform_device *pdev)
> +                                            struct device *parent)
>   {
>          struct input_dev *input;
>          struct acpi_video_device *dev;
> @@ -1908,7 +1906,7 @@ static int acpi_video_bus_add_notify_han
>          input->phys = video->phys;
>          input->id.bustype = BUS_HOST;
>          input->id.product = 0x06;
> -       input->dev.parent = &pdev->dev;
> +       input->dev.parent = parent;
>          input->evbit[0] = BIT(EV_KEY);
>          set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
>          set_bit(KEY_VIDEO_NEXT, input->keybit);
> @@ -1980,9 +1978,10 @@ static int acpi_video_bus_put_devices(st
> 
>   static int instance;
> 
> -static int acpi_video_bus_probe(struct platform_device *pdev)
> +static int acpi_video_bus_probe(struct auxiliary_device *aux_dev,
> +                               const struct auxiliary_device_id *id_unused)
>   {
> -       struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> +       struct acpi_device *device = ACPI_COMPANION(&aux_dev->dev);
>          struct acpi_video_bus *video;
>          bool auto_detect;
>          int error;
> @@ -2019,7 +2018,7 @@ static int acpi_video_bus_probe(struct p
>                  instance++;
>          }
> 
> -       platform_set_drvdata(pdev, video);
> +       auxiliary_set_drvdata(aux_dev, video);
> 
>          video->device = device;
>          strscpy(acpi_device_name(device), ACPI_VIDEO_BUS_NAME);
> @@ -2068,7 +2067,7 @@ static int acpi_video_bus_probe(struct p
>              !auto_detect)
>                  acpi_video_bus_register_backlight(video);
> 
> -       error = acpi_video_bus_add_notify_handler(video, pdev);
> +       error = acpi_video_bus_add_notify_handler(video, &aux_dev->dev);
>          if (error)
>                  goto err_del;
> 
> @@ -2096,10 +2095,10 @@ err_free_video:
>          return error;
>   }
> 
> -static void acpi_video_bus_remove(struct platform_device *pdev)
> +static void acpi_video_bus_remove(struct auxiliary_device *aux_dev)
>   {
> -       struct acpi_video_bus *video = platform_get_drvdata(pdev);
> -       struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> +       struct acpi_video_bus *video = auxiliary_get_drvdata(aux_dev);
> +       struct acpi_device *device = ACPI_COMPANION(&aux_dev->dev);
> 
>          acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
>                                         acpi_video_bus_notify);
> @@ -2163,7 +2162,7 @@ int acpi_video_register(void)
> 
>          dmi_check_system(video_dmi_table);
> 
> -       ret = platform_driver_register(&acpi_video_bus);
> +       ret = auxiliary_driver_register(&acpi_video_bus);
>          if (ret)
>                  goto leave;
> 
> @@ -2183,7 +2182,7 @@ void acpi_video_unregister(void)
>   {
>          mutex_lock(&register_count_mutex);
>          if (register_count) {
> -               platform_driver_unregister(&acpi_video_bus);
> +               auxiliary_driver_unregister(&acpi_video_bus);
>                  register_count = 0;
>                  may_report_brightness_keys = false;
>          }
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -6,6 +6,7 @@
>   #define pr_fmt(fmt) "ACPI: " fmt
> 
>   #include <linux/async.h>
> +#include <linux/auxiliary_bus.h>
>   #include <linux/module.h>
>   #include <linux/init.h>
>   #include <linux/slab.h>
> @@ -2192,6 +2193,46 @@ static acpi_status acpi_bus_check_add_2(
>          return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p);
>   }
> 
> +static void acpi_video_bus_device_release(struct device *dev)
> +{
> +       struct auxiliary_device *aux_dev = to_auxiliary_dev(dev);
> +
> +       kfree(aux_dev);
> +}
> +
> +static void acpi_create_video_bus_device(struct acpi_device *adev,
> +                                        struct acpi_device *parent)
> +{
> +       struct auxiliary_device *aux_dev;
> +       static unsigned int aux_dev_id;
> +
> +       aux_dev = kzalloc_obj(*aux_dev);
> +       if (!aux_dev)
> +               return;
> +
> +       aux_dev->id = aux_dev_id++;
> +       aux_dev->name = "video_bus";
> +       aux_dev->dev.parent = acpi_get_first_physical_node(parent);
> +       if (!aux_dev->dev.parent)
> +               goto err;
> +
> +       aux_dev->dev.release = acpi_video_bus_device_release;
> +
> +       if (auxiliary_device_init(aux_dev))
> +               goto err;
> +
> +       ACPI_COMPANION_SET(&aux_dev->dev, adev);
> +       if (__auxiliary_device_add(aux_dev, "acpi")) {
> +               auxiliary_device_uninit(aux_dev);
> +               goto err;
> +       }
> +
> +       return;
> +
> +err:
> +       kfree(aux_dev);
> +}
> +
>   struct acpi_scan_system_dev {
>          struct list_head node;
>          struct acpi_device *adev;
> @@ -2229,6 +2270,12 @@ static void acpi_default_enumeration(str
>                          sd->adev = device;
>                          list_add_tail(&sd->node, &acpi_scan_system_dev_list);
>                  }
> +       } else if (device->pnp.type.backlight) {
> +               struct acpi_device *parent;
> +
> +               parent = acpi_dev_parent(device);
> +               if (parent)
> +                       acpi_create_video_bus_device(device, parent);
>          } else {
>                  /* For a regular device object, create a platform device. */
>                  acpi_create_platform_device(device, NULL);
> 
> 
> 
>