Re: [PATCH RFC 3/5] drm/amdgpu: Add DRM cgroup support for AMD devices

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 20.11.18 um 19:58 schrieb Kenny Ho:
> Change-Id: Ib66c44ac1b1c367659e362a2fc05b6fbb3805876
> Signed-off-by: Kenny Ho <Kenny.Ho@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/Makefile         |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  7 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.c | 37 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.h | 19 +++++++++++
>   include/drm/drmcgrp_vendors.h               |  1 +
>   5 files changed, 67 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 138cb787d27e..5cf8048f2d75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -186,4 +186,7 @@ amdgpu-y += $(AMD_DISPLAY_FILES)
>   
>   endif
>   
> +#DRM cgroup controller
> +amdgpu-y += amdgpu_drmcgrp.o
> +
>   obj-$(CONFIG_DRM_AMDGPU)+= amdgpu.o
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 30bc345d6fdf..ad0373f83ed3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -33,6 +33,7 @@
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/amdgpu_drm.h>
> +#include <drm/drm_cgroup.h>
>   #include <linux/vgaarb.h>
>   #include <linux/vga_switcheroo.h>
>   #include <linux/efi.h>
> @@ -2645,6 +2646,12 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   		goto failed;
>   	}
>   
> +	/* TODO:docs */
> +	if (drmcgrp_vendors[amd_drmcgrp_vendor_id] == NULL)
> +		drmcgrp_register_vendor(&amd_drmcgrp_vendor, amd_drmcgrp_vendor_id);
> +
> +	drmcgrp_register_device(adev->ddev, amd_drmcgrp_vendor_id);
> +

Well that is most likely racy because it is possible that multiple 
instances of the driver initialize at the same time.

Better put the call to drmcgrp_register_vendor() into the module init 
section.

Christian.

>   	return 0;
>   
>   failed:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.c
> new file mode 100644
> index 000000000000..ed8aac17769c
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: MIT
> +// Copyright 2018 Advanced Micro Devices, Inc.
> +#include <linux/slab.h>
> +#include <linux/cgroup_drm.h>
> +#include <drm/drm_device.h>
> +#include "amdgpu_drmcgrp.h"
> +
> +struct cftype files[] = {
> +	{ } /* terminate */
> +};
> +
> +struct cftype *drmcgrp_amd_get_cftypes(void)
> +{
> +	return files;
> +}
> +
> +struct drmcgrp_device_resource *amd_drmcgrp_alloc_dev_resource(void)
> +{
> +	struct amd_drmcgrp_dev_resource *a_ddr;
> +
> +	a_ddr = kzalloc(sizeof(struct amd_drmcgrp_dev_resource), GFP_KERNEL);
> +	if (!a_ddr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	return &a_ddr->ddr;
> +}
> +
> +void amd_drmcgrp_free_dev_resource(struct drmcgrp_device_resource *ddr)
> +{
> +	kfree(ddr_amdddr(ddr));
> +}
> +
> +struct drmcgrp_vendor amd_drmcgrp_vendor = {
> +	.get_cftypes = drmcgrp_amd_get_cftypes,
> +	.alloc_dev_resource = amd_drmcgrp_alloc_dev_resource,
> +	.free_dev_resource = amd_drmcgrp_free_dev_resource,
> +};
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.h
> new file mode 100644
> index 000000000000..e2934b7a49f5
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drmcgrp.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + */
> +#ifndef _AMDGPU_DRMCGRP_H
> +#define _AMDGPU_DRMCGRP_H
> +
> +#include <linux/cgroup_drm.h>
> +
> +/* for AMD specific DRM resources */
> +struct amd_drmcgrp_dev_resource {
> +	struct drmcgrp_device_resource ddr;
> +};
> +
> +static inline struct amd_drmcgrp_dev_resource *ddr_amdddr(struct drmcgrp_device_resource *ddr)
> +{
> +	return ddr ? container_of(ddr, struct amd_drmcgrp_dev_resource, ddr) : NULL;
> +}
> +
> +#endif	/* _AMDGPU_DRMCGRP_H */
> diff --git a/include/drm/drmcgrp_vendors.h b/include/drm/drmcgrp_vendors.h
> index b04d8649851b..6cfbf1825344 100644
> --- a/include/drm/drmcgrp_vendors.h
> +++ b/include/drm/drmcgrp_vendors.h
> @@ -3,5 +3,6 @@
>    */
>   #if IS_ENABLED(CONFIG_CGROUP_DRM)
>   
> +DRMCGRP_VENDOR(amd)
>   
>   #endif

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx