* [PATCH v2 0/3] Add ISP driver support in amdgpu
@ 2024-05-09 19:35 Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery Pratap Nirujogi
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Pratap Nirujogi @ 2024-05-09 19:35 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du,
Pratap Nirujogi
Add the isp driver in amdgpu to support ISP device on the APUs that
supports ISP IP block. ISP hw block is used for camera front-end, pre
and post processing operations.
Pratap Nirujogi (3):
drm/amd/amdgpu: Add ISP support to amdgpu_discovery
drm/amd/amdgpu: Add ISP driver support
drm/amd/amdgpu: Enable ISP in amdgpu_discovery
drivers/gpu/drm/amd/amdgpu/Makefile | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 23 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 287 ++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h | 54 ++++
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 1 +
drivers/gpu/drm/amd/include/amd_shared.h | 1 +
9 files changed, 382 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery
2024-05-09 19:35 [PATCH v2 0/3] Add ISP driver support in amdgpu Pratap Nirujogi
@ 2024-05-09 19:35 ` Pratap Nirujogi
2024-05-09 19:55 ` Mario Limonciello
2024-05-09 19:35 ` [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery Pratap Nirujogi
2 siblings, 1 reply; 8+ messages in thread
From: Pratap Nirujogi @ 2024-05-09 19:35 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du,
Pratap Nirujogi
ISP hw block is supported in some of the AMD GPU versions, add support
to discover ISP IP in amdgpu_discovery.
Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 1 +
drivers/gpu/drm/amd/include/amd_shared.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 8bb8b414d511..eb60d28a3a13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -718,6 +718,7 @@ enum amd_hw_ip_block_type {
XGMI_HWIP,
DCI_HWIP,
PCIE_HWIP,
+ ISP_HWIP,
MAX_HWIP
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index ece462f8a324..6586feab8c2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -226,6 +226,7 @@ static int hw_id_map[MAX_HWIP] = {
[DCI_HWIP] = DCI_HWID,
[PCIE_HWIP] = PCIE_HWID,
[VPE_HWIP] = VPE_HWID,
+ [ISP_HWIP] = ISP_HWID,
};
static int amdgpu_discovery_read_binary_from_sysmem(struct amdgpu_device *adev, uint8_t *binary)
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index 36ee9d3d6d9c..8bc2134cdd6b 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -105,6 +105,7 @@ enum amd_ip_block_type {
AMD_IP_BLOCK_TYPE_JPEG,
AMD_IP_BLOCK_TYPE_VPE,
AMD_IP_BLOCK_TYPE_UMSCH_MM,
+ AMD_IP_BLOCK_TYPE_ISP,
AMD_IP_BLOCK_TYPE_NUM,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support
2024-05-09 19:35 [PATCH v2 0/3] Add ISP driver support in amdgpu Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery Pratap Nirujogi
@ 2024-05-09 19:35 ` Pratap Nirujogi
2024-05-09 19:55 ` Mario Limonciello
2024-05-09 19:35 ` [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery Pratap Nirujogi
2 siblings, 1 reply; 8+ messages in thread
From: Pratap Nirujogi @ 2024-05-09 19:35 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du,
Pratap Nirujogi
Add the isp driver in amdgpu to support ISP device on the APUs that
supports ISP IP block. ISP hw block is used for camera front-end, pre
and post processing operations.
Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
Change-Id: I67ef206e5eca1fe74e495c3262746be495e17d09
---
Changes in v2:
- Remove adding IORESOURCE_IRQ
- Remove noisy info prints
- Avoid isp version numbers while naming
- Fix incorrect argument description
- Replace // with /* */ for comemnts
drivers/gpu/drm/amd/amdgpu/Makefile | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 287 ++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h | 54 ++++
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 1 +
7 files changed, 357 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index de7b76327f5b..12ba76025cb7 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -324,4 +324,7 @@ amdgpu-y += $(AMD_DISPLAY_FILES)
endif
+# add isp block
+amdgpu-y += amdgpu_isp.o
+
obj-$(CONFIG_DRM_AMDGPU)+= amdgpu.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index eb60d28a3a13..6d7f9ef53269 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -112,6 +112,7 @@
#include "amdgpu_xcp.h"
#include "amdgpu_seq64.h"
#include "amdgpu_reg_state.h"
+#include "amdgpu_isp.h"
#define MAX_GPU_INSTANCE 64
@@ -1045,6 +1046,9 @@ struct amdgpu_device {
/* display related functionality */
struct amdgpu_display_manager dm;
+ /* isp */
+ struct amdgpu_isp isp;
+
/* mes */
bool enable_mes;
bool enable_mes_kiq;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
new file mode 100644
index 000000000000..c28d90c25b10
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
@@ -0,0 +1,287 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ */
+
+#include <linux/irqdomain.h>
+#include <linux/pm_domain.h>
+#include <linux/platform_device.h>
+#include <sound/designware_i2s.h>
+#include <sound/pcm.h>
+#include <linux/acpi.h>
+#include <linux/firmware.h>
+
+#include "amdgpu_smu.h"
+#include "atom.h"
+#include "amdgpu_isp.h"
+#include "smu_internal.h"
+#include "smu_v11_5_ppsmc.h"
+#include "smu_v11_5_pmfw.h"
+
+#define mmDAGB0_WRCLI5_V4_1 0x6811C
+#define mmDAGB0_WRCLI9_V4_1 0x6812C
+#define mmDAGB0_WRCLI10_V4_1 0x68130
+#define mmDAGB0_WRCLI14_V4_1 0x68140
+#define mmDAGB0_WRCLI19_V4_1 0x68154
+#define mmDAGB0_WRCLI20_V4_1 0x68158
+
+static int isp_sw_init(void *handle)
+{
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ adev->isp.parent = adev->dev;
+
+ adev->isp.cgs_device = amdgpu_cgs_create_device(adev);
+ if (!adev->isp.cgs_device)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int isp_sw_fini(void *handle)
+{
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ if (adev->isp.cgs_device)
+ amdgpu_cgs_destroy_device(adev->isp.cgs_device);
+
+ return 0;
+}
+
+/**
+ * isp_hw_init - start and test isp block
+ *
+ * @handle: handle for amdgpu_device pointer
+ *
+ */
+static int isp_hw_init(void *handle)
+{
+ int r;
+ u64 isp_base;
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ const struct amdgpu_ip_block *ip_block =
+ amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_ISP);
+
+ if (!ip_block)
+ return -EINVAL;
+
+ if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
+ return -EINVAL;
+
+ isp_base = adev->rmmio_base;
+
+ adev->isp.isp_cell = kcalloc(1, sizeof(struct mfd_cell), GFP_KERNEL);
+ if (!adev->isp.isp_cell) {
+ r = -ENOMEM;
+ DRM_ERROR("%s: isp mfd cell alloc failed\n", __func__);
+ goto failure;
+ }
+
+ adev->isp.isp_res = kcalloc(1, sizeof(struct resource), GFP_KERNEL);
+ if (!adev->isp.isp_res) {
+ r = -ENOMEM;
+ DRM_ERROR("%s: isp mfd res alloc failed\n", __func__);
+ goto failure;
+ }
+
+ adev->isp.isp_pdata = kzalloc(sizeof(*adev->isp.isp_pdata), GFP_KERNEL);
+ if (!adev->isp.isp_pdata) {
+ r = -ENOMEM;
+ DRM_ERROR("%s: isp platform data alloc failed\n", __func__);
+ goto failure;
+ }
+
+ /* initialize isp platform data */
+ adev->isp.isp_pdata->adev = (void *)adev;
+ adev->isp.isp_pdata->asic_type = adev->asic_type;
+ adev->isp.isp_pdata->base_rmmio_size = adev->rmmio_size;
+
+ adev->isp.isp_res[0].name = "isp_reg";
+ adev->isp.isp_res[0].flags = IORESOURCE_MEM;
+ adev->isp.isp_res[0].start = isp_base;
+ adev->isp.isp_res[0].end = isp_base + ISP_REGS_OFFSET_END;
+
+ adev->isp.isp_cell[0].name = "amd_isp_capture";
+ adev->isp.isp_cell[0].num_resources = 1;
+ adev->isp.isp_cell[0].resources = &adev->isp.isp_res[0];
+ adev->isp.isp_cell[0].platform_data = adev->isp.isp_pdata;
+ adev->isp.isp_cell[0].pdata_size = sizeof(struct isp_platform_data);
+
+ r = mfd_add_hotplug_devices(adev->isp.parent, adev->isp.isp_cell, 1);
+ if (r) {
+ DRM_ERROR("%s: add mfd hotplug device failed\n", __func__);
+ goto failure;
+ }
+
+ /* Temporary WA added to disable MMHUB TLSi until the GART initialization
+ is ready to support MMHUB TLSi and SAW for ISP HW to access GART memory
+ using the TLSi path */
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI5_V4_1 >> 2, 0xFE5FEAA8);
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI9_V4_1 >> 2, 0xFE5FEAA8);
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI10_V4_1 >> 2, 0xFE5FEAA8);
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI14_V4_1 >> 2, 0xFE5FEAA8);
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI19_V4_1 >> 2, 0xFE5FEAA8);
+ cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI20_V4_1 >> 2, 0xFE5FEAA8);
+
+ return 0;
+
+failure:
+
+ kfree(adev->isp.isp_pdata);
+ kfree(adev->isp.isp_res);
+ kfree(adev->isp.isp_cell);
+
+ return r;
+}
+
+/**
+ * isp_hw_fini - stop the hardware block
+ *
+ * @handle: handle for amdgpu_device pointer
+ *
+ */
+static int isp_hw_fini(void *handle)
+{
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ /* remove isp mfd device */
+ mfd_remove_devices(adev->isp.parent);
+
+ kfree(adev->isp.isp_res);
+ kfree(adev->isp.isp_cell);
+ kfree(adev->isp.isp_pdata);
+
+ return 0;
+}
+
+static int isp_suspend(void *handle)
+{
+ return 0;
+}
+
+static int isp_resume(void *handle)
+{
+ return 0;
+}
+
+static int isp_load_fw_by_psp(struct amdgpu_device *adev)
+{
+ const struct common_firmware_header *hdr;
+ char ucode_prefix[30];
+ char fw_name[40];
+ int r = 0;
+
+ /* get isp fw binary name and path */
+ amdgpu_ucode_ip_version_decode(adev, ISP_HWIP, ucode_prefix,
+ sizeof(ucode_prefix));
+ snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
+
+ /* read isp fw */
+ r = amdgpu_ucode_request(adev, &adev->isp.fw, fw_name);
+ if (r) {
+ DRM_ERROR("%s: failed to read isp fw %s\n", __func__, fw_name);
+ amdgpu_ucode_release(&adev->isp.fw);
+ return r;
+ }
+
+ hdr = (const struct common_firmware_header *)adev->isp.fw->data;
+
+ adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].ucode_id =
+ AMDGPU_UCODE_ID_ISP;
+ adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].fw = adev->isp.fw;
+
+ adev->firmware.fw_size +=
+ ALIGN(le32_to_cpu(hdr->ucode_size_bytes), PAGE_SIZE);
+
+ return r;
+}
+
+static int isp_early_init(void *handle)
+{
+ int ret = 0;
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ ret = isp_load_fw_by_psp(adev);
+ if (ret) {
+ DRM_WARN("%s: isp fw load failed %d\n", __func__, ret);
+ /* allow amdgpu init to proceed though isp fw load fails */
+ ret = 0;
+ }
+
+ return ret;
+}
+
+static bool isp_is_idle(void *handle)
+{
+ return true;
+}
+
+static int isp_wait_for_idle(void *handle)
+{
+ return 0;
+}
+
+static int isp_soft_reset(void *handle)
+{
+ return 0;
+}
+
+static int isp_set_clockgating_state(void *handle,
+ enum amd_clockgating_state state)
+{
+ return 0;
+}
+
+static int isp_set_powergating_state(void *handle,
+ enum amd_powergating_state state)
+{
+ return 0;
+}
+
+static const struct amd_ip_funcs isp_ip_funcs = {
+ .name = "isp_ip",
+ .early_init = isp_early_init,
+ .late_init = NULL,
+ .sw_init = isp_sw_init,
+ .sw_fini = isp_sw_fini,
+ .hw_init = isp_hw_init,
+ .hw_fini = isp_hw_fini,
+ .suspend = isp_suspend,
+ .resume = isp_resume,
+ .is_idle = isp_is_idle,
+ .wait_for_idle = isp_wait_for_idle,
+ .soft_reset = isp_soft_reset,
+ .set_clockgating_state = isp_set_clockgating_state,
+ .set_powergating_state = isp_set_powergating_state,
+};
+
+const struct amdgpu_ip_block_version isp_ip_block = {
+ .type = AMD_IP_BLOCK_TYPE_ISP,
+ .major = 4,
+ .minor = 1,
+ .rev = 0,
+ .funcs = &isp_ip_funcs,
+};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
new file mode 100644
index 000000000000..2adcb4b4dc6e
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ */
+
+#ifndef __AMDGPU_ISP_H__
+#define __AMDGPU_ISP_H__
+
+#include <linux/mfd/core.h>
+#include <linux/mfd/core.h>
+
+#define ISP_REGS_OFFSET_END 0x629A4
+
+struct isp_platform_data {
+ void *adev;
+ u32 asic_type;
+ resource_size_t base_rmmio_size;
+};
+
+struct amdgpu_isp {
+ struct device *parent;
+ struct cgs_device *cgs_device;
+ struct mfd_cell *isp_cell;
+ struct resource *isp_res;
+ struct isp_platform_data *isp_pdata;
+ unsigned int harvest_config;
+ const struct firmware *fw;
+};
+
+extern const struct amdgpu_ip_block_version isp_ip_block;
+
+#endif /* __AMDGPU_ISP_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 37820dd03cab..b4bd943a7cc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -2539,6 +2539,9 @@ static int psp_get_fw_type(struct amdgpu_firmware_info *ucode,
case AMDGPU_UCODE_ID_JPEG_RAM:
*type = GFX_FW_TYPE_JPEG_RAM;
break;
+ case AMDGPU_UCODE_ID_ISP:
+ *type = GFX_FW_TYPE_ISP;
+ break;
case AMDGPU_UCODE_ID_MAXIMUM:
default:
return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 75ece8a2f96b..a9de78bb96e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -712,6 +712,8 @@ const char *amdgpu_ucode_name(enum AMDGPU_UCODE_ID ucode_id)
return "RS64_MEC_P2_STACK";
case AMDGPU_UCODE_ID_CP_RS64_MEC_P3_STACK:
return "RS64_MEC_P3_STACK";
+ case AMDGPU_UCODE_ID_ISP:
+ return "ISP";
default:
return "UNKNOWN UCODE";
}
@@ -1411,6 +1413,9 @@ void amdgpu_ucode_ip_version_decode(struct amdgpu_device *adev, int block_type,
case VPE_HWIP:
ip_name = "vpe";
break;
+ case ISP_HWIP:
+ ip_name = "isp";
+ break;
default:
BUG();
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index a3c04f711099..db745ab7b0c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -523,6 +523,7 @@ enum AMDGPU_UCODE_ID {
AMDGPU_UCODE_ID_UMSCH_MM_CMD_BUFFER,
AMDGPU_UCODE_ID_P2S_TABLE,
AMDGPU_UCODE_ID_JPEG_RAM,
+ AMDGPU_UCODE_ID_ISP,
AMDGPU_UCODE_ID_MAXIMUM,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery
2024-05-09 19:35 [PATCH v2 0/3] Add ISP driver support in amdgpu Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support Pratap Nirujogi
@ 2024-05-09 19:35 ` Pratap Nirujogi
2024-05-09 19:56 ` Mario Limonciello
2 siblings, 1 reply; 8+ messages in thread
From: Pratap Nirujogi @ 2024-05-09 19:35 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du,
Pratap Nirujogi
Enable ISP for ISP V4.1.0 and V4.1.1 in amdgpu_discovery.
Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 6586feab8c2c..82d064adaa49 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -107,6 +107,7 @@
#include "jpeg_v5_0_0.h"
#include "amdgpu_vpe.h"
+#include "amdgpu_isp.h"
#define FIRMWARE_IP_DISCOVERY "amdgpu/ip_discovery.bin"
MODULE_FIRMWARE(FIRMWARE_IP_DISCOVERY);
@@ -682,6 +683,10 @@ static void amdgpu_discovery_read_from_harvest_table(struct amdgpu_device *adev,
adev->sdma.sdma_mask &=
~(1U << harvest_info->list[i].number_instance);
break;
+ case ISP_HWID:
+ adev->isp.harvest_config |=
+ ~(1U << harvest_info->list[i].number_instance);
+ break;
default:
break;
}
@@ -2303,6 +2308,20 @@ static int amdgpu_discovery_set_umsch_mm_ip_blocks(struct amdgpu_device *adev)
return 0;
}
+static int amdgpu_discovery_set_isp_ip_blocks(struct amdgpu_device *adev)
+{
+ switch (amdgpu_ip_version(adev, ISP_HWIP, 0)) {
+ case IP_VERSION(4, 1, 0):
+ case IP_VERSION(4, 1, 1):
+ amdgpu_device_ip_block_add(adev, &isp_ip_block);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
{
int r;
@@ -2829,6 +2848,9 @@ int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
if (r)
return r;
+ r = amdgpu_discovery_set_isp_ip_blocks(adev);
+ if (r)
+ return r;
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support
2024-05-09 19:35 ` [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support Pratap Nirujogi
@ 2024-05-09 19:55 ` Mario Limonciello
2024-05-09 22:13 ` Nirujogi, Pratap
0 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2024-05-09 19:55 UTC (permalink / raw)
To: Pratap Nirujogi, amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du
On 5/9/2024 14:35, Pratap Nirujogi wrote:
> Add the isp driver in amdgpu to support ISP device on the APUs that
> supports ISP IP block. ISP hw block is used for camera front-end, pre
> and post processing operations.
>
> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
> Change-Id: I67ef206e5eca1fe74e495c3262746be495e17d09
Ttypically we strip the Change-Id for things that were in Gerrit originally.
> ---
> Changes in v2:
> - Remove adding IORESOURCE_IRQ
> - Remove noisy info prints
> - Avoid isp version numbers while naming
> - Fix incorrect argument description
> - Replace // with /* */ for comemnts
>
> drivers/gpu/drm/amd/amdgpu/Makefile | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 287 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h | 54 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 5 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 1 +
> 7 files changed, 357 insertions(+)
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index de7b76327f5b..12ba76025cb7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -324,4 +324,7 @@ amdgpu-y += $(AMD_DISPLAY_FILES)
>
> endif
>
> +# add isp block
> +amdgpu-y += amdgpu_isp.o
> +
> obj-$(CONFIG_DRM_AMDGPU)+= amdgpu.o
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index eb60d28a3a13..6d7f9ef53269 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -112,6 +112,7 @@
> #include "amdgpu_xcp.h"
> #include "amdgpu_seq64.h"
> #include "amdgpu_reg_state.h"
> +#include "amdgpu_isp.h"
>
> #define MAX_GPU_INSTANCE 64
>
> @@ -1045,6 +1046,9 @@ struct amdgpu_device {
> /* display related functionality */
> struct amdgpu_display_manager dm;
>
> + /* isp */
> + struct amdgpu_isp isp;
> +
> /* mes */
> bool enable_mes;
> bool enable_mes_kiq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> new file mode 100644
> index 000000000000..c28d90c25b10
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> @@ -0,0 +1,287 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + */
> +
> +#include <linux/irqdomain.h>
> +#include <linux/pm_domain.h>
> +#include <linux/platform_device.h>
> +#include <sound/designware_i2s.h>
> +#include <sound/pcm.h>
> +#include <linux/acpi.h>
> +#include <linux/firmware.h>
This list of headers should be sorted alphabetically.
> +
> +#include "amdgpu_smu.h"
> +#include "atom.h"
> +#include "amdgpu_isp.h"
> +#include "smu_internal.h"
> +#include "smu_v11_5_ppsmc.h"
> +#include "smu_v11_5_pmfw.h"
> +
> +#define mmDAGB0_WRCLI5_V4_1 0x6811C
> +#define mmDAGB0_WRCLI9_V4_1 0x6812C
> +#define mmDAGB0_WRCLI10_V4_1 0x68130
> +#define mmDAGB0_WRCLI14_V4_1 0x68140
> +#define mmDAGB0_WRCLI19_V4_1 0x68154
> +#define mmDAGB0_WRCLI20_V4_1 0x68158
> +
> +static int isp_sw_init(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + adev->isp.parent = adev->dev;
> +
> + adev->isp.cgs_device = amdgpu_cgs_create_device(adev);
> + if (!adev->isp.cgs_device)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int isp_sw_fini(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + if (adev->isp.cgs_device)
> + amdgpu_cgs_destroy_device(adev->isp.cgs_device);
> +
> + return 0;
> +}
> +
> +/**
> + * isp_hw_init - start and test isp block
> + *
> + * @handle: handle for amdgpu_device pointer
> + *
> + */
> +static int isp_hw_init(void *handle)
> +{
> + int r;
> + u64 isp_base;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
Spurious new line here
> + const struct amdgpu_ip_block *ip_block =
> + amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_ISP);
> +
> + if (!ip_block)
> + return -EINVAL;
> +
> + if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
> + return -EINVAL;
> +
> + isp_base = adev->rmmio_base;
> +
> + adev->isp.isp_cell = kcalloc(1, sizeof(struct mfd_cell), GFP_KERNEL);
> + if (!adev->isp.isp_cell) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp mfd cell alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + adev->isp.isp_res = kcalloc(1, sizeof(struct resource), GFP_KERNEL);
> + if (!adev->isp.isp_res) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp mfd res alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + adev->isp.isp_pdata = kzalloc(sizeof(*adev->isp.isp_pdata), GFP_KERNEL);
> + if (!adev->isp.isp_pdata) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp platform data alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + /* initialize isp platform data */
> + adev->isp.isp_pdata->adev = (void *)adev;
> + adev->isp.isp_pdata->asic_type = adev->asic_type;
> + adev->isp.isp_pdata->base_rmmio_size = adev->rmmio_size;
> +
> + adev->isp.isp_res[0].name = "isp_reg";
> + adev->isp.isp_res[0].flags = IORESOURCE_MEM;
> + adev->isp.isp_res[0].start = isp_base;
> + adev->isp.isp_res[0].end = isp_base + ISP_REGS_OFFSET_END;
> +
> + adev->isp.isp_cell[0].name = "amd_isp_capture";
> + adev->isp.isp_cell[0].num_resources = 1;
> + adev->isp.isp_cell[0].resources = &adev->isp.isp_res[0];
> + adev->isp.isp_cell[0].platform_data = adev->isp.isp_pdata;
> + adev->isp.isp_cell[0].pdata_size = sizeof(struct isp_platform_data);
> +
> + r = mfd_add_hotplug_devices(adev->isp.parent, adev->isp.isp_cell, 1);
> + if (r) {
> + DRM_ERROR("%s: add mfd hotplug device failed\n", __func__);
> + goto failure;
> + }
> +
> + /* Temporary WA added to disable MMHUB TLSi until the GART initialization
> + is ready to support MMHUB TLSi and SAW for ISP HW to access GART memory
> + using the TLSi path */
Linux "multi-line comment style" is
/*
* Foo the bar
* Bar the foo
*/
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI5_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI9_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI10_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI14_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI19_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI20_V4_1 >> 2, 0xFE5FEAA8);
> +
> + return 0;
> +
> +failure:
> +
> + kfree(adev->isp.isp_pdata);
> + kfree(adev->isp.isp_res);
> + kfree(adev->isp.isp_cell);
> +
> + return r;
> +}
> +
> +/**
> + * isp_hw_fini - stop the hardware block
> + *
> + * @handle: handle for amdgpu_device pointer
> + *
> + */
> +static int isp_hw_fini(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + /* remove isp mfd device */
> + mfd_remove_devices(adev->isp.parent);
> +
> + kfree(adev->isp.isp_res);
> + kfree(adev->isp.isp_cell);
> + kfree(adev->isp.isp_pdata);
> +
> + return 0;
> +}
> +
> +static int isp_suspend(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_resume(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_load_fw_by_psp(struct amdgpu_device *adev)
> +{
> + const struct common_firmware_header *hdr;
> + char ucode_prefix[30];
> + char fw_name[40];
> + int r = 0;
> +
> + /* get isp fw binary name and path */
> + amdgpu_ucode_ip_version_decode(adev, ISP_HWIP, ucode_prefix,
> + sizeof(ucode_prefix));
> + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
> +
> + /* read isp fw */
> + r = amdgpu_ucode_request(adev, &adev->isp.fw, fw_name);
> + if (r) {
> + DRM_ERROR("%s: failed to read isp fw %s\n", __func__, fw_name);
I think you should lose this DRM_ERROR() statement.
when isp_early_init() is called with the file missing you'll see this
error as well as the DRM_WARN() from isp_early_init().
We should show exactly one message for problems like this. Since it's
not fatal I think we should only show the DRM_WARN() message.
> + amdgpu_ucode_release(&adev->isp.fw);
> + return r;
> + }
> +
> + hdr = (const struct common_firmware_header *)adev->isp.fw->data;
> +
> + adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].ucode_id =
> + AMDGPU_UCODE_ID_ISP;
> + adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].fw = adev->isp.fw;
> +
> + adev->firmware.fw_size +=
> + ALIGN(le32_to_cpu(hdr->ucode_size_bytes), PAGE_SIZE);
> +
> + return r;
> +}
> +
> +static int isp_early_init(void *handle)
> +{
> + int ret = 0;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + ret = isp_load_fw_by_psp(adev);
> + if (ret) {
> + DRM_WARN("%s: isp fw load failed %d\n", __func__, ret);
> + /* allow amdgpu init to proceed though isp fw load fails */
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static bool isp_is_idle(void *handle)
> +{
> + return true;
> +}
> +
> +static int isp_wait_for_idle(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_soft_reset(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_set_clockgating_state(void *handle,
> + enum amd_clockgating_state state)
> +{
> + return 0;
> +}
> +
> +static int isp_set_powergating_state(void *handle,
> + enum amd_powergating_state state)
> +{
> + return 0;
> +}
> +
> +static const struct amd_ip_funcs isp_ip_funcs = {
> + .name = "isp_ip",
> + .early_init = isp_early_init,
> + .late_init = NULL,
> + .sw_init = isp_sw_init,
> + .sw_fini = isp_sw_fini,
> + .hw_init = isp_hw_init,
> + .hw_fini = isp_hw_fini,
> + .suspend = isp_suspend,
> + .resume = isp_resume,
> + .is_idle = isp_is_idle,
> + .wait_for_idle = isp_wait_for_idle,
> + .soft_reset = isp_soft_reset,
> + .set_clockgating_state = isp_set_clockgating_state,
> + .set_powergating_state = isp_set_powergating_state,
> +};
> +
> +const struct amdgpu_ip_block_version isp_ip_block = {
> + .type = AMD_IP_BLOCK_TYPE_ISP,
> + .major = 4,
> + .minor = 1,
> + .rev = 0,
> + .funcs = &isp_ip_funcs,
> +};
I think this is the wrong place for this isp_ip_block and also it should
be named differently. Maybe it should be named isp_ip_block_v4_1 and
then in a different file?
Maybe Alex can confirm how he would rather see it done.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
> new file mode 100644
> index 000000000000..2adcb4b4dc6e
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + */
> +
> +#ifndef __AMDGPU_ISP_H__
> +#define __AMDGPU_ISP_H__
> +
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/core.h>
> +
> +#define ISP_REGS_OFFSET_END 0x629A4
> +
> +struct isp_platform_data {
> + void *adev;
> + u32 asic_type;
> + resource_size_t base_rmmio_size;
> +};
> +
> +struct amdgpu_isp {
> + struct device *parent;
> + struct cgs_device *cgs_device;
> + struct mfd_cell *isp_cell;
> + struct resource *isp_res;
> + struct isp_platform_data *isp_pdata;
> + unsigned int harvest_config;
> + const struct firmware *fw;
> +};
> +
> +extern const struct amdgpu_ip_block_version isp_ip_block;
> +
> +#endif /* __AMDGPU_ISP_H__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 37820dd03cab..b4bd943a7cc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -2539,6 +2539,9 @@ static int psp_get_fw_type(struct amdgpu_firmware_info *ucode,
> case AMDGPU_UCODE_ID_JPEG_RAM:
> *type = GFX_FW_TYPE_JPEG_RAM;
> break;
> + case AMDGPU_UCODE_ID_ISP:
> + *type = GFX_FW_TYPE_ISP;
> + break;
> case AMDGPU_UCODE_ID_MAXIMUM:
> default:
> return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 75ece8a2f96b..a9de78bb96e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -712,6 +712,8 @@ const char *amdgpu_ucode_name(enum AMDGPU_UCODE_ID ucode_id)
> return "RS64_MEC_P2_STACK";
> case AMDGPU_UCODE_ID_CP_RS64_MEC_P3_STACK:
> return "RS64_MEC_P3_STACK";
> + case AMDGPU_UCODE_ID_ISP:
> + return "ISP";
> default:
> return "UNKNOWN UCODE";
> }
> @@ -1411,6 +1413,9 @@ void amdgpu_ucode_ip_version_decode(struct amdgpu_device *adev, int block_type,
> case VPE_HWIP:
> ip_name = "vpe";
> break;
> + case ISP_HWIP:
> + ip_name = "isp";
> + break;
> default:
> BUG();
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index a3c04f711099..db745ab7b0c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -523,6 +523,7 @@ enum AMDGPU_UCODE_ID {
> AMDGPU_UCODE_ID_UMSCH_MM_CMD_BUFFER,
> AMDGPU_UCODE_ID_P2S_TABLE,
> AMDGPU_UCODE_ID_JPEG_RAM,
> + AMDGPU_UCODE_ID_ISP,
> AMDGPU_UCODE_ID_MAXIMUM,
> };
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery
2024-05-09 19:35 ` [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery Pratap Nirujogi
@ 2024-05-09 19:55 ` Mario Limonciello
0 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2024-05-09 19:55 UTC (permalink / raw)
To: Pratap Nirujogi, amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du
On 5/9/2024 14:35, Pratap Nirujogi wrote:
> ISP hw block is supported in some of the AMD GPU versions, add support
> to discover ISP IP in amdgpu_discovery.
>
> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 1 +
> drivers/gpu/drm/amd/include/amd_shared.h | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 8bb8b414d511..eb60d28a3a13 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -718,6 +718,7 @@ enum amd_hw_ip_block_type {
> XGMI_HWIP,
> DCI_HWIP,
> PCIE_HWIP,
> + ISP_HWIP,
> MAX_HWIP
> };
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index ece462f8a324..6586feab8c2c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -226,6 +226,7 @@ static int hw_id_map[MAX_HWIP] = {
> [DCI_HWIP] = DCI_HWID,
> [PCIE_HWIP] = PCIE_HWID,
> [VPE_HWIP] = VPE_HWID,
> + [ISP_HWIP] = ISP_HWID,
> };
>
> static int amdgpu_discovery_read_binary_from_sysmem(struct amdgpu_device *adev, uint8_t *binary)
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 36ee9d3d6d9c..8bc2134cdd6b 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -105,6 +105,7 @@ enum amd_ip_block_type {
> AMD_IP_BLOCK_TYPE_JPEG,
> AMD_IP_BLOCK_TYPE_VPE,
> AMD_IP_BLOCK_TYPE_UMSCH_MM,
> + AMD_IP_BLOCK_TYPE_ISP,
> AMD_IP_BLOCK_TYPE_NUM,
> };
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery
2024-05-09 19:35 ` [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery Pratap Nirujogi
@ 2024-05-09 19:56 ` Mario Limonciello
0 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2024-05-09 19:56 UTC (permalink / raw)
To: Pratap Nirujogi, amd-gfx
Cc: alexander.deucher, mlimonci, benjamin.chan, king.li, bin.du
On 5/9/2024 14:35, Pratap Nirujogi wrote:
> Enable ISP for ISP V4.1.0 and V4.1.1 in amdgpu_discovery.
>
> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 22 +++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 6586feab8c2c..82d064adaa49 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -107,6 +107,7 @@
> #include "jpeg_v5_0_0.h"
>
> #include "amdgpu_vpe.h"
> +#include "amdgpu_isp.h"
>
> #define FIRMWARE_IP_DISCOVERY "amdgpu/ip_discovery.bin"
> MODULE_FIRMWARE(FIRMWARE_IP_DISCOVERY);
> @@ -682,6 +683,10 @@ static void amdgpu_discovery_read_from_harvest_table(struct amdgpu_device *adev,
> adev->sdma.sdma_mask &=
> ~(1U << harvest_info->list[i].number_instance);
> break;
> + case ISP_HWID:
> + adev->isp.harvest_config |=
> + ~(1U << harvest_info->list[i].number_instance);
> + break;
> default:
> break;
> }
> @@ -2303,6 +2308,20 @@ static int amdgpu_discovery_set_umsch_mm_ip_blocks(struct amdgpu_device *adev)
> return 0;
> }
>
> +static int amdgpu_discovery_set_isp_ip_blocks(struct amdgpu_device *adev)
> +{
> + switch (amdgpu_ip_version(adev, ISP_HWIP, 0)) {
> + case IP_VERSION(4, 1, 0):
> + case IP_VERSION(4, 1, 1):
> + amdgpu_device_ip_block_add(adev, &isp_ip_block);
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
> {
> int r;
> @@ -2829,6 +2848,9 @@ int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
> if (r)
> return r;
>
> + r = amdgpu_discovery_set_isp_ip_blocks(adev);
> + if (r)
> + return r;
> return 0;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support
2024-05-09 19:55 ` Mario Limonciello
@ 2024-05-09 22:13 ` Nirujogi, Pratap
0 siblings, 0 replies; 8+ messages in thread
From: Nirujogi, Pratap @ 2024-05-09 22:13 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Chan, Benjamin (Koon Pan), Li, King, Du, Bin
[-- Attachment #1: Type: text/plain, Size: 20825 bytes --]
[AMD Official Use Only - General]
Hi Mario / Alex,
Please see inline for below comment, others I addressed and submitted patch v3.
RE: I think this is the wrong place for this isp_ip_block and also it should be named differently. Maybe it should be named isp_ip_block_v4_1 and then in a different file?
Maybe Alex can confirm how he would rather see it done.
>> In amdgpu_isp all we are doing is: (a) load isp fw (b) add isp as MFD device; and anything specific to IP version 4.1.x is taken care in the v4l2 isp driver. We can consider adding private implementation for each ISP ip version in amdgpu if it requires special handling on top of the generic amdgpu_isp + mfd changes. Please let us know your thoughts.
Thanks,
Pratap
-----Original Message-----
From: Limonciello, Mario <Mario.Limonciello@amd.com>
Sent: Thursday, May 9, 2024 3:55 PM
To: Nirujogi, Pratap <Pratap.Nirujogi@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>; Chan, Benjamin (Koon Pan) <Benjamin.Chan@amd.com>; Li, King <King.Li@amd.com>; Du, Bin <Bin.Du@amd.com>
Subject: Re: [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support
On 5/9/2024 14:35, Pratap Nirujogi wrote:
> Add the isp driver in amdgpu to support ISP device on the APUs that
> supports ISP IP block. ISP hw block is used for camera front-end, pre
> and post processing operations.
>
> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com<mailto:pratap.nirujogi@amd.com>>
> Change-Id: I67ef206e5eca1fe74e495c3262746be495e17d09
Ttypically we strip the Change-Id for things that were in Gerrit originally.
> ---
> Changes in v2:
> - Remove adding IORESOURCE_IRQ
> - Remove noisy info prints
> - Avoid isp version numbers while naming
> - Fix incorrect argument description
> - Replace // with /* */ for comemnts
>
> drivers/gpu/drm/amd/amdgpu/Makefile | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c | 287 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h | 54 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 5 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 1 +
> 7 files changed, 357 insertions(+)
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index de7b76327f5b..12ba76025cb7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -324,4 +324,7 @@ amdgpu-y += $(AMD_DISPLAY_FILES)
>
> endif
>
> +# add isp block
> +amdgpu-y += amdgpu_isp.o
> +
> obj-$(CONFIG_DRM_AMDGPU)+= amdgpu.o
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index eb60d28a3a13..6d7f9ef53269 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -112,6 +112,7 @@
> #include "amdgpu_xcp.h"
> #include "amdgpu_seq64.h"
> #include "amdgpu_reg_state.h"
> +#include "amdgpu_isp.h"
>
> #define MAX_GPU_INSTANCE 64
>
> @@ -1045,6 +1046,9 @@ struct amdgpu_device {
> /* display related functionality */
> struct amdgpu_display_manager dm;
>
> + /* isp */
> + struct amdgpu_isp isp;
> +
> /* mes */
> bool enable_mes;
> bool enable_mes_kiq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> new file mode 100644
> index 000000000000..c28d90c25b10
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c
> @@ -0,0 +1,287 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction,
> +including
> + * without limitation the rights to use, copy, modify, merge,
> +publish,
> + * distribute, sub license, and/or sell copies of the Software, and
> +to
> + * permit persons to whom the Software is furnished to do so, subject
> +to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT
> +SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
> +ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> +OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
> +OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including
> +the
> + * next paragraph) shall be included in all copies or substantial
> +portions
> + * of the Software.
> + *
> + */
> +
> +#include <linux/irqdomain.h>
> +#include <linux/pm_domain.h>
> +#include <linux/platform_device.h>
> +#include <sound/designware_i2s.h>
> +#include <sound/pcm.h>
> +#include <linux/acpi.h>
> +#include <linux/firmware.h>
This list of headers should be sorted alphabetically.
> +
> +#include "amdgpu_smu.h"
> +#include "atom.h"
> +#include "amdgpu_isp.h"
> +#include "smu_internal.h"
> +#include "smu_v11_5_ppsmc.h"
> +#include "smu_v11_5_pmfw.h"
> +
> +#define mmDAGB0_WRCLI5_V4_1 0x6811C
> +#define mmDAGB0_WRCLI9_V4_1 0x6812C
> +#define mmDAGB0_WRCLI10_V4_1 0x68130
> +#define mmDAGB0_WRCLI14_V4_1 0x68140
> +#define mmDAGB0_WRCLI19_V4_1 0x68154
> +#define mmDAGB0_WRCLI20_V4_1 0x68158
> +
> +static int isp_sw_init(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + adev->isp.parent = adev->dev;
> +
> + adev->isp.cgs_device = amdgpu_cgs_create_device(adev);
> + if (!adev->isp.cgs_device)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int isp_sw_fini(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + if (adev->isp.cgs_device)
> + amdgpu_cgs_destroy_device(adev->isp.cgs_device);
> +
> + return 0;
> +}
> +
> +/**
> + * isp_hw_init - start and test isp block
> + *
> + * @handle: handle for amdgpu_device pointer
> + *
> + */
> +static int isp_hw_init(void *handle)
> +{
> + int r;
> + u64 isp_base;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
Spurious new line here
> + const struct amdgpu_ip_block *ip_block =
> + amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_ISP);
> +
> + if (!ip_block)
> + return -EINVAL;
> +
> + if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
> + return -EINVAL;
> +
> + isp_base = adev->rmmio_base;
> +
> + adev->isp.isp_cell = kcalloc(1, sizeof(struct mfd_cell), GFP_KERNEL);
> + if (!adev->isp.isp_cell) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp mfd cell alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + adev->isp.isp_res = kcalloc(1, sizeof(struct resource), GFP_KERNEL);
> + if (!adev->isp.isp_res) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp mfd res alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + adev->isp.isp_pdata = kzalloc(sizeof(*adev->isp.isp_pdata), GFP_KERNEL);
> + if (!adev->isp.isp_pdata) {
> + r = -ENOMEM;
> + DRM_ERROR("%s: isp platform data alloc failed\n", __func__);
> + goto failure;
> + }
> +
> + /* initialize isp platform data */
> + adev->isp.isp_pdata->adev = (void *)adev;
> + adev->isp.isp_pdata->asic_type = adev->asic_type;
> + adev->isp.isp_pdata->base_rmmio_size = adev->rmmio_size;
> +
> + adev->isp.isp_res[0].name = "isp_reg";
> + adev->isp.isp_res[0].flags = IORESOURCE_MEM;
> + adev->isp.isp_res[0].start = isp_base;
> + adev->isp.isp_res[0].end = isp_base + ISP_REGS_OFFSET_END;
> +
> + adev->isp.isp_cell[0].name = "amd_isp_capture";
> + adev->isp.isp_cell[0].num_resources = 1;
> + adev->isp.isp_cell[0].resources = &adev->isp.isp_res[0];
> + adev->isp.isp_cell[0].platform_data = adev->isp.isp_pdata;
> + adev->isp.isp_cell[0].pdata_size = sizeof(struct isp_platform_data);
> +
> + r = mfd_add_hotplug_devices(adev->isp.parent, adev->isp.isp_cell, 1);
> + if (r) {
> + DRM_ERROR("%s: add mfd hotplug device failed\n", __func__);
> + goto failure;
> + }
> +
> + /* Temporary WA added to disable MMHUB TLSi until the GART initialization
> + is ready to support MMHUB TLSi and SAW for ISP HW to access GART memory
> + using the TLSi path */
Linux "multi-line comment style" is
/*
* Foo the bar
* Bar the foo
*/
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI5_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI9_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI10_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI14_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI19_V4_1 >> 2, 0xFE5FEAA8);
> + cgs_write_register(adev->isp.cgs_device, mmDAGB0_WRCLI20_V4_1 >> 2,
> +0xFE5FEAA8);
> +
> + return 0;
> +
> +failure:
> +
> + kfree(adev->isp.isp_pdata);
> + kfree(adev->isp.isp_res);
> + kfree(adev->isp.isp_cell);
> +
> + return r;
> +}
> +
> +/**
> + * isp_hw_fini - stop the hardware block
> + *
> + * @handle: handle for amdgpu_device pointer
> + *
> + */
> +static int isp_hw_fini(void *handle)
> +{
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + /* remove isp mfd device */
> + mfd_remove_devices(adev->isp.parent);
> +
> + kfree(adev->isp.isp_res);
> + kfree(adev->isp.isp_cell);
> + kfree(adev->isp.isp_pdata);
> +
> + return 0;
> +}
> +
> +static int isp_suspend(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_resume(void *handle)
> +{
> + return 0;
> +}
> +
> +static int isp_load_fw_by_psp(struct amdgpu_device *adev) {
> + const struct common_firmware_header *hdr;
> + char ucode_prefix[30];
> + char fw_name[40];
> + int r = 0;
> +
> + /* get isp fw binary name and path */
> + amdgpu_ucode_ip_version_decode(adev, ISP_HWIP, ucode_prefix,
> + sizeof(ucode_prefix));
> + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
> +
> + /* read isp fw */
> + r = amdgpu_ucode_request(adev, &adev->isp.fw, fw_name);
> + if (r) {
> + DRM_ERROR("%s: failed to read isp fw %s\n", __func__, fw_name);
I think you should lose this DRM_ERROR() statement.
when isp_early_init() is called with the file missing you'll see this error as well as the DRM_WARN() from isp_early_init().
We should show exactly one message for problems like this. Since it's not fatal I think we should only show the DRM_WARN() message.
> + amdgpu_ucode_release(&adev->isp.fw);
> + return r;
> + }
> +
> + hdr = (const struct common_firmware_header *)adev->isp.fw->data;
> +
> + adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].ucode_id =
> + AMDGPU_UCODE_ID_ISP;
> + adev->firmware.ucode[AMDGPU_UCODE_ID_ISP].fw = adev->isp.fw;
> +
> + adev->firmware.fw_size +=
> + ALIGN(le32_to_cpu(hdr->ucode_size_bytes), PAGE_SIZE);
> +
> + return r;
> +}
> +
> +static int isp_early_init(void *handle) {
> + int ret = 0;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + ret = isp_load_fw_by_psp(adev);
> + if (ret) {
> + DRM_WARN("%s: isp fw load failed %d\n", __func__, ret);
> + /* allow amdgpu init to proceed though isp fw load fails */
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static bool isp_is_idle(void *handle) {
> + return true;
> +}
> +
> +static int isp_wait_for_idle(void *handle) {
> + return 0;
> +}
> +
> +static int isp_soft_reset(void *handle) {
> + return 0;
> +}
> +
> +static int isp_set_clockgating_state(void *handle,
> + enum amd_clockgating_state state) {
> + return 0;
> +}
> +
> +static int isp_set_powergating_state(void *handle,
> + enum amd_powergating_state state) {
> + return 0;
> +}
> +
> +static const struct amd_ip_funcs isp_ip_funcs = {
> + .name = "isp_ip",
> + .early_init = isp_early_init,
> + .late_init = NULL,
> + .sw_init = isp_sw_init,
> + .sw_fini = isp_sw_fini,
> + .hw_init = isp_hw_init,
> + .hw_fini = isp_hw_fini,
> + .suspend = isp_suspend,
> + .resume = isp_resume,
> + .is_idle = isp_is_idle,
> + .wait_for_idle = isp_wait_for_idle,
> + .soft_reset = isp_soft_reset,
> + .set_clockgating_state = isp_set_clockgating_state,
> + .set_powergating_state = isp_set_powergating_state, };
> +
> +const struct amdgpu_ip_block_version isp_ip_block = {
> + .type = AMD_IP_BLOCK_TYPE_ISP,
> + .major = 4,
> + .minor = 1,
> + .rev = 0,
> + .funcs = &isp_ip_funcs,
> +};
I think this is the wrong place for this isp_ip_block and also it should be named differently. Maybe it should be named isp_ip_block_v4_1 and then in a different file?
Maybe Alex can confirm how he would rather see it done.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
> new file mode 100644
> index 000000000000..2adcb4b4dc6e
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_isp.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction,
> +including
> + * without limitation the rights to use, copy, modify, merge,
> +publish,
> + * distribute, sub license, and/or sell copies of the Software, and
> +to
> + * permit persons to whom the Software is furnished to do so, subject
> +to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT
> +SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
> +ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> +OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
> +OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including
> +the
> + * next paragraph) shall be included in all copies or substantial
> +portions
> + * of the Software.
> + *
> + */
> +
> +#ifndef __AMDGPU_ISP_H__
> +#define __AMDGPU_ISP_H__
> +
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/core.h>
> +
> +#define ISP_REGS_OFFSET_END 0x629A4
> +
> +struct isp_platform_data {
> + void *adev;
> + u32 asic_type;
> + resource_size_t base_rmmio_size;
> +};
> +
> +struct amdgpu_isp {
> + struct device *parent;
> + struct cgs_device *cgs_device;
> + struct mfd_cell *isp_cell;
> + struct resource *isp_res;
> + struct isp_platform_data *isp_pdata;
> + unsigned int harvest_config;
> + const struct firmware *fw;
> +};
> +
> +extern const struct amdgpu_ip_block_version isp_ip_block;
> +
> +#endif /* __AMDGPU_ISP_H__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 37820dd03cab..b4bd943a7cc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -2539,6 +2539,9 @@ static int psp_get_fw_type(struct amdgpu_firmware_info *ucode,
> case AMDGPU_UCODE_ID_JPEG_RAM:
> *type = GFX_FW_TYPE_JPEG_RAM;
> break;
> + case AMDGPU_UCODE_ID_ISP:
> + *type = GFX_FW_TYPE_ISP;
> + break;
> case AMDGPU_UCODE_ID_MAXIMUM:
> default:
> return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 75ece8a2f96b..a9de78bb96e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -712,6 +712,8 @@ const char *amdgpu_ucode_name(enum AMDGPU_UCODE_ID ucode_id)
> return "RS64_MEC_P2_STACK";
> case AMDGPU_UCODE_ID_CP_RS64_MEC_P3_STACK:
> return "RS64_MEC_P3_STACK";
> + case AMDGPU_UCODE_ID_ISP:
> + return "ISP";
> default:
> return "UNKNOWN UCODE";
> }
> @@ -1411,6 +1413,9 @@ void amdgpu_ucode_ip_version_decode(struct amdgpu_device *adev, int block_type,
> case VPE_HWIP:
> ip_name = "vpe";
> break;
> + case ISP_HWIP:
> + ip_name = "isp";
> + break;
> default:
> BUG();
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index a3c04f711099..db745ab7b0c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -523,6 +523,7 @@ enum AMDGPU_UCODE_ID {
> AMDGPU_UCODE_ID_UMSCH_MM_CMD_BUFFER,
> AMDGPU_UCODE_ID_P2S_TABLE,
> AMDGPU_UCODE_ID_JPEG_RAM,
> + AMDGPU_UCODE_ID_ISP,
> AMDGPU_UCODE_ID_MAXIMUM,
> };
>
[-- Attachment #2: Type: text/html, Size: 57137 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-09 22:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-09 19:35 [PATCH v2 0/3] Add ISP driver support in amdgpu Pratap Nirujogi
2024-05-09 19:35 ` [PATCH v2 1/3] drm/amd/amdgpu: Add ISP support to amdgpu_discovery Pratap Nirujogi
2024-05-09 19:55 ` Mario Limonciello
2024-05-09 19:35 ` [PATCH v2 2/3] drm/amd/amdgpu: Add ISP driver support Pratap Nirujogi
2024-05-09 19:55 ` Mario Limonciello
2024-05-09 22:13 ` Nirujogi, Pratap
2024-05-09 19:35 ` [PATCH v2 3/3] drm/amd/amdgpu: Enable ISP in amdgpu_discovery Pratap Nirujogi
2024-05-09 19:56 ` Mario Limonciello
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.