[PATCH] drm/amdgpu: fix Kconfig for ISP v2

[Limonciello, Mario]

On 5/14/2024 4:28 PM, Alex Deucher wrote:
> Add new config option and set proper dependencies for ISP.
> 
> v2: add missed guards, drop separate Kconfig
> 
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>

I have one optional remark regarding headers, but otherwise it looks 
fine by me.  Feel free to ignore it or squash it in while committing.

Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>

> Cc: Pratap Nirujogi <pratap.nirujogi at amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/Kconfig            | 11 +++++++++++
>   drivers/gpu/drm/amd/amdgpu/Makefile           |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |  6 ++++++
>   4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 22d88f8ef5279..0cd9d29394072 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -70,6 +70,17 @@ config DRM_AMDGPU_USERPTR
>   	  This option selects CONFIG_HMM and CONFIG_HMM_MIRROR if it
>   	  isn't already selected to enabled full userptr support.
>   
> +config DRM_AMD_ISP
> +	bool "Enable AMD Image Signal Processor IP support"
> +	depends on DRM_AMDGPU
> +	select MFD_CORE
> +	select PM_GENERIC_DOMAINS if PM
> +	help
> +	Choose this option to enable ISP IP support for AMD SOCs.
> +	This adds the ISP (Image Signal Processor) IP driver and wires
> +	it up into the amdgpu driver.  It is required for cameras
> +	on APUs which utilize mipi cameras.
> +
>   config DRM_AMDGPU_WERROR
>   	bool "Force the compiler to throw an error instead of a warning when compiling"
>   	depends on DRM_AMDGPU
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 12ba76025cb7c..c95ec19a38264 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -325,6 +325,8 @@ amdgpu-y += $(AMD_DISPLAY_FILES)
>   endif
>   
>   # add isp block
> +ifneq ($(CONFIG_DRM_AMD_ISP),)
>   amdgpu-y += amdgpu_isp.o
> +endif
>   
>   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 846c3550fbda8..936ed3c10c884 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -113,7 +113,9 @@
>   #include "amdgpu_seq64.h"
>   #include "amdgpu_reg_state.h"
>   #include "amdgpu_umsch_mm.h"
> +#if defined(CONFIG_DRM_AMD_ISP)
>   #include "amdgpu_isp.h"
> +#endif
>   
>   #define MAX_GPU_INSTANCE		64
>   
> @@ -1049,8 +1051,10 @@ struct amdgpu_device {
>   	/* display related functionality */
>   	struct amdgpu_display_manager dm;
>   
> +#if defined(CONFIG_DRM_AMD_ISP)
>   	/* isp */
>   	struct amdgpu_isp		isp;
> +#endif
>   
>   	/* mes */
>   	bool                            enable_mes;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 378d5a5cda917..1bab8dd37d621 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -107,7 +107,9 @@
>   #include "jpeg_v5_0_0.h"
>   
>   #include "amdgpu_vpe.h"
> +#if defined(CONFIG_DRM_AMD_ISP)
>   #include "amdgpu_isp.h"
> +#endif

IMO including this header is probably relatively safe no matter if you 
have ISP enabled or not, no?

>   
>   #define FIRMWARE_IP_DISCOVERY "amdgpu/ip_discovery.bin"
>   MODULE_FIRMWARE(FIRMWARE_IP_DISCOVERY);
> @@ -712,10 +714,12 @@ static void amdgpu_discovery_read_from_harvest_table(struct amdgpu_device *adev,
>   			adev->sdma.sdma_mask &=
>   				~(1U << harvest_info->list[i].number_instance);
>   			break;
> +#if defined(CONFIG_DRM_AMD_ISP)
>   		case ISP_HWID:
>   			adev->isp.harvest_config |=
>   				~(1U << harvest_info->list[i].number_instance);
>   			break;
> +#endif
>   		default:
>   			break;
>   		}
> @@ -2402,6 +2406,7 @@ static int amdgpu_discovery_set_umsch_mm_ip_blocks(struct amdgpu_device *adev)
>   
>   static int amdgpu_discovery_set_isp_ip_blocks(struct amdgpu_device *adev)
>   {
> +#if defined(CONFIG_DRM_AMD_ISP)
>   	switch (amdgpu_ip_version(adev, ISP_HWIP, 0)) {
>   	case IP_VERSION(4, 1, 0):
>   	case IP_VERSION(4, 1, 1):
> @@ -2410,6 +2415,7 @@ static int amdgpu_discovery_set_isp_ip_blocks(struct amdgpu_device *adev)
>   	default:
>   		break;
>   	}
> +#endif
>   
>   	return 0;
>   }