Re: [v8,06/14] drm/i915/histogram: Add support for histogram

["Thasleem, Mohammed" <mohammed.thasleem@intel.com>]

Hi Arun,

On 1/28/2025 9:21 PM, Arun R Murthy wrote:
> Statistics is generated from the image frame that is coming to display
> and an event is sent to user after reading this histogram data.
> 
> v2: forward declaration in header file along with error handling (Jani)
> v3: Replaced i915 with intel_display (Suraj)
> v4: Removed dithering enable/disable (Vandita)
>      New patch for histogram register definitions (Suraj)
> v5: IET LUT pgm follow the seq in spec and removed change to TC at end
>      (Suraj)
> v8: Retained only the Histogram part and move IET LUT to a different
>      patch.
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                      |   1 +
>   drivers/gpu/drm/i915/display/intel_display_types.h |   2 +
>   drivers/gpu/drm/i915/display/intel_histogram.c     | 157 +++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_histogram.h     |  48 +++++++
>   4 files changed, 208 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 4caa8e30bc98387d45212fbc7cea8b38687bd0d5..f993b19174ba79c0bcc34994619937be7d2797ed 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -270,6 +270,7 @@ i915-y += \
>   	display/intel_hdcp.o \
>   	display/intel_hdcp_gsc.o \
>   	display/intel_hdcp_gsc_message.o \
> +	display/intel_histogram.o \
>   	display/intel_hotplug.o \
>   	display/intel_hotplug_irq.o \
>   	display/intel_hti.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index cb51b7936f9338caaf14b1c6f7bbcc4327da4ef1..761fefed9376439c0ee5d346e8110a219ad0a586 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1434,6 +1434,8 @@ struct intel_crtc {
>   	/* for loading single buffered registers during vblank */
>   	struct pm_qos_request vblank_pm_qos;
>   
> +	struct intel_histogram *histogram;
> +
>   #ifdef CONFIG_DEBUG_FS
>   	struct intel_pipe_crc pipe_crc;
>   #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_histogram.c b/drivers/gpu/drm/i915/display/intel_histogram.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..26eae8f40d0bf642546d583546782e22d5cefa9c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_histogram.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_vblank.h>
> +
> +#include "i915_reg.h"
> +#include "i915_drv.h"
> +#include "intel_de.h"
> +#include "intel_display.h"
> +#include "intel_display_types.h"
> +#include "intel_histogram.h"
> +#include "intel_histogram_regs.h"
> +
> +/* 3.0% of the pipe's current pixel count, hw does x4 */
> +#define HISTOGRAM_GUARDBAND_THRESHOLD_DEFAULT 300
> +/* Precision factor for threshold guardband */
> +#define HISTOGRAM_GUARDBAND_PRECISION_FACTOR 10000
> +#define HISTOGRAM_DEFAULT_GUARDBAND_DELAY 0x04
> +
> +int intel_histogram_atomic_check(struct intel_crtc *intel_crtc)
> +{
> +	struct intel_histogram *histogram = intel_crtc->histogram;
> +
> +	/* TODO: Restrictions for enabling histogram */
> +	histogram->can_enable = true;
> +
> +	return 0;
> +}
> +
> +static int intel_histogram_enable(struct intel_crtc *intel_crtc, u8 mode)
> +{
> +	struct intel_display *display = to_intel_display(intel_crtc);
> +	struct intel_histogram *histogram = intel_crtc->histogram;
> +	int pipe = intel_crtc->pipe;
> +	u64 res;
> +	u32 gbandthreshold;
> +
> +	if (!histogram || !histogram->can_enable)
> +		return -EINVAL;
> +
> +	if (histogram->enable)
> +		return 0;
> +
> +	 /* enable histogram, clear DPST_CTL bin reg func select to TC */
> +	intel_de_rmw(display, DPST_CTL(pipe),
> +		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_IE_HIST_EN |
> +		     DPST_CTL_HIST_MODE | DPST_CTL_IE_TABLE_VALUE_FORMAT |
> +		     DPST_CTL_ENHANCEMENT_MODE_MASK | DPST_CTL_IE_MODI_TABLE_EN,
> +		     ((mode == DRM_MODE_HISTOGRAM_HSV_MAX_RGB) ?
> +		      DPST_CTL_BIN_REG_FUNC_TC : 0) | DPST_CTL_IE_HIST_EN |
> +		     DPST_CTL_HIST_MODE_HSV |
> +		     DPST_CTL_IE_TABLE_VALUE_FORMAT_1INT_9FRAC |
> +		     DPST_CTL_EN_MULTIPLICATIVE | DPST_CTL_IE_MODI_TABLE_EN);
> +
> +	/* Re-Visit: check if wait for one vblank is required */
> +	drm_crtc_wait_one_vblank(&intel_crtc->base);
> +
> +	/* TODO: Program GuardBand Threshold needs to be moved to modeset path */
> +	res = (intel_crtc->config->hw.adjusted_mode.vtotal *
> +	       intel_crtc->config->hw.adjusted_mode.htotal);
> +
> +	gbandthreshold = (res *	HISTOGRAM_GUARDBAND_THRESHOLD_DEFAULT) /
> +			  HISTOGRAM_GUARDBAND_PRECISION_FACTOR;
> +
> +	/* Enable histogram interrupt mode */
> +	intel_de_rmw(display, DPST_GUARD(pipe),
> +		     DPST_GUARD_THRESHOLD_GB_MASK |
> +		     DPST_GUARD_INTERRUPT_DELAY_MASK | DPST_GUARD_HIST_INT_EN,
> +		     DPST_GUARD_THRESHOLD_GB(gbandthreshold) |
> +		     DPST_GUARD_INTERRUPT_DELAY(HISTOGRAM_DEFAULT_GUARDBAND_DELAY) |
> +		     DPST_GUARD_HIST_INT_EN);
> +
> +	/* Clear pending interrupts has to be done on separate write */
> +	intel_de_rmw(display, DPST_GUARD(pipe),
> +		     DPST_GUARD_HIST_EVENT_STATUS, 1);
> +
> +	histogram->enable = true;
> +
> +	return 0;
> +}
> +
> +static void intel_histogram_disable(struct intel_crtc *intel_crtc)
> +{
> +	struct intel_display *display = to_intel_display(intel_crtc);
> +	struct intel_histogram *histogram = intel_crtc->histogram;
> +	int pipe = intel_crtc->pipe;
> +
> +	if (!histogram)
> +		return;
> +
> +	/* If already disabled return */
> +	if (histogram->enable)

It should be !histogram->enable right?

Thanks,
Thasleem

> +		return;
> +
> +	/* Clear pending interrupts and disable interrupts */
> +	intel_de_rmw(display, DPST_GUARD(pipe),
> +		     DPST_GUARD_HIST_INT_EN | DPST_GUARD_HIST_EVENT_STATUS, 0);
> +
> +	/* disable DPST_CTL Histogram mode */
> +	intel_de_rmw(display, DPST_CTL(pipe),
> +		     DPST_CTL_IE_HIST_EN, 0);
> +
> +	histogram->enable = false;
> +}
> +
> +int intel_histogram_update(struct intel_crtc *intel_crtc,
> +			   struct drm_histogram_config *config)
> +{
> +	struct intel_display *display = to_intel_display(intel_crtc);
> +
> +	if (config->enable) {
> +		if (config->hist_mode != DRM_MODE_HISTOGRAM_HSV_MAX_RGB) {
> +			drm_err(display->drm,
> +				"Only max(RGB) mode is supported for histogram\n");
> +			return -EINVAL;
> +		}
> +		return intel_histogram_enable(intel_crtc, config->hist_mode);
> +	}
> +
> +	intel_histogram_disable(intel_crtc);
> +	return 0;
> +}
> +
> +void intel_histogram_finish(struct intel_crtc *intel_crtc)
> +{
> +	struct intel_histogram *histogram = intel_crtc->histogram;
> +
> +	kfree(histogram);
> +}
> +
> +int intel_histogram_init(struct intel_crtc *crtc)
> +{
> +	struct intel_histogram *histogram;
> +	struct drm_histogram_caps *histogram_caps;
> +
> +	/* Allocate histogram internal struct */
> +	histogram = kzalloc(sizeof(*histogram), GFP_KERNEL);
> +	if (!histogram)
> +		return -ENOMEM;
> +	histogram_caps = kzalloc(sizeof(*histogram_caps), GFP_KERNEL);
> +	if (!histogram_caps)
> +		return -ENOMEM;
> +
> +	histogram_caps->histogram_mode = DRM_MODE_HISTOGRAM_HSV_MAX_RGB;
> +	histogram_caps->bins_count = HISTOGRAM_BIN_COUNT;
> +
> +	crtc->histogram = histogram;
> +	histogram->crtc = crtc;
> +	histogram->can_enable = false;
> +	histogram->caps = histogram_caps;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_histogram.h b/drivers/gpu/drm/i915/display/intel_histogram.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..5ea19ef2d3ecadf1ac159a784f51278fdde593de
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_histogram.h
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef __INTEL_HISTOGRAM_H__
> +#define __INTEL_HISTOGRAM_H__
> +
> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +
> +struct delayed_work;
> +struct drm_property_blob;
> +struct drm_histogram_config;
> +struct drm_histogram_caps;
> +struct intel_crtc;
> +
> +#define HISTOGRAM_BIN_COUNT                    32
> +
> +struct intel_histogram {
> +	struct drm_histogram_caps *caps;
> +	struct intel_crtc *crtc;
> +	struct delayed_work work;
> +	bool enable;
> +	bool can_enable;
> +	u32 bin_data[HISTOGRAM_BIN_COUNT];
> +};
> +
> +enum intel_global_hist_status {
> +	INTEL_HISTOGRAM_ENABLE,
> +	INTEL_HISTOGRAM_DISABLE,
> +};
> +
> +enum intel_global_histogram {
> +	INTEL_HISTOGRAM,
> +};
> +
> +enum intel_global_hist_lut {
> +	INTEL_HISTOGRAM_PIXEL_FACTOR,
> +};
> +
> +int intel_histogram_atomic_check(struct intel_crtc *intel_crtc);
> +int intel_histogram_update(struct intel_crtc *intel_crtc,
> +			   struct drm_histogram_config *config);
> +int intel_histogram_init(struct intel_crtc *intel_crtc);
> +void intel_histogram_finish(struct intel_crtc *intel_crtc);
> +
> +#endif /* __INTEL_HISTOGRAM_H__ */