[Intel-xe] [PATCH 09/12] drm/xe/gsc: Add an interface for GSC packet submissions

[John Harrison]

On 10/27/2023 15:29, Daniele Ceraolo Spurio wrote:
> Communication with the GSC FW is done via input/output buffers, whose
> addresses are provided via a GSCCS command. The buffers contain a
> generic header and a client-specific packet (e.g. PXP, HDCP); the
> clients don't care about the header format and/or the GSCCS command in
> the batch, they only care about their client-specific header. This patch
> therefore introduces helpers that allow the callers to automatically
> fill in the input header, submit the GSCCS job and decode the output
> header, to make it so that the caller only needs to worry about their
> client-specific input and output messages.
>
> NOTE: this patch by itself only adds the interface so it does nothing,
> I've kept it separate for review but the plan is to squash it with the
> follow up patch before merge, so that the interface and the user are
> introduced at the same time.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
> Cc: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile                   |   1 +
>   .../gpu/drm/xe/abi/gsc_command_header_abi.h   |  46 +++++
>   .../gpu/drm/xe/instructions/xe_gsc_commands.h |   2 +
>   drivers/gpu/drm/xe/xe_gsc_submit.c            | 170 ++++++++++++++++++
>   drivers/gpu/drm/xe/xe_gsc_submit.h            |  30 ++++
>   5 files changed, 249 insertions(+)
>   create mode 100644 drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
>   create mode 100644 drivers/gpu/drm/xe/xe_gsc_submit.c
>   create mode 100644 drivers/gpu/drm/xe/xe_gsc_submit.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 4a442dcf4d79..876c122ad63c 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -58,6 +58,7 @@ xe-y += xe_bb.o \
>   	xe_force_wake.o \
>   	xe_ggtt.o \
>   	xe_gsc.o \
> +	xe_gsc_submit.o \
>   	xe_gt.o \
>   	xe_gt_clock.o \
>   	xe_gt_debugfs.o \
> diff --git a/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h b/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
> new file mode 100644
> index 000000000000..a4c2646803b5
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _ABI_GSC_COMMAND_HEADER_ABI_H
> +#define _ABI_GSC_COMMAND_HEADER_ABI_H
> +
> +#include <linux/types.h>
> +
> +struct intel_gsc_mtl_header {
Is this all really MTL platform specific? Or is it GSC version specific?

Given that the reset of the driver is moving to be IP version based 
instead of platform name based, it seems like this should also be based 
in some kind of version rather than platform name. Or are the GSC 
versions all just relative to the platform anyway?

Apart from that, it all looks good to me.

John.