Re: [PATCH v2 13/15] drm/panthor: Allow driver compilation

[Steven Price <steven.price@arm.com>]

On 09/08/2023 17:53, Boris Brezillon wrote:
> Now that all blocks are available, we can add/update Kconfig/Makefile
> files to allow compilation.
> 
> v2:
> - Rename the driver (pancsf -> panthor)
> - Change the license (GPL2 -> MIT + GPL2)
> - Split the driver addition commit
> - Add new dependencies on GPUVA and DRM_SCHED
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/Kconfig          |  2 ++
>  drivers/gpu/drm/Makefile         |  1 +
>  drivers/gpu/drm/panthor/Kconfig  | 16 ++++++++++++++++
>  drivers/gpu/drm/panthor/Makefile | 15 +++++++++++++++
>  4 files changed, 34 insertions(+)
>  create mode 100644 drivers/gpu/drm/panthor/Kconfig
>  create mode 100644 drivers/gpu/drm/panthor/Makefile
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 2a44b9419d4d..bddfbdb2ffee 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -358,6 +358,8 @@ source "drivers/gpu/drm/lima/Kconfig"
>  
>  source "drivers/gpu/drm/panfrost/Kconfig"
>  
> +source "drivers/gpu/drm/panthor/Kconfig"
> +
>  source "drivers/gpu/drm/aspeed/Kconfig"
>  
>  source "drivers/gpu/drm/mcde/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 215e78e79125..0a260727505f 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -188,6 +188,7 @@ obj-$(CONFIG_DRM_TVE200) += tve200/
>  obj-$(CONFIG_DRM_XEN) += xen/
>  obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
>  obj-$(CONFIG_DRM_LIMA)  += lima/
> +obj-$(CONFIG_DRM_PANTHOR) += panthor/
>  obj-$(CONFIG_DRM_PANFROST) += panfrost/

NIT: Here panthor is before panfrost, above (in the kconfig 'source')
they are the other way around. Although both lists seem to be in an
arbitrary order.

>  obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/
>  obj-$(CONFIG_DRM_MCDE) += mcde/
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> new file mode 100644
> index 000000000000..a9d17b1bbb75
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +config DRM_PANTHOR
> +	tristate "Panthor (DRM support for ARM Mali CSF-based GPUs)"
> +	depends on DRM
> +	depends on ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)

This is technically wrong. There are ARM configurations that do select
GENERIC_ATOMIC64 and will cause the "select IOMMU_IO_PGTABLE_LPAE" to
conflict with the depends of that option.

Splitting it onto two lines, like panfrost does, matches the iommu
config and I think is easier to read:

        depends on ARM || ARM64 || COMPILE_TEST
        depends on !GENERIC_ATOMIC64    # for IOMMU_IO_PGTABLE_LPAE

Steve

> +	depends on MMU
> +	select DRM_EXEC
> +	select DRM_SCHED
> +	select IOMMU_SUPPORT
> +	select IOMMU_IO_PGTABLE_LPAE
> +	select DRM_GEM_SHMEM_HELPER
> +	select PM_DEVFREQ
> +	select DEVFREQ_GOV_SIMPLE_ONDEMAND
> +	help
> +	  DRM driver for ARM Mali CSF-based GPUs.

It might be worth expanding this to mention Valhall and/or Mali-Gxxx.

Steve

> diff --git a/drivers/gpu/drm/panthor/Makefile b/drivers/gpu/drm/panthor/Makefile
> new file mode 100644
> index 000000000000..64193a484879
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/Makefile
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +panthor-y := \
> +	panthor_devfreq.o \
> +	panthor_device.o \
> +	panthor_drv.o \
> +	panthor_gem.o \
> +	panthor_gpu.o \
> +	panthor_heap.o \
> +	panthor_heap.o \
> +	panthor_fw.o \
> +	panthor_mmu.o \
> +	panthor_sched.o
> +
> +obj-$(CONFIG_DRM_PANTHOR) += panthor.o