[PATCH] drm/amd/amdgpu: Add GPIO resources required for amdisp

[Mario Limonciello]

On 5/15/2025 12:47 PM, Nirujogi, Pratap wrote:
> Hi Mario,
> 
> On 5/14/2025 6:05 PM, Mario Limonciello wrote:
>> On 5/14/2025 4:35 PM, Pratap Nirujogi wrote:
>>> ISP is a child device to GFX, and its device specific information
>>> is not available in ACPI. Adding the 2 GPIO resources required for
>>> ISP_v4_1_1 in amdgpu_isp driver.
>>>
>>> - GPIO 0 to allow sensor driver to enable and disable sensor module.
>>> - GPIO 85 to allow ISP driver to enable and disable ISP RGB streaming 
>>> mode.
>>>
>>> Signed-off-by: Pratap Nirujogi <pratap.nirujogi at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c | 46 +++++++++++++++++++++++++
>>>   1 file changed, 46 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c b/drivers/gpu/ 
>>> drm/amd/amdgpu/isp_v4_1_1.c
>>> index 69dd92f6e86d..c488af6c8013 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
>>> @@ -25,6 +25,7 @@
>>>    *
>>>    */
>>> +#include <linux/gpio/machine.h>
>>>   #include "amdgpu.h"
>>>   #include "isp_v4_1_1.h"
>>> @@ -39,15 +40,60 @@ static const unsigned int 
>>> isp_4_1_1_int_srcid[MAX_ISP411_INT_SRC] = {
>>>       ISP_4_1__SRCID__ISP_RINGBUFFER_WPT16
>>>   };
>>> +static struct gpiod_lookup_table isp_gpio_table = {
>>> +    .dev_id = "amd_isp_capture",
>>> +    .table = {
>>> +        GPIO_LOOKUP("AMDI0030:00", 85, "enable_isp", GPIO_ACTIVE_HIGH),
>>> +        { }
>>> +    },
>>> +};
>>> +
>>> +static struct gpiod_lookup_table isp_sensor_gpio_table = {
>>> +    .dev_id = "i2c-ov05c10",
>>> +    .table = {
>>> +        GPIO_LOOKUP("amdisp-pinctrl", 0, "enable", GPIO_ACTIVE_HIGH),
>>> +        { }
>>> +    },
>>> +};
>>> +
>>> +static const struct acpi_device_id isp_sensor_ids[] = {
>>> +    { "OMNI5C10" },
>>> +    { }
>>> +};
>>> +
>>> +static int isp_match_acpi_device_ids(struct device *dev, const void 
>>> *data)
>>> +{
>>> +    return acpi_match_device(data, dev) ? 1 : 0;
>>> +}
>>> +
>>>   static int isp_v4_1_1_hw_init(struct amdgpu_isp *isp)
>>>   {
>>>       struct amdgpu_device *adev = isp->adev;
>>> +    struct acpi_device *acpi_pdev;
>>>       int idx, int_idx, num_res, r;
>>> +    struct device *pdev;
>>>       u64 isp_base;
>>>       if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
>>>           return -EINVAL;
>>> +    pdev = bus_find_device(&platform_bus_type, NULL, isp_sensor_ids,
>>> +                   isp_match_acpi_device_ids);
>>> +    if (!pdev) {
>>> +        drm_dbg(&adev->ddev, "Invalid isp platform detected:%ld",
>>> +            PTR_ERR(pdev));
>>> +        /* allow GPU init to progress */
>>> +        return 0;
>>> +    }
>>> +    acpi_pdev = ACPI_COMPANION(pdev);
>>> +
>>> +    /* add GPIO resources required for OMNI5C10 sensor */
>>> +    if (!strcmp("OMNI5C10", acpi_device_hid(acpi_pdev))) {
>>> +        gpiod_add_lookup_table(&isp_gpio_table);
>>> +        gpiod_add_lookup_table(&isp_sensor_gpio_table);
>>> +    }
>>> +    put_device(pdev);
>>> +
>>
>> Can you please move this into a helper in amdgpu_acpi.c?  We try not 
>> to have ACPI code outside of there in case someone compiles without 
>> CONFIG_ACPI.
>>
>> Sorry I should have mentioned this sooner.
>>
> sure, I will add "int amdgpu_acpi_get_isp4_dev_hid(char **hid)", which 
> takes care of:
> 
> 1. finding the matching isp4 platform device(pdev) using bus_find_device()
> 2. gets acpi device handle(acpi_pdev) for the matching pdev and returns 
> valid hid incase of no failures.
> 
> Is this approach okay? hope this function signature makes sense. Please 
> let me know if any comments or suggestions on this approach.
> 
> Thanks,
> Pratap

I think this approach is fine, but it would be good to get Alex's 
comments on this.

> 
>>>       isp_base = adev->rmmio_base;
>>>       isp->isp_cell = kcalloc(3, sizeof(struct mfd_cell), GFP_KERNEL);
>>
>