Re: [Intel-gfx] [PATCH 1/4] drm/i915/mtl: Define GSC Proxy component interface

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> GSC Proxy component is used for communication between the
> Intel graphics driver and MEI driver.



> diff --git a/include/drm/i915_gsc_proxy_mei_interface.h b/include/drm/i915_gsc_proxy_mei_interface.h
> new file mode 100644
> index 000000000000..e817bb316d5c
> --- /dev/null
> +++ b/include/drm/i915_gsc_proxy_mei_interface.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (c) 2022-2023 Intel Corporation
> + */
> +
> +#ifndef _I915_GSC_PROXY_MEI_INTERFACE_H_
> +#define _I915_GSC_PROXY_MEI_INTERFACE_H_
> +
> +#include <linux/mutex.h>

alan: i notice u have included mutex.h but don't have any mutex use in this header.
side note: looking at at least one other component interfaces (pxp), I see a mutex in the
component struct but don't see it being used at all - a topic for a different series.


> +#include <linux/device.h>
alan: any reason we should include "device.h"? as opposed to just define the ptr type
(since w only reference the ptrs). this could save us a little on build time

> +
> +/**
> + * struct i915_gsc_proxy_component_ops - ops for GSC Proxy services.
> + * @owner: Module providing the ops
> + * @send: sends data through GSC proxy
> + * @recv: receives data through GSC proxy
alan: nit: to be more specific "... from i915 through GSC proxy"

> + */
> +struct i915_gsc_proxy_component_ops {
> +	struct module *owner;
> +
> +	int (*send)(struct device *dev, const void *buf, size_t size);
> +	int (*recv)(struct device *dev, void *buf, size_t size);
> +};
alan: i believe we should have proper documentation on the possible list of
return values for the various error conditions (assuming 0 is success).

> +
> +/**
> + * struct i915_gsc_proxy_component - Used for communication between i915 and
> + * MEI drivers for GSC proxy services
> + * @mei_dev: device that provide the GSC proxy service.
> + * @ops: Ops implemented by GSC proxy driver, used by i915 driver.
> + */
> +struct i915_gsc_proxy_component {
> +	struct device *mei_dev;
> +	const struct i915_gsc_proxy_component_ops *ops;
> +};
> +
> +#endif /* _I915_GSC_PROXY_MEI_INTERFACE_H_ */