[PATCHv2 3/5] Add crtc properties for global histogram
Murthy, Arun R
arun.r.murthy at intel.com
Wed Sep 18 10:55:09 UTC 2024
> > #define INTEL_CRTC_FUNCS \
> > .set_config = drm_atomic_helper_set_config, \
> > .destroy = intel_crtc_destroy, \
> > @@ -229,7 +326,9 @@ static int intel_crtc_late_register(struct drm_crtc
> *crtc)
> > .set_crc_source = intel_crtc_set_crc_source, \
> > .verify_crc_source = intel_crtc_verify_crc_source, \
> > .get_crc_sources = intel_crtc_get_crc_sources, \
> > - .late_register = intel_crtc_late_register
> > + .late_register = intel_crtc_late_register, \
> > + .atomic_set_property = intel_crtc_set_property, \
> > + .atomic_get_property = intel_crtc_get_property
> >
> > static const struct drm_crtc_funcs bdw_crtc_funcs = {
> > INTEL_CRTC_FUNCS,
> > @@ -374,6 +473,10 @@ int intel_crtc_init(struct drm_i915_private
> *dev_priv, enum pipe pipe)
> > intel_color_crtc_init(crtc);
> > intel_drrs_crtc_init(crtc);
> > intel_crtc_crc_init(crtc);
> > + intel_histogram_init(crtc);
> > +
> > + /* Initialize crtc properties */
> > + intel_crtc_add_property(crtc);
> >
> > cpu_latency_qos_add_request(&crtc->vblank_pm_qos,
> > PM_QOS_DEFAULT_VALUE);
> >
> > @@ -690,3 +793,100 @@ void intel_pipe_update_end(struct
> > intel_atomic_state *state,
> > out:
> > intel_psr_unlock(new_crtc_state);
> > }
> > +
> > +static const struct drm_prop_enum_list histogram_en_names[] = {
>
> en_names?
Changed to enable_names
>
> > + { INTEL_HISTOGRAM_DISABLE, "Disable" },
> > + { INTEL_HISTOGRAM_ENABLE, "Enable" }, };
> > +
> > +/**
> > + * intel_attach_histogram_en_property() - add property to
> > +enable/disable histogram
> > + * @intel_crtc: pointer to the struct intel_crtc on which the global histogram
> is to
> > + * be enabled/disabled
> > + *
> > + * "HISTOGRAM_EN" is the crtc propety to enable/disable global
> > +histogram
>
> There's zero gain in abbreviating enable to _EN.
>
Changed to Enable, also removed all caps.
> > + */
> > +void intel_attach_histogram_en_property(struct intel_crtc
> > +*intel_crtc) {
> > + struct drm_crtc *crtc = &intel_crtc->base;
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_property *prop;
> > +
> > + prop = intel_crtc->histogram_en_property;
> > + if (!prop) {
> > + prop = drm_property_create_enum(dev, 0,
> > + "HISTOGRAM_EN",
> > + histogram_en_names,
> > +
> ARRAY_SIZE(histogram_en_names));
> > + if (!prop)
> > + return;
> > +
> > + intel_crtc->histogram_en_property = prop;
> > + }
> > +
> > + drm_object_attach_property(&crtc->base, prop, 0); }
> > +
> > +/**
> > + * intel_attach_global_iet_property() - add property to write Image
> > +Enhancement data
> > + * @intel_crtc: pointer to the struct intel_crtc on which global
> > +histogram is enabled
> > + *
> > + * "Global IET" is the crtc property to write the Image Enhancement
> > +LUT binary data */ void intel_attach_global_iet_property(struct
> > +intel_crtc *intel_crtc) {
> > + struct drm_crtc *crtc = &intel_crtc->base;
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_property *prop;
> > +
> > + prop = intel_crtc->global_iet_property;
> > + if (!prop) {
> > + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> DRM_MODE_PROP_ATOMIC,
> > + "Global IET", 0);
> > + if (!prop)
> > + return;
> > +
> > + intel_crtc->global_iet_property = prop;
> > + }
> > +
> > + drm_object_attach_property(&crtc->base, prop, 0); }
> > +
> > +/**
> > + * intel_attach_histogram_property() - crtc property to read the histogram.
> > + * @intel_crtc: pointer to the struct intel_crtc on which the global histogram
> > + * was enabled.
> > + * "Global Histogram" is the crtc property to read the binary histogram data.
> > + */
> > +void intel_attach_histogram_property(struct intel_crtc *intel_crtc) {
> > + struct drm_crtc *crtc = &intel_crtc->base;
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_property *prop;
> > + struct drm_property_blob *blob;
> > +
> > + prop = intel_crtc->histogram_property;
> > + if (!prop) {
> > + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> > + DRM_MODE_PROP_ATOMIC |
> > + DRM_MODE_PROP_IMMUTABLE,
> > + "Global Histogram", 0);
> > + if (!prop)
> > + return;
> > +
> > + intel_crtc->histogram_property = prop;
> > + }
> > + blob = drm_property_create_blob(dev, sizeof(uint32_t) *
> HISTOGRAM_BIN_COUNT, NULL);
> > + intel_crtc->config->histogram = blob;
> > +
> > + drm_object_attach_property(&crtc->base, prop, blob->base.id); }
> > +
> > +int intel_crtc_add_property(struct intel_crtc *intel_crtc) {
> > + intel_attach_histogram_en_property(intel_crtc);
> > + intel_attach_histogram_property(intel_crtc);
> > + intel_attach_global_iet_property(intel_crtc);
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h
> > b/drivers/gpu/drm/i915/display/intel_crtc.h
> > index b615b7ab5ccd..56c6b7c6037e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> > @@ -7,6 +7,7 @@
> > #define _INTEL_CRTC_H_
> >
> > #include <linux/types.h>
> > +#include <drm/drm_crtc.h>
> >
> > enum i9xx_plane_id;
> > enum pipe;
> > @@ -49,4 +50,8 @@ void intel_wait_for_vblank_if_active(struct
> drm_i915_private *i915,
> > enum pipe pipe);
> > void intel_crtc_wait_for_next_vblank(struct intel_crtc *crtc);
> >
> > +int intel_crtc_add_property(struct intel_crtc *intel_crtc); void
> > +intel_attach_histogram_en_property(struct intel_crtc *intel_crtc);
> > +void intel_attach_global_iet_property(struct intel_crtc *intel_crtc);
> > +void intel_attach_histogram_property(struct intel_crtc *intel_crtc);
> > #endif
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 9f2a4a854548..20caa952d687 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -94,6 +94,7 @@
> > #include "intel_fifo_underrun.h"
> > #include "intel_frontbuffer.h"
> > #include "intel_hdmi.h"
> > +#include "intel_histogram.h"
> > #include "intel_hotplug.h"
> > #include "intel_link_bw.h"
> > #include "intel_lvds.h"
> > @@ -4335,6 +4336,10 @@ static int intel_crtc_atomic_check(struct
> intel_atomic_state *state,
> > if (ret)
> > return ret;
> >
> > + /* HISTOGRAM changed */
> > + if (crtc_state->histogram_en_changed)
> > + return intel_histogram_atomic_check(crtc);
> > +
> > return 0;
> > }
> >
> > @@ -7512,6 +7517,14 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
> > * FIXME get rid of this funny new->old swapping
> > */
> > old_crtc_state->dsb = fetch_and_zero(&new_crtc_state->dsb);
> > +
> > + /* Re-Visit: HISTOGRAM related stuff */
> > + if (new_crtc_state->histogram_en_changed)
> > + intel_histogram_update(crtc,
> > + new_crtc_state->histogram_en);
> > + if (new_crtc_state->global_iet_changed)
> > + intel_histogram_set_iet_lut(crtc,
> > + (u32 *)new_crtc_state-
> >global_iet->data);
> > }
> >
> > /* Underruns don't always raise interrupts, so check manually */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 79d34d6d537d..ddf1cb0ab26d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -99,6 +99,12 @@ enum intel_broadcast_rgb {
> > INTEL_BROADCAST_RGB_LIMITED,
> > };
> >
> > +/* HISTOGRAM property */
> > +enum intel_histogram_en_prop {
> > + INTEL_HISTOGRAM_PROP_DISABLE,
> > + INTEL_HISTOGRAM_PROP_ENABLE,
> > +};
> > +
>
> This is not a property. This is a regular enum. And it does not belong in this file.
>
Sorry, this is obsolete, maybe I missed removed this.
This will be removed in the next series.
> > struct intel_fb_view {
> > /*
> > * The remap information used in the remapped and rotated views to
> > @@ -1431,6 +1437,13 @@ struct intel_crtc_state {
> >
> > /* LOBF flag */
> > bool has_lobf;
> > +
> > + /* HISTOGRAM data */
>
> Why all caps?
>
Changed
> > + int histogram_en;
> > + struct drm_property_blob *global_iet;
> > + struct drm_property_blob *histogram;
> > + bool global_iet_changed;
> > + bool histogram_en_changed;
>
> Please add a substruct for all the histogram stuff to keep it clean.
>
Done
Thanks and Regards,
Arun R Murthy
--------------------
More information about the Intel-gfx
mailing list