[PATCHv3 01/10] drm/crtc: Add histogram properties

[Murthy, Arun R]

> On Mon, Dec 09, 2024 at 09:54:55PM +0530, Arun R Murthy wrote:
> > Add variables for histogram drm_property, its corrsponding crtc_state
> > variables and define the structure pointed by the blob property.
> >
> > struct drm_histogram and drm_iet defined in
> > include/uapi/drm/drm_mode.h
> >
> > The property HISTOGRAM_ENABLE allows user to enable/disable the
> > histogram feature in the hardware. Upon KMD enabling by writing to the
> > hardware registers, a histogram is generated. Histogram is composed of
> > 'n' bins with each bin being an integer(pixel count).
> 
> Is it really a count of pixels that fall into one of the bins?
It's the statistics generated for each frame that is sent to the display and the value corresponds to 5 bit pixel depth.

> 
> > An event HISTOGRAM will be sent to the user. User upon recieving this
> > event user can read the hardware generated histogram using crtc
> > property
> 
> Nit: here and further, it's CRTC, not crtc
Ok.

> 
> > HISTOGRAM_DATA. User can use this histogram data, apply various
> > equilization techniques to come up with a pixel factor. This pixel
> > factor is an array of integer with 'n+1' elements. This is fed back to
> > the KMD by crtc property HISTOGRAM_IET. KMD will write this IET data
> > back to the hardware to see the enhancement/equilization done to the
> > images on that pipe.
> > The crtc property HISTOGRAM_DATA and HISTOGRAM_IET is of type blob.
> >
> > For crtc property HISTOGRAM_DATA,
> > the blob data pointer will be the address of the struct drm_histogram.
> > struct drm_histogram {
> > 	u64 data_ptr;
> > 	u32 nr_elements;
> > }
> > Histogram is composed of @nr_elements of bins with each bin being an
> > integer value, referred to as pixel_count.
> > The element @data_ptr holds the address of histogam.
> > Sample:
> > Historgram[0] = 2050717
> > Historgram[1] = 244619
> > Historgram[2] = 173368
> > ....
> > Historgram[31] = 21631
> >
> > For crtc_property HISTOGRAM_IET,
> > the blob data pointer will be the address of the struct drm_iet.
> > struct drm_iet {
> > 	u64 data_ptr;
> > 	u32 nr_elements;
> > }
> > ImageEnhancemenT(IET) is composed of @nr_elements of bins with each
> > bin being an integer value, referred to as pixel factor.
> 
> What are pixel factors? How are they supposed to be used? You have specified
> the format of the data, but one still can not implement proper HISTOGRAM
> support for the VKMS.
>
This pixel factor is a value that can be multiplied/appended/prepended to the pixels of the incoming image on that pipe thereby enhancing the image quality.
 
> > The element @data_ptr holds the addess of pixel factor.
> > Sample:
> > Pixel Factor[0] = 1023
> > Pixel Factor[1] = 695
> > Pixel Factor[2] = 1023
> > ....
> > Pixel Factor[32] = 512
> >
> > Histogram is the statistics generated with 'n' number of frames on a
> > pipe.
> > Usually the size of pixel factor is one more than the size of
> > histogram data.
> 
> What does that mean? What does it mean if this "Usually" isn't being followed?
> Previously you've written that it always has n+1 elements.
> 
In Intel platform its 'n' for histogram and 'n+1' for IET. Can vary with other vendors supporting this feature.
Hence have a variable(nr_elements) to convey this.

Thanks and Regards,
Arun R Murthy
--------------------
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> > ---
> >  include/drm/drm_crtc.h      | 51
> +++++++++++++++++++++++++++++++++++++
> >  include/uapi/drm/drm_mode.h | 24 +++++++++++++++++
> >  2 files changed, 75 insertions(+)
> >
> 
> --
> With best wishes
> Dmitry