[Intel-gfx] [PATCH 6/6] drm/eld: add helpers to modify the SADs of an ELD

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: 07 September 2023 14:58
> To: dri-devel at lists.freedesktop.org
> Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>;
> Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>
> Subject: [PATCH 6/6] drm/eld: add helpers to modify the SADs of an ELD
> 
> Occasionally it's necessary for drivers to modify the SADs of an ELD, but it's
> not so cool to have drivers poke at the ELD buffer directly.
> 
> Using the helpers to translate between 3-byte SAD and struct cea_sad, add
> ELD helpers to get/set the SADs from/to an ELD.
> 
> Cc: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  Documentation/gpu/drm-kms-helpers.rst |  3 ++
>  drivers/gpu/drm/Makefile              |  1 +
>  drivers/gpu/drm/drm_eld.c             | 55 +++++++++++++++++++++++++++
>  include/drm/drm_eld.h                 |  5 +++
>  4 files changed, 64 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_eld.c
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst
> index f0f93aa62545..df91b7cd992e 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -366,6 +366,9 @@ EDID Helper Functions Reference  .. kernel-doc::
> include/drm/drm_eld.h
>     :internal:
> 
> +.. kernel-doc:: drivers/gpu/drm/drm_eld.c
> +   :export:
> +
>  SCDC Helper Functions Reference
>  ===============================
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index
> 215e78e79125..632e74d823e8 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -22,6 +22,7 @@ drm-y := \
>  	drm_drv.o \
>  	drm_dumb_buffers.o \
>  	drm_edid.o \
> +	drm_eld.o \
>  	drm_encoder.o \
>  	drm_file.o \
>  	drm_fourcc.o \
> diff --git a/drivers/gpu/drm/drm_eld.c b/drivers/gpu/drm/drm_eld.c new
> file mode 100644 index 000000000000..34e0d71c3550
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_eld.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_edid.h>
> +#include <drm/drm_eld.h>
> +
> +#include "drm_internal.h"
> +
> +/**
> + * drm_eld_sad_get - get SAD from ELD to struct cea_sad
> + * @eld: ELD buffer
> + * @i: SAD number
> + * @cta_sad: destination struct cea_sad
> + *
> + * @return: 0 on success, or negative on errors  */ int
> +drm_eld_sad_get(const u8 *eld, int i, struct cea_sad *cta_sad) {

Could we use a more descriptive variable name than 'i' for better code readability in the functions drm_eld_sad_get and drm_eld_sad_set?
possibly something like, `sad_number` or `index` ?

Regards,
Mitul
> +	const u8 *sad;
> +
> +	if (i >= drm_eld_sad_count(eld))
> +		return -EINVAL;
> +
> +	sad = eld + DRM_ELD_CEA_SAD(drm_eld_mnl(eld), i);
> +
> +	drm_edid_cta_sad_set(cta_sad, sad);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_eld_sad_get);
> +
> +/**
> + * drm_eld_sad_set - set SAD to ELD from struct cea_sad
> + * @eld: ELD buffer
> + * @i: SAD number
> + * @cta_sad: source struct cea_sad
> + *
> + * @return: 0 on success, or negative on errors  */ int
> +drm_eld_sad_set(u8 *eld, int i, const struct cea_sad *cta_sad) {
> +	u8 *sad;
> +
> +	if (i >= drm_eld_sad_count(eld))
> +		return -EINVAL;
> +
> +	sad = eld + DRM_ELD_CEA_SAD(drm_eld_mnl(eld), i);
> +
> +	drm_edid_cta_sad_get(cta_sad, sad);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_eld_sad_set);
> diff --git a/include/drm/drm_eld.h b/include/drm/drm_eld.h index
> 7b674256b9aa..5b320157684c 100644
> --- a/include/drm/drm_eld.h
> +++ b/include/drm/drm_eld.h
> @@ -8,6 +8,8 @@
> 
>  #include <linux/types.h>
> 
> +struct cea_sad;
> +
>  /* ELD Header Block */
>  #define DRM_ELD_HEADER_BLOCK_SIZE	4
> 
> @@ -75,6 +77,9 @@ static inline int drm_eld_mnl(const u8 *eld)
>  	return (eld[DRM_ELD_CEA_EDID_VER_MNL] &
> DRM_ELD_MNL_MASK) >> DRM_ELD_MNL_SHIFT;  }
> 
> +int drm_eld_sad_get(const u8 *eld, int i, struct cea_sad *cta_sad); int
> +drm_eld_sad_set(u8 *eld, int i, const struct cea_sad *cta_sad);
> +
>  /**
>   * drm_eld_sad - Get ELD SAD structures.
>   * @eld: pointer to an eld memory structure with sad_count set
> --
> 2.39.2