> -----Original Message-----
> From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Kandpal, Suraj
> Sent: Friday, February 14, 2025 3:32 PM
> To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx;
> intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: dmitry.baryshkov@xxxxxxxxxx
> Subject: RE: [PATCH v8 06/14] drm/i915/histogram: Add support for histogram
>
>
>
> > -----Original Message-----
> > From: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>
> > Sent: Tuesday, January 28, 2025 9:21 PM
> > To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx;
> > dri- devel@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>;
> > dmitry.baryshkov@xxxxxxxxxx; Murthy, Arun R <arun.r.murthy@xxxxxxxxx>
> > Subject: [PATCH v8 06/14] drm/i915/histogram: Add support for
> > histogram
> >
> > 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@xxxxxxxxx>
LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> > ---
> > 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..f993b19174ba79c0bcc349946
> > 19937be7d2797ed 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..761fefed9376439c0ee5d346e81
> > 10a219ad0a586 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..26eae8f40d0bf642546d5835
> > 46782e22d5cefa9c
> > --- /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;
>
> We don’t change anything in the intel_crtc structure during compute or atomic
> check phase We need to consider moving this to the crtc_state.
>
> > +
> > + 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)
> > + 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;
>
> New line here
>
> > + 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;
> > +
>
> Remove extra line
>
> Regards,
> Suraj Kandpal
> > + 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..5ea19ef2d3ecadf1ac159a78
> > 4f51278fdde593de
> > --- /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__ */
> >
> > --
> > 2.25.1
