On Thu, 16 Jan 2025 at 09:08, Murthy, Arun R <arun.r.murthy@xxxxxxxxx> wrote:
>
> > On Fri, Jan 10, 2025 at 01:15:30AM +0530, Arun R Murthy wrote:
> > > ImageEnhancemenT(IET) hardware interpolates the LUT value to generate
> > > the enhanced output image. LUT takes an input value, outputs a new
> > > value based on the data within the LUT. 1D LUT can remap individual
> > > input values to new output values based on the LUT sample. LUT can be
> > > interpolated by the hardware by multiple modes Ex: Direct Lookup LUT,
> > > Multiplicative LUT etc The list of supported mode by hardware along
> > > with the format(exponent
> > > mantissa) is exposed to user by the iet_lut_caps property. Maximum
> > > format being 8.24 i.e 8 exponent and 24 mantissa.
> > > For illustration a hardware supporting 1.9 format denotes this as
> > > 0x10001FF. In order to know the exponent do a bitwise AND with
> > > 0xF000000. The LUT value to be provided by user would be a 10bit value
> > > with 1 bit integer and 9 bit fractional value.
> > >
> > > Multiple formats can be supported, hence pointer is used over here.
> > > User can then provide the LUT with any one of the supported modes in
> > > any of the supported formats.
> > > The entries in the LUT can vary depending on the hardware capability
> > > with max being 255. This will also be exposed as iet_lut_caps so user
> > > can generate a LUT with the specified entries.
> > >
> > > Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx>
> > > ---
> > > include/uapi/drm/drm_mode.h | 50
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 50 insertions(+)
> > >
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index
> > >
> > 7a7039381142bb5dba269bdaec42c18be34e2d05..056c2efef1589848034afc00
> > 89f1
> > > 838c2547bcf8 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -1367,6 +1367,17 @@ struct drm_mode_closefb {
> > > */
> > > #define DRM_MODE_HISTOGRAM_HSV_MAX_RGB (1 <<
> > 0)
> > >
> > > +/* LUT values are points on exponential graph with x axis and y-axis
> > > +y=f(x) */
> >
> > Huh?
> >
> This f(x) can be the algorithm defined by the user space algorithm to generate the lookup
> table. Generation of the LUT value is left to the user space algorithm.
> When this LUT table is passed to the hardware its just signifies how hardware should
> use this table to get the LUT value. In this mode it's a direct lookup table.
Your documentation should be describing what is expected from the
userspace. What is y, x and f(x)? How is it being used?
>
> > > +#define DRM_MODE_IET_LOOKUP_LUT (1 <<
> > 0)
> >
> > Again, what is the reason for a shift? Can these values be OR'd?
> >
> Yes can be OR'd values as well.
> Let me know if this has to be changed?
> Just chose bitwise shift to denote the multiple modes.
What does it mean if drm_iet_1dlut_sample.iet_mode contains OR of two values?
>
> > > +/*
> > > + * LUT values, points on negative exponential graph with x-axis and
> > > +y-axis
> > > + * Y = y/x so upon multiplying x, y is obtained, hence
> > > +multiplicative. The
> >
> > Can't parse this sentence.
> >
> We need x and y points in the exponential graph.
> For retrieving the value Y on the graph the value passed by the user is in the format y/x
> In order to get the Y points on the graph the value has to be multiplied by x.
> This is a floating point value when compared with an integer value with the direct
> lookup mode.
Again, what are x and y? Bin indices? Pixel counts? Number of CPUs in
the current generation?
>
>
> > > + * format of LUT can at max be 8.24(8integer 24 fractional)
> > > + represented by
> > > + * u32. Depending on the hardware capability and exponent mantissa
> > > + can be
> > > + * chosen.
> >
> > What does that mean? How is it choosen?
> >
> The max value that these kind of 1DLUT can be is 8.24
Why?
> Hardware design can choose anything within this range. This depends
> on the accuracy required by hardware keeping in mind the hardware cost for
> implementation.
> Just a precision for 32bit value.
>
> > > + */
> > > +#define DRM_MODE_IET_MULTIPLICATIVE (1 << 1)
> > > +
> > > /**
> > > * struct drm_histogram_caps
> > > *
> > > @@ -1414,6 +1425,45 @@ struct drm_histogram {
> > > __u32 nr_elements;
> > > };
> > >
> > > +/**
> > > + * struct drm_iet_caps
> > > + *
> > > + * @iet_mode: pixel factor enhancement modes defined in the above
> > > +macros
> > > + * @iet_sample_format: holds the address of an array of u32 LUT sample
> > formats
> > > + * depending on the hardware capability. Max being 8.24
> > > + * Doing a bitwise AND will get the present sample.
> > > + * Ex: for 1 integer 9 fraction AND with 0x10001FF
> >
> > ?? Can hardware support 16.16? 32.0?
> >
> No, for a 1D LUT maximum floating number can be 8.24
Why? Is it a limitation of the Intel hardware or just a random API choice?
> Hence hardware will have to adhere to anything within this range.
>
> > > + * @nr_iet_sample_formats: number of iet_sample_formsts supported by
> > the
> > > + * hardware
> > > + * @nr_iet_lut_entries: number of LUT entries */ struct drm_iet_caps
> > > +{
> > > + __u8 iet_mode;
> > > + u64 iet_sample_format;
> > > + __u32 nr_iet_sample_formats;
> > > + __u32 nr_iet_lut_entries;
> > > +};
> > > +
> > > +/**
> > > + * struct drm_iet_1dlut_sample
> >
> > Is it supposed to be used with DRM_MODE_IET_MULTIPLICATIVE only? Or is it
> > supposed to be used with DRM_MODE_IET_LOOKUP_LUT? In the latter case
> > what should be the iet_format value?
> >
> The struct iet_1dlut_sample will be used for all the IET modes i.e direct lookup and
> multiplicative.
> The element iet_sample_format will not be applicable for direct lookup. This will be
> used for multiplicative and the value what it can hold for multiplicative is mentioned
> in the above description.
> I missed adding this info in the description, will add it in the next version.
And some other formats will also require additional data. This
multi-format structure sounds bad from my POV.
>
> > > + * @iet_mode: image enhancement mode, this will also convey the channel.
> > > + * @iet_format: LUT exponent and mantissa format, max being 8.24
> > > + * @data_ptr: pointer to the array of values which is of type u32.
> > > + * 1 channel: 10 bit corrected value and remaining bits are reserved.
> > > + * multi channel: pointer to struct drm_color_lut
> > > + * @nr_elements: number of entries pointed by the data @data_ptr
> > > + * @reserved: reserved for future use
> > > + * @reserved1: reserved for future use */ struct
> > > +drm_iet_1dlut_sample {
> > > + __u8 iet_mode;
> > > + __u32 iet_format;
> > > + __u64 data_ptr;
> > > + __u32 nr_elements;
> > > + __u32 reserved;
> > > + __u32 reserved1;
> > > +};
> > > +
> > > #if defined(__cplusplus)
> > > }
> > > #endif
> > >
> > > --
> > > 2.25.1
> > >
> >
> Thanks and Regards,
> Arun R Murthy
> --------------------
--
With best wishes
Dmitry
