[PATCH v8 01/14] drm: Define histogram structures exposed to user

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy at intel.com>
> Sent: Tuesday, January 28, 2025 9:21 PM
> To: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; dri-
> devel at lists.freedesktop.org
> Cc: Kandpal, Suraj <suraj.kandpal at intel.com>; dmitry.baryshkov at linaro.org;
> Murthy, Arun R <arun.r.murthy at intel.com>
> Subject: [PATCH v8 01/14] drm: Define histogram structures exposed to user
> 
> Display Histogram is an array of bins and can be generated in many ways
> referred to as modes.
> Ex: HSV max(RGB), Wighted RGB etc.
> 
> Understanding the histogram data format(Ex: HSV max(RGB)) Histogram is just
> the pixel count.
> For a maximum resolution of 10k (10240 x 4320 = 44236800)
> 25 bits should be sufficient to represent this along with a buffer of 7 bits(future
> use) u32 is being considered.
> max(RGB) can be 255 i.e 0xFF 8 bit, considering the most significant 5 bits,
> hence 32 bins.
> Below mentioned algorithm illustrates the histogram generation in hardware.
> 
> hist[32] = {0};
> for (i = 0; i < resolution; i++) {
> 	bin = max(RGB[i]);
> 	bin = bin >> 3;	/* consider the most significant bits */
> 	hist[bin]++;
> }
> If the entire image is Red color then max(255,0,0) is 255 so the pixel count of
> each pixels will be placed in the last bin. Hence except hist[31] all other bins
> will have a value zero.
> Generated histogram in this case would be hist[32] = {0,0,....44236800}
> 
> Description of the structures, properties defined are documented in the header
> file include/uapi/drm/drm_mode.h
> 
> v8: Added doc for HDR planes, removed reserved variables (Dmitry)
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> ---
>  include/uapi/drm/drm_mode.h | 65
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index
> c082810c08a8b234ef2672ecf54fc8c05ddc2bd3..b8b7b18843ae7224263a9c61b
> 20ac6cbf5df69e9 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -1355,6 +1355,71 @@ struct drm_mode_closefb {
>  	__u32 pad;
>  };
> 
> +/**
> + * enum drm_mode_histogram
> + *
> + * @DRM_MODE_HISTOGRAM_HSV_MAX_RGB:
> + * Maximum resolution at present 10k, 10240x4320 = 44236800
> + * can be denoted in 25bits. With an additional 7 bits in buffer each
> +bin
> + * can be a u32 value.
> + * For SDL, Maximum value of max(RGB) is 255, so max 255 bins.

Type: SDR

> + * If the most significant 5 bits are considered, then bins = 2^5
> + * will be 32 bins.
> + * For HDR, maximum value of max(RGB) is 65535, so max 65535 bins.
> + * For illustration consider a full RED image of 10k resolution
> +considering all
> + * 8 bits histogram would look like hist[255] = {0,0,....44236800} with
> +SDR
> + * plane similarly with HDR the same would look like hist[65535] =
> + * {0,0,0,....44236800}
> + */
> +enum drm_mode_histogram {
> +	DRM_MODE_HISTOGRAM_HSV_MAX_RGB = 0x01, };
> +
> +/**
> + * struct drm_histogram_caps
> + *
> + * @histogram_mode: histogram generation modes, defined in the
> + *		    enum drm_mode_histogram
> + * @bins_count: number of bins for a chosen histogram mode. For illustration
> + *		refer the above defined histogram mode.
> + */
> +struct drm_histogram_caps {
> +	__u32 histogram_mode;

Do we really need __u32 for histogram mode don't you think a __u16 should suffice?


> +	__u32 bins_count;

Nit: bin_count sounds better.

> +};
> +
> +/**
> + * struct drm_histogram_config
> + *
> + * @hist_mode_data: address to the histogram mode specific data if any
> + * @nr_hist_mode_data: number of elements pointed by the address in
> + *		       hist_mode_data
> + * @hist_mode: histogram mode(HSV max(RGB), RGB, LUMA etc)
> + * @enable: flag to enable/disable histogram  */ struct
> +drm_histogram_config {
> +	__u64 hist_mode_data;
> +	__u32 nr_hist_mode_data;
> +	enum drm_mode_histogram hist_mode;
> +	bool enable;
> +};
> +
> +/**
> + * struct drm_histogram
> + *
> + * @config: histogram configuration data pointed by struct
> +drm_histogram_config
> + * @data_ptr: pointer to the array of histogram.
> + *	      Histogram is an array of bins. Data format for each bin depends
> + *	      on the histogram mode. Refer to the above histogram modes for

I think you can write the drm_histogram_mode_caps instead of writing histogram mode
So people can directly jump to it

Regards,
Suraj Kandpal

> + *	      more information.
> + * @nr_elements: number of bins in the histogram.
> + */
> +struct drm_histogram {
> +	struct drm_histogram_config config;
> +	__u64 data_ptr;
> +	__u32 nr_elements;
> +};
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> 
> --
> 2.25.1