[PATCHv2 1/5] drm/i915/display: Add support for histogram

[Murthy, Arun R]

> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of
> > Arun R Murthy
> > Sent: Wednesday, August 21, 2024 3:54 PM
> > To: intel-gfx at lists.freedesktop.org
> > Cc: Murthy, Arun R <arun.r.murthy at intel.com>
> > Subject: [PATCHv2 1/5] drm/i915/display: 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.
> > This statistics/histogram is then shared with the user upon getting a
> > request from user. User can then use this histogram and generate an
> > enhancement factor. This enhancement factor can be multiplied/added
> > with the incoming pixel data frame.
> >
> > v2: forward declaration in header file along with error handling
> > (Jani)
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                 |   1 +
> >  .../drm/i915/display/intel_display_types.h    |   2 +
> >  .../gpu/drm/i915/display/intel_histogram.c    | 205 ++++++++++++++++++
> >  .../gpu/drm/i915/display/intel_histogram.h    |  78 +++++++
> >  drivers/gpu/drm/xe/Makefile                   |   1 +
> >  5 files changed, 287 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile index c63fa2133ccb..03caf3a24966
> > 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -264,6 +264,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 bd290536a1b7..79d34d6d537d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1537,6 +1537,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 000000000000..45e968e00af6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_histogram.c
> > @@ -0,0 +1,205 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation  */
> > +
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_file.h>
> > +
> > +#include "i915_reg.h"
> > +#include "i915_drv.h"
> > +#include "intel_display.h"
> > +#include "intel_histogram.h"
> > +#include "intel_display_types.h"
> > +#include "intel_de.h"
> > +
> > +#define HISTOGRAM_GUARDBAND_THRESHOLD_DEFAULT 300    // 3.0% of
> > the pipe's current pixel count.
> > +#define HISTOGRAM_GUARDBAND_PRECISION_FACTOR 10000   // Precision
> > factor for threshold guardband.
> > +#define HISTOGRAM_DEFAULT_GUARDBAND_DELAY 0x04
> > +
> > +struct intel_histogram {
> > +	struct drm_i915_private *i915;
> > +	bool enable;
> > +	bool can_enable;
> > +	enum pipe pipe;
> > +	u32 bindata[HISTOGRAM_BIN_COUNT];
> > +};
> > +
> > +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;
> > +}
> > +
> Looks like we are totally bypassing crtc_state->dither.
> Also I see some comments on dither not being enabled on anything which is
> not 6bpc. Is that constraint resolved now?
> 

Crtc_state->dither is used for enabling dithering during the crtc_enable and at this point we are far ahead of that.
That restriction is for older platforms(ironlake) and we don't have any such for ADLP+

> > +static void intel_histogram_enable_dithering(struct drm_i915_private
> > *dev_priv,
> > +					     enum pipe pipe)
> > +{
> > +	intel_de_rmw(dev_priv, PIPE_MISC(pipe),
> > PIPE_MISC_DITHER_ENABLE,
> > +		     PIPE_MISC_DITHER_ENABLE);
> > +}
> > +
> > +static int intel_histogram_enable(struct intel_crtc *intel_crtc) {
> > +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> > +	struct intel_histogram *histogram = intel_crtc->histogram;
> > +	int pipe = intel_crtc->pipe;
> > +	u64 res;
> > +	u32 gbandthreshold;
> > +
> > +	if (!histogram)
> > +		return -EINVAL;
> > +
> > +	if (!histogram->can_enable) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (histogram->enable)
> > +		return 0;
> > +
> I don't see in the spec that dither should be enabled, any quick bspec
> references?
Dithering should be enabled by default for ADL+ for DPST. This is an enhancement to avoid artifacts.

Thanks and Regards,
Arun R Murthy
--------------------