Re: [Intel-xe] [PATCH 1/7] drm/xe: Add drm-client infrastructure

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6041 bytes --]

Hi Tejas,

On 31-08-2023 14:35, Tejas Upadhyay wrote:
> Add drm-client infrastructure to record stats of consumption
> done by individual drm client.
>
> Signed-off-by: Tejas Upadhyay<tejas.upadhyay@intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile          |  1 +
>   drivers/gpu/drm/xe/xe_device.c       | 15 +++++++-
>   drivers/gpu/drm/xe/xe_device_types.h |  6 ++++
>   drivers/gpu/drm/xe/xe_drm_client.c   | 52 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_drm_client.h   | 43 +++++++++++++++++++++++
>   5 files changed, 116 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/gpu/drm/xe/xe_drm_client.c
>   create mode 100644 drivers/gpu/drm/xe/xe_drm_client.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 9d2311f8141f..f9c25cb2f890 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -51,6 +51,7 @@ xe-y += xe_bb.o \
>   	xe_device.o \
>   	xe_device_sysfs.o \
>   	xe_dma_buf.o \
> +	xe_drm_client.o \
>   	xe_exec.o \
>   	xe_execlist.o \
>   	xe_exec_queue.o \
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 109aeb25d19c..cf59c7b74eaf 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -18,6 +18,7 @@
>   #include "xe_debugfs.h"
>   #include "xe_display.h"
>   #include "xe_dma_buf.h"
> +#include "xe_drm_client.h"
>   #include "xe_drv.h"
>   #include "xe_exec_queue.h"
>   #include "xe_exec.h"
> @@ -43,13 +44,24 @@ struct lockdep_map xe_device_mem_access_lockdep_map = {
>   
>   static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>   {
> +	struct xe_device *xe = to_xe_device(dev);

Don't see xe_device being used in this patch. Better to make this 
assignment in patch where the xe will be used to reference ttm 
device.[Patch 7]

> +	struct xe_drm_client *client;
>   	struct xe_file *xef;
> +	int ret = -ENOMEM;
>   
>   	xef = kzalloc(sizeof(*xef), GFP_KERNEL);
>   	if (!xef)
> -		return -ENOMEM;
> +		return ret;
> +
> +	client = xe_drm_client_alloc();
> +	if (!client) {
> +		kfree(xef);
> +		return ret;
> +	}
>   
>   	xef->drm = file;
> +	xef->client = client;
> +	xef->xe = xe;
>   
>   	mutex_init(&xef->vm.lock);
>   	xa_init_flags(&xef->vm.xa, XA_FLAGS_ALLOC1);
> @@ -89,6 +101,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>   	xa_destroy(&xef->vm.xa);
>   	mutex_destroy(&xef->vm.lock);
>   
> +	xe_drm_client_put(xef->client);
>   	kfree(xef);
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 750e1f0d3339..d210a535c703 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -462,6 +462,9 @@ struct xe_device {
>    * struct xe_file - file handle for XE driver
>    */
>   struct xe_file {
> +	/** @xe: xe DEVICE **/
> +	struct xe_device *xe;
> +
same comment as above.
>   	/** @drm: base DRM file */
>   	struct drm_file *drm;
>   
> @@ -480,6 +483,9 @@ struct xe_file {
>   		/** @lock: protects file engine state */
>   		struct mutex lock;
>   	} exec_queue;
> +
> +	/** @client: drm client */
> +	struct xe_drm_client *client;
>   };
>   
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
> new file mode 100644
> index 000000000000..ea7993338076
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_print.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include "xe_device_types.h"
> +#include "xe_drm_client.h"
> +
> +/**
> + * xe_drm_client_alloc() - Allocate drm client
> + * @void: No arg
> + *
> + * Allocate drm client struct to track client memory against
> + * same till client life. Call this API whenever new client
> + * has opened xe device.
> + *
> + * Return: pointer to client struct or NULL if cant allocate
> + */
> +struct xe_drm_client *xe_drm_client_alloc(void)
> +{
> +	struct xe_drm_client *client;
> +
> +	client = kzalloc(sizeof(*client), GFP_KERNEL);
> +	if (!client)
> +		return NULL;
> +
> +	kref_init(&client->kref);
> +
> +	return client;
> +}
> +
> +/**
> + * __xe_drm_client_free() - Free client struct
> + * @kref: The reference
> + *
> + * This frees client struct. Call this API when xe device is closed
> + * by drm client.
> + *
> + * Return: void
> + */
> +void __xe_drm_client_free(struct kref *kref)
> +{
> +	struct xe_drm_client *client =
> +		container_of(kref, typeof(*client), kref);
> +
> +	kfree(client);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.h b/drivers/gpu/drm/xe/xe_drm_client.h
> new file mode 100644
> index 000000000000..be097cdf5d12
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_drm_client.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_DRM_CLIENT_H_
> +#define _XE_DRM_CLIENT_H_
> +
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/pid.h>
> +#include <linux/rcupdate.h>
> +#include <linux/sched.h>
> +#include <linux/spinlock.h>
> +
> +struct drm_file;
> +struct drm_printer;
> +
> +struct xe_drm_client {
> +	struct kref kref;
> +	unsigned int id;
> +};
> +
> +	static inline struct xe_drm_client *
> +xe_drm_client_get(struct xe_drm_client *client)
> +{
> +	kref_get(&client->kref);
> +	return client;
> +}
> +
> +void __xe_drm_client_free(struct kref *kref);
> +
> +static inline void xe_drm_client_put(struct xe_drm_client *client)
> +{
> +	kref_put(&client->kref, __xe_drm_client_free);
> +}
> +
> +struct xe_drm_client *xe_drm_client_alloc(void);
> +static inline struct xe_drm_client *
> +xe_drm_client_get(struct xe_drm_client *client);

kref_get/xe_drm_client_getcan be moved to the patch where we are 
introducing |xe_drm_client_add_bo |to maintain refcount.

BR

Himal

> +static inline void xe_drm_client_put(struct xe_drm_client *client);
> +
> +#endif

[-- Attachment #2: Type: text/html, Size: 7020 bytes --]