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

[Nirujogi, Pratap]

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

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