On 5/15/2025 5:16 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>
---
Changes v1 -> v2:
* Use fixed "isp_dev_hid[ACPI_ID_LEN]" instead of allocating using
devm_kzalloc()
* Instead of "pointer to pointer" use "pointer to array" type arg in
amdgpu_acpi_get_isp4_dev_hid()
* Include error code in the debug statement
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 29 ++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c | 31 ++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index cc26cf1bd843..2aa7e89a190e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1657,10 +1657,12 @@ static inline void
amdgpu_acpi_get_backlight_caps(struct amdgpu_dm_backlight_cap
bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev);
bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);
void amdgpu_choose_low_power_state(struct amdgpu_device *adev);
+int amdgpu_acpi_get_isp4_dev_hid(u8 (*hid)[ACPI_ID_LEN]);
#else
static inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) {
return false; }
static inline bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) {
return false; }
static inline void amdgpu_choose_low_power_state(struct amdgpu_device *adev)
{ }
+static int amdgpu_acpi_get_isp4_dev_hid(u8 (*hid)[ACPI_ID_LEN]) { }
#endif
void amdgpu_register_gpu_instance(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index b7f8f2ff143d..c39d3a09cd04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1551,4 +1551,33 @@ void amdgpu_choose_low_power_state(struct amdgpu_device
*adev)
adev->in_s3 = true;
}
+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;
+}
+
+int amdgpu_acpi_get_isp4_dev_hid(u8 (*hid)[ACPI_ID_LEN])
+{
+ struct acpi_device *acpi_pdev;
+ struct device *pdev;
+
+ pdev = bus_find_device(&platform_bus_type, NULL, isp_sensor_ids,
+ isp_match_acpi_device_ids);
+ if (!pdev)
+ return -EINVAL;
+
+ acpi_pdev = ACPI_COMPANION(pdev);
+ if (acpi_pdev)
+ strscpy(*hid, acpi_device_hid(acpi_pdev));
+
+ put_device(pdev);
+
+ return 0;
+}
+
#endif /* CONFIG_SUSPEND */
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..574880d67009 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,45 @@ 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 int isp_v4_1_1_hw_init(struct amdgpu_isp *isp)
{
struct amdgpu_device *adev = isp->adev;
int idx, int_idx, num_res, r;
+ u8 isp_dev_hid[ACPI_ID_LEN];
I don't think this is going to happen in practice, but because this
isn't initialized you have a case that the ACPI_COMPANION() isn't found
and thus amdgpu_acpi_get_isp4_dev_hid() returns 0 and there is garbage
in isp_dev_hid.
I think it's best for amdgpu_acpi_get_isp4_dev_hid() to check acpi_pdev
for being NULL and return -ENODEV to avoid this risk or isp_dev_hid to
be initialized to NULL.
u64 isp_base;
if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
return -EINVAL;
+ r = amdgpu_acpi_get_isp4_dev_hid(&isp_dev_hid);
+ if (r) {
+ drm_dbg(&adev->ddev, "Invalid isp platform detected (%d)", r);
+ /* allow GPU init to progress */
+ return 0;
+ }
+
+ /* add GPIO resources required for OMNI5C10 sensor */
+ if (!strcmp("OMNI5C10", isp_dev_hid)) {
+ gpiod_add_lookup_table(&isp_gpio_table);
+ gpiod_add_lookup_table(&isp_sensor_gpio_table);
+ }
+
isp_base = adev->rmmio_base;
isp->isp_cell = kcalloc(3, sizeof(struct mfd_cell), GFP_KERNEL);
