Re: [PATCH 01/20] drm/amdgpu: Introduce GEM object functions

From: "Christian König" <christian.koenig@amd.com>
To: Thomas Zimmermann <tzimmermann@suse.de>,
	alexander.deucher@amd.com, airlied@linux.ie, daniel@ffwll.ch,
	linux@armlinux.org.uk, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, l.stach@pengutronix.de,
	christian.gmeiner@gmail.com, inki.dae@samsung.com,
	jy0922.shim@samsung.com, sw0312.kim@samsung.com,
	kyungmin.park@samsung.com, kgene@kernel.org, krzk@kernel.org,
	patrik.r.jakobsson@gmail.com, jani.nikula@linux.intel.com,
	joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
	chunkuang.hu@kernel.org, p.zabel@pengutronix.de,
	matthias.bgg@gmail.com, robdclark@gmail.com, sean@poorly.run,
	bskeggs@redhat.com, tomi.valkeinen@ti.com, eric@anholt.net,
	hjc@rock-chips.com, heiko@sntech.de, thierry.reding@gmail.com,
	jonathanh@nvidia.com, rodrigosiqueiramelo@gmail.com,
	hamohammed.sa@gmail.com, oleksandr_andrushchenko@epam.com,
	hyun.kwon@xilinx.com, laurent.pinchart@ideasonboard.com,
	michal.simek@xilinx.com, sumit.semwal@linaro.org,
	evan.quan@amd.com, Hawking.Zhang@amd.com, tianci.yin@amd.com,
	marek.olsak@amd.com, hdegoede@redhat.com,
	andrey.grodzovsky@amd.com, Felix.Kuehling@amd.com,
	xinhui.pan@amd.com, aaron.liu@amd.com, nirmoy.das@amd.com,
	chris@chris-wilson.co.uk, matthew.auld@intel.com,
	abdiel.janulgue@linux.intel.com, tvrtko.ursulin@linux.intel.com,
	andi.shyti@intel.com, sam@ravnborg.org, miaoqinglang@huawei.com,
	emil.velikov@collabora.com
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	etnaviv@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org,
	intel-gfx@lists.freedesktop.org,
	linux-mediatek@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	nouveau@lists.freedesktop.org,
	linux-rockchip@lists.infradead.org, linux-tegra@vger.kernel.org,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 01/20] drm/amdgpu: Introduce GEM object functions
Date: Thu, 13 Aug 2020 12:22:52 +0200	[thread overview]
Message-ID: <5c1b3cab-1898-46df-2c5c-23ab6cbfbb7a@amd.com> (raw)
In-Reply-To: <20200813083644.31711-2-tzimmermann@suse.de>

Am 13.08.20 um 10:36 schrieb Thomas Zimmermann:
> GEM object functions deprecate several similar callback interfaces in
> struct drm_driver. This patch replaces the per-driver callbacks with
> per-instance callbacks in amdgpu. The only exception is gem_prime_mmap,
> which is non-trivial to convert.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  6 ------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++++++++
>   2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 81a79760ca61..51525b8774c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = {
>   	.lastclose = amdgpu_driver_lastclose_kms,
>   	.irq_handler = amdgpu_irq_handler,
>   	.ioctls = amdgpu_ioctls_kms,
> -	.gem_free_object_unlocked = amdgpu_gem_object_free,
> -	.gem_open_object = amdgpu_gem_object_open,
> -	.gem_close_object = amdgpu_gem_object_close,
>   	.dumb_create = amdgpu_mode_dumb_create,
>   	.dumb_map_offset = amdgpu_mode_dumb_mmap,
>   	.fops = &amdgpu_driver_kms_fops,
>   
>   	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>   	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_export = amdgpu_gem_prime_export,
>   	.gem_prime_import = amdgpu_gem_prime_import,
> -	.gem_prime_vmap = amdgpu_gem_prime_vmap,
> -	.gem_prime_vunmap = amdgpu_gem_prime_vunmap,
>   	.gem_prime_mmap = amdgpu_gem_prime_mmap,
>   
>   	.name = DRIVER_NAME,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 43f4966331dd..ca2b79f94e99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -36,6 +36,7 @@
>   #include <drm/amdgpu_drm.h>
>   #include <drm/drm_cache.h>
>   #include "amdgpu.h"
> +#include "amdgpu_dma_buf.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
>   
> @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags)
>   #endif
>   }
>   
> +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
> +	.free = amdgpu_gem_object_free,
> +	.open = amdgpu_gem_object_open,
> +	.close = amdgpu_gem_object_close,
> +	.export = amdgpu_gem_prime_export,
> +	.vmap = amdgpu_gem_prime_vmap,
> +	.vunmap = amdgpu_gem_prime_vunmap,
> +};
> +

Wrong file, this belongs into amdgpu_gem.c

>   static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   			       struct amdgpu_bo_param *bp,
>   			       struct amdgpu_bo **bo_ptr)
> @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>   	if (bo == NULL)
>   		return -ENOMEM;
> +
> +	bo->tbo.base.funcs = &amdgpu_gem_object_funcs;

And this should probably go into amdgpu_gem_object_create().

Apart from that looks like a good idea to me.

Christian.

>   	drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size);
>   	INIT_LIST_HEAD(&bo->shadow_list);
>   	bo->vm_bo = NULL;