Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-11 Thread Pekka Paalanen
On Wed, 10 Nov 2021 10:17:59 -0500
Harry Wentland  wrote:

> On 2021-11-10 06:55, Ville Syrjälä wrote:
> > On Wed, Nov 10, 2021 at 10:49:24AM +0200, Pekka Paalanen wrote:  
> >> On Wed, 10 Nov 2021 00:02:16 +0200
> >> Ville Syrjälä  wrote:
> >>  
> >>> On Tue, Nov 09, 2021 at 03:47:58PM -0500, Harry Wentland wrote:  
>  On 2021-11-08 04:54, Pekka Paalanen wrote:
> > On Thu, 4 Nov 2021 12:27:56 -0400
> > Harry Wentland  wrote:
> > 
> >> On 2021-11-04 04:38, Pekka Paalanen wrote:
> >>> On Wed, 3 Nov 2021 11:08:13 -0400
> >>> Harry Wentland  wrote:
> >>>   
>  On 2021-09-06 17:38, Uma Shankar wrote:  
> > Existing LUT precision structure is having only 16 bit
> > precision. This is not enough for upcoming enhanced hardwares
> > and advance usecases like HDR processing. Hence added a new
> > structure with 32 bit precision values.
> >
> > This also defines a new structure to define color lut ranges,
> > along with related macro definitions and enums. This will help
> > describe multi segmented lut ranges in the hardware.
> >
> > Signed-off-by: Uma Shankar 
> > ---
> >  include/uapi/drm/drm_mode.h | 58 
> > +
> >  1 file changed, 58 insertions(+)  
> >>
> >> ...
> >>  
> >> If the framebuffer is not in FP16 the question then becomes how
> >> the integer pixel values relate to LUT addressing.
> >
> > Traditionally, and in any API I've seen (GL, Vulkan), a usual mapping
> > is to match minimum unsigned integer value to 0.0, and unsigned maximum
> > integer value to 1.0. This is how things work on the cable too, right?
> > (Also taking full vs. limited range video signal into account. And
> > conversion to cable-YUV if that happens.)
> >
> > If you want integer format FB values to map to something else, then you
> > have to tag the FB with that range information, somehow. New UAPI.
> > 
> 
>  On the cable we send integer values, not floating point. AMD HW uses
>  floating point internally, though, and the PWL API defines floating
>  point entries, so on some level we need to be clear what the floating
>  point entries mean. Either we document that to be [0.0, 1.0] or we
>  have some UAPI to define it. I'm leaning toward the latter but have
>  to think about it some more.
> >>>
> >>> As for Intel hw if you have an integer pixel value of 0xff... (with
> >>> however many bits you have with a specific pixel format) it will get
> >>> extended to 0.fff... (to whatever precision the pipe has internally).
> >>> So if we go by that a fixed point 1.0 value in the proposed
> >>> drm_color_lut_range would be considered just outside the gamut. And
> >>> pretty sure fp16 input of 1.0 should also result in a 0.fff... internal
> >>> value as well [1]. I think that definition pretty much matches how GL
> >>> UNORM<->float conversion works as well.  
> >>
> >> Does it work that way in GL though?
> >>
> >> I've always thought that with GL_UNSIGNED_BYTE, 0xff maps to 1.0, not
> >> 255.0/256.0.

...

> > 
> > That seems to match what I said, or at least tried to say (~0 <-> 1.0 in
> > float). drm_color_lut_range being fixed point would follow the ~0 side of
> > that. Or at least that interpretation would very easily map to our hw.
> >   
> 
> If I understand you right Intel HW represents 0xff (assuming 8 bpc) as
> the largest (representable) float that is less than 1.0. That float would
> be bigger than 255.0/256.0 but smaller than 256.0/256.0.

I was just really confused and re-reading what Ville wrote originally
now makes sense to me.

So, not what Harry wrote. Let me attempt to reiterate and mark fixed
point hex values with a h to discriminate from float values.

With 8-bit:
0x00 -> 0.00..0h = 0.0 float
0xff -> 0.ff..fh = 1.0 float

Then 1.000..0h is the first value out of range.


Thanks,
pq


pgpUVhWmvEY9S.pgp
Description: OpenPGP digital signature


Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-10 Thread Ville Syrjälä
On Wed, Nov 10, 2021 at 10:49:24AM +0200, Pekka Paalanen wrote:
> On Wed, 10 Nov 2021 00:02:16 +0200
> Ville Syrjälä  wrote:
> 
> > On Tue, Nov 09, 2021 at 03:47:58PM -0500, Harry Wentland wrote:
> > > On 2021-11-08 04:54, Pekka Paalanen wrote:  
> > > > On Thu, 4 Nov 2021 12:27:56 -0400
> > > > Harry Wentland  wrote:
> > > >   
> > > >> On 2021-11-04 04:38, Pekka Paalanen wrote:  
> > > >>> On Wed, 3 Nov 2021 11:08:13 -0400
> > > >>> Harry Wentland  wrote:
> > > >>> 
> > >  On 2021-09-06 17:38, Uma Shankar wrote:
> > > > Existing LUT precision structure is having only 16 bit
> > > > precision. This is not enough for upcoming enhanced hardwares
> > > > and advance usecases like HDR processing. Hence added a new
> > > > structure with 32 bit precision values.
> > > >
> > > > This also defines a new structure to define color lut ranges,
> > > > along with related macro definitions and enums. This will help
> > > > describe multi segmented lut ranges in the hardware.
> > > >
> > > > Signed-off-by: Uma Shankar 
> > > > ---
> > > >  include/uapi/drm/drm_mode.h | 58 
> > > > +
> > > >  1 file changed, 58 insertions(+)
> 
> ...
> 
> > > >> If the framebuffer is not in FP16 the question then becomes how
> > > >> the integer pixel values relate to LUT addressing.  
> > > > 
> > > > Traditionally, and in any API I've seen (GL, Vulkan), a usual mapping
> > > > is to match minimum unsigned integer value to 0.0, and unsigned maximum
> > > > integer value to 1.0. This is how things work on the cable too, right?
> > > > (Also taking full vs. limited range video signal into account. And
> > > > conversion to cable-YUV if that happens.)
> > > > 
> > > > If you want integer format FB values to map to something else, then you
> > > > have to tag the FB with that range information, somehow. New UAPI.
> > > >   
> > > 
> > > On the cable we send integer values, not floating point. AMD HW uses
> > > floating point internally, though, and the PWL API defines floating
> > > point entries, so on some level we need to be clear what the floating
> > > point entries mean. Either we document that to be [0.0, 1.0] or we
> > > have some UAPI to define it. I'm leaning toward the latter but have
> > > to think about it some more.  
> > 
> > As for Intel hw if you have an integer pixel value of 0xff... (with
> > however many bits you have with a specific pixel format) it will get
> > extended to 0.fff... (to whatever precision the pipe has internally).
> > So if we go by that a fixed point 1.0 value in the proposed
> > drm_color_lut_range would be considered just outside the gamut. And
> > pretty sure fp16 input of 1.0 should also result in a 0.fff... internal
> > value as well [1]. I think that definition pretty much matches how GL
> > UNORM<->float conversion works as well.
> 
> Does it work that way in GL though?
> 
> I've always thought that with GL_UNSIGNED_BYTE, 0xff maps to 1.0, not
> 255.0/256.0.
> 
> Taking a random spec: OpenGL ES 2.0.25
> 
> Section 2.1.2 Data Conversions says:
> 
>   Normalized unsigned integers represent numbers in the range
>   [0, 1]. The conversion from a normalized unsigned integer c to
>   the corresponding floating-point f is defined as
>   f = c / (2^b - 1)
> 
> Note how the divisor has -1.

That seems to match what I said, or at least tried to say (~0 <-> 1.0 in
float). drm_color_lut_range being fixed point would follow the ~0 side of
that. Or at least that interpretation would very easily map to our hw.

-- 
Ville Syrjälä
Intel


Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-10 Thread Pekka Paalanen
On Wed, 10 Nov 2021 00:02:16 +0200
Ville Syrjälä  wrote:

> On Tue, Nov 09, 2021 at 03:47:58PM -0500, Harry Wentland wrote:
> > On 2021-11-08 04:54, Pekka Paalanen wrote:  
> > > On Thu, 4 Nov 2021 12:27:56 -0400
> > > Harry Wentland  wrote:
> > >   
> > >> On 2021-11-04 04:38, Pekka Paalanen wrote:  
> > >>> On Wed, 3 Nov 2021 11:08:13 -0400
> > >>> Harry Wentland  wrote:
> > >>> 
> >  On 2021-09-06 17:38, Uma Shankar wrote:
> > > Existing LUT precision structure is having only 16 bit
> > > precision. This is not enough for upcoming enhanced hardwares
> > > and advance usecases like HDR processing. Hence added a new
> > > structure with 32 bit precision values.
> > >
> > > This also defines a new structure to define color lut ranges,
> > > along with related macro definitions and enums. This will help
> > > describe multi segmented lut ranges in the hardware.
> > >
> > > Signed-off-by: Uma Shankar 
> > > ---
> > >  include/uapi/drm/drm_mode.h | 58 
> > > +
> > >  1 file changed, 58 insertions(+)

...

> > >> If the framebuffer is not in FP16 the question then becomes how
> > >> the integer pixel values relate to LUT addressing.  
> > > 
> > > Traditionally, and in any API I've seen (GL, Vulkan), a usual mapping
> > > is to match minimum unsigned integer value to 0.0, and unsigned maximum
> > > integer value to 1.0. This is how things work on the cable too, right?
> > > (Also taking full vs. limited range video signal into account. And
> > > conversion to cable-YUV if that happens.)
> > > 
> > > If you want integer format FB values to map to something else, then you
> > > have to tag the FB with that range information, somehow. New UAPI.
> > >   
> > 
> > On the cable we send integer values, not floating point. AMD HW uses
> > floating point internally, though, and the PWL API defines floating
> > point entries, so on some level we need to be clear what the floating
> > point entries mean. Either we document that to be [0.0, 1.0] or we
> > have some UAPI to define it. I'm leaning toward the latter but have
> > to think about it some more.  
> 
> As for Intel hw if you have an integer pixel value of 0xff... (with
> however many bits you have with a specific pixel format) it will get
> extended to 0.fff... (to whatever precision the pipe has internally).
> So if we go by that a fixed point 1.0 value in the proposed
> drm_color_lut_range would be considered just outside the gamut. And
> pretty sure fp16 input of 1.0 should also result in a 0.fff... internal
> value as well [1]. I think that definition pretty much matches how GL
> UNORM<->float conversion works as well.

Does it work that way in GL though?

I've always thought that with GL_UNSIGNED_BYTE, 0xff maps to 1.0, not
255.0/256.0.

Taking a random spec: OpenGL ES 2.0.25

Section 2.1.2 Data Conversions says:

Normalized unsigned integers represent numbers in the range
[0, 1]. The conversion from a normalized unsigned integer c to
the corresponding floating-point f is defined as
f = c / (2^b - 1)

Note how the divisor has -1.


Thanks,
pq

> [1] though IIRC some our hw did get that a bit wrong and it
> actually generates a 1.0 fixed point value for the pipe



pgpIPz92VmrZq.pgp
Description: OpenPGP digital signature


Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-09 Thread Ville Syrjälä
On Tue, Nov 09, 2021 at 03:47:58PM -0500, Harry Wentland wrote:
> On 2021-11-08 04:54, Pekka Paalanen wrote:
> > On Thu, 4 Nov 2021 12:27:56 -0400
> > Harry Wentland  wrote:
> > 
> >> On 2021-11-04 04:38, Pekka Paalanen wrote:
> >>> On Wed, 3 Nov 2021 11:08:13 -0400
> >>> Harry Wentland  wrote:
> >>>   
>  On 2021-09-06 17:38, Uma Shankar wrote:  
> > Existing LUT precision structure is having only 16 bit
> > precision. This is not enough for upcoming enhanced hardwares
> > and advance usecases like HDR processing. Hence added a new
> > structure with 32 bit precision values.
> >
> > This also defines a new structure to define color lut ranges,
> > along with related macro definitions and enums. This will help
> > describe multi segmented lut ranges in the hardware.
> >
> > Signed-off-by: Uma Shankar 
> > ---
> >  include/uapi/drm/drm_mode.h | 58 +
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 90c55383f1ee..1079794c86c3 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -903,6 +903,64 @@ struct hdr_output_metadata {
> > };
> >  };
> >  
> > +/*
> > + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> > + * can be used for either purpose, but not simultaneously. To expose
> > + * modes that support gamma and degamma simultaneously the gamma mode
> > + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> > + * ranges.
> > + */
> > +/* LUT is for gamma (after CTM) */
> > +#define DRM_MODE_LUT_GAMMA BIT(0)
> > +/* LUT is for degamma (before CTM) */
> > +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> > +/* linearly interpolate between the points */
> > +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> > +/*
> > + * the last value of the previous range is the
> > + * first value of the current range.
> > + */
> > +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> > +/* the curve must be non-decreasing */
> > +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> > +/* the curve is reflected across origin for negative inputs */
> > +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> > +/* the same curve (red) is used for blue and green channels as well */
> > +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> > +
> > +struct drm_color_lut_range {
> > +   /* DRM_MODE_LUT_* */
> > +   __u32 flags;
> > +   /* number of points on the curve */
> > +   __u16 count;
> > +   /* input/output bits per component */
> > +   __u8 input_bpc, output_bpc;
> > +   /* input start/end values */
> > +   __s32 start, end;
> > +   /* output min/max values */
> > +   __s32 min, max;
> > +};
> > +
> > +enum lut_type {
> > +   LUT_TYPE_DEGAMMA = 0,
> > +   LUT_TYPE_GAMMA = 1,
> > +};
> > +
> > +/*
> > + * Creating 64 bit palette entries for better data
> > + * precision. This will be required for HDR and
> > + * similar color processing usecases.
> > + */
> > +struct drm_color_lut_ext {
> > +   /*
> > +* Data is U32.32 fixed point format.
> > +*/
> > +   __u64 red;
> > +   __u64 green;
> > +   __u64 blue;
> > +   __u64 reserved;
> > +};
> 
>  I've been drawing out examples of drm_color_lut_range defined PWLs
>  and understand a bit better what you and Ville are trying to accomplish
>  with it. It actually makes a lot of sense and would allow for a generic
>  way to populate different PWL definitions with a generic function.
> 
>  But I still have some key questions that either are not answered in these
>  patch sets or that I somehow overlooked.
> 
>  Can you explain how the U32.32 fixed point format relates to the 
>  input_bpc
>  and output_bpc in drm_color_lut_range, as we as to the pixel coming in 
>  from
>  the framebuffer.
> 
>  E.g. if we have ARGB2101010 what happens to a 0x3ff red value (assuming 
>  alpha
>  is non-multiplied)?
> 
>  If the drm_color_lut_range segments are defined with input_bpc of 24 bpc 
>  will
>  0x3ff be zero-expanded to 24-bit? Is the 24 bpc an integer? I.e. would 
>  our 3xff
>  be interpreted as 0x3ff << (24-10)? 
> 
>  Assuming the output_bpc is 16 bpc and the programmed LUT makes this 
>  1-to-1 would
>  the output value be 0x3ff << (16-10)?
> 
>  On AMD HW the pipe-internal format is a custom floating point format. We 
>  could
>  probably express that in terms of input/output_bpc and do the 
>  translation in our
>  driver between that and the internal floating point format, depending on 
>  the
>  

Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-08 Thread Pekka Paalanen
On Thu, 4 Nov 2021 12:27:56 -0400
Harry Wentland  wrote:

> On 2021-11-04 04:38, Pekka Paalanen wrote:
> > On Wed, 3 Nov 2021 11:08:13 -0400
> > Harry Wentland  wrote:
> >   
> >> On 2021-09-06 17:38, Uma Shankar wrote:  
> >>> Existing LUT precision structure is having only 16 bit
> >>> precision. This is not enough for upcoming enhanced hardwares
> >>> and advance usecases like HDR processing. Hence added a new
> >>> structure with 32 bit precision values.
> >>>
> >>> This also defines a new structure to define color lut ranges,
> >>> along with related macro definitions and enums. This will help
> >>> describe multi segmented lut ranges in the hardware.
> >>>
> >>> Signed-off-by: Uma Shankar 
> >>> ---
> >>>  include/uapi/drm/drm_mode.h | 58 +
> >>>  1 file changed, 58 insertions(+)
> >>>
> >>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >>> index 90c55383f1ee..1079794c86c3 100644
> >>> --- a/include/uapi/drm/drm_mode.h
> >>> +++ b/include/uapi/drm/drm_mode.h
> >>> @@ -903,6 +903,64 @@ struct hdr_output_metadata {
> >>>   };
> >>>  };
> >>>  
> >>> +/*
> >>> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> >>> + * can be used for either purpose, but not simultaneously. To expose
> >>> + * modes that support gamma and degamma simultaneously the gamma mode
> >>> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> >>> + * ranges.
> >>> + */
> >>> +/* LUT is for gamma (after CTM) */
> >>> +#define DRM_MODE_LUT_GAMMA BIT(0)
> >>> +/* LUT is for degamma (before CTM) */
> >>> +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> >>> +/* linearly interpolate between the points */
> >>> +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> >>> +/*
> >>> + * the last value of the previous range is the
> >>> + * first value of the current range.
> >>> + */
> >>> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> >>> +/* the curve must be non-decreasing */
> >>> +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> >>> +/* the curve is reflected across origin for negative inputs */
> >>> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> >>> +/* the same curve (red) is used for blue and green channels as well */
> >>> +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> >>> +
> >>> +struct drm_color_lut_range {
> >>> + /* DRM_MODE_LUT_* */
> >>> + __u32 flags;
> >>> + /* number of points on the curve */
> >>> + __u16 count;
> >>> + /* input/output bits per component */
> >>> + __u8 input_bpc, output_bpc;
> >>> + /* input start/end values */
> >>> + __s32 start, end;
> >>> + /* output min/max values */
> >>> + __s32 min, max;
> >>> +};
> >>> +
> >>> +enum lut_type {
> >>> + LUT_TYPE_DEGAMMA = 0,
> >>> + LUT_TYPE_GAMMA = 1,
> >>> +};
> >>> +
> >>> +/*
> >>> + * Creating 64 bit palette entries for better data
> >>> + * precision. This will be required for HDR and
> >>> + * similar color processing usecases.
> >>> + */
> >>> +struct drm_color_lut_ext {
> >>> + /*
> >>> +  * Data is U32.32 fixed point format.
> >>> +  */
> >>> + __u64 red;
> >>> + __u64 green;
> >>> + __u64 blue;
> >>> + __u64 reserved;
> >>> +};
> >>
> >> I've been drawing out examples of drm_color_lut_range defined PWLs
> >> and understand a bit better what you and Ville are trying to accomplish
> >> with it. It actually makes a lot of sense and would allow for a generic
> >> way to populate different PWL definitions with a generic function.
> >>
> >> But I still have some key questions that either are not answered in these
> >> patch sets or that I somehow overlooked.
> >>
> >> Can you explain how the U32.32 fixed point format relates to the input_bpc
> >> and output_bpc in drm_color_lut_range, as we as to the pixel coming in from
> >> the framebuffer.
> >>
> >> E.g. if we have ARGB2101010 what happens to a 0x3ff red value (assuming 
> >> alpha
> >> is non-multiplied)?
> >>
> >> If the drm_color_lut_range segments are defined with input_bpc of 24 bpc 
> >> will
> >> 0x3ff be zero-expanded to 24-bit? Is the 24 bpc an integer? I.e. would our 
> >> 3xff
> >> be interpreted as 0x3ff << (24-10)? 
> >>
> >> Assuming the output_bpc is 16 bpc and the programmed LUT makes this 1-to-1 
> >> would
> >> the output value be 0x3ff << (16-10)?
> >>
> >> On AMD HW the pipe-internal format is a custom floating point format. We 
> >> could
> >> probably express that in terms of input/output_bpc and do the translation 
> >> in our
> >> driver between that and the internal floating point format, depending on 
> >> the
> >> framebuffer format, but there is the added complication of the magnitude 
> >> of the
> >> pixel data and correlating HDR with SDR planes.
> >>
> >> E.g. any SDR data would map from 0.0 to 1.0 floating point, while HDR 
> >> content would
> >> map from 0.0 to some value larger than 1.0. I don't (yet) have a clear 
> >> picture how
> >> to represent that with the drm_color_lut_range definition.  
> > 
> > 
> > Hi Harry,
> > 
> > I think you just would not. Conceptually an SDR plane gets its 

Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-05 Thread Ville Syrjälä
On Thu, Nov 04, 2021 at 12:27:56PM -0400, Harry Wentland wrote:
> 
> 
> On 2021-11-04 04:38, Pekka Paalanen wrote:
> > On Wed, 3 Nov 2021 11:08:13 -0400
> > Harry Wentland  wrote:
> > 
> >> On 2021-09-06 17:38, Uma Shankar wrote:
> >>> Existing LUT precision structure is having only 16 bit
> >>> precision. This is not enough for upcoming enhanced hardwares
> >>> and advance usecases like HDR processing. Hence added a new
> >>> structure with 32 bit precision values.
> >>>
> >>> This also defines a new structure to define color lut ranges,
> >>> along with related macro definitions and enums. This will help
> >>> describe multi segmented lut ranges in the hardware.
> >>>
> >>> Signed-off-by: Uma Shankar 
> >>> ---
> >>>  include/uapi/drm/drm_mode.h | 58 +
> >>>  1 file changed, 58 insertions(+)
> >>>
> >>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >>> index 90c55383f1ee..1079794c86c3 100644
> >>> --- a/include/uapi/drm/drm_mode.h
> >>> +++ b/include/uapi/drm/drm_mode.h
> >>> @@ -903,6 +903,64 @@ struct hdr_output_metadata {
> >>>   };
> >>>  };
> >>>  
> >>> +/*
> >>> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> >>> + * can be used for either purpose, but not simultaneously. To expose
> >>> + * modes that support gamma and degamma simultaneously the gamma mode
> >>> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> >>> + * ranges.
> >>> + */
> >>> +/* LUT is for gamma (after CTM) */
> >>> +#define DRM_MODE_LUT_GAMMA BIT(0)
> >>> +/* LUT is for degamma (before CTM) */
> >>> +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> >>> +/* linearly interpolate between the points */
> >>> +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> >>> +/*
> >>> + * the last value of the previous range is the
> >>> + * first value of the current range.
> >>> + */
> >>> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> >>> +/* the curve must be non-decreasing */
> >>> +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> >>> +/* the curve is reflected across origin for negative inputs */
> >>> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> >>> +/* the same curve (red) is used for blue and green channels as well */
> >>> +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> >>> +
> >>> +struct drm_color_lut_range {
> >>> + /* DRM_MODE_LUT_* */
> >>> + __u32 flags;
> >>> + /* number of points on the curve */
> >>> + __u16 count;
> >>> + /* input/output bits per component */
> >>> + __u8 input_bpc, output_bpc;
> >>> + /* input start/end values */
> >>> + __s32 start, end;
> >>> + /* output min/max values */
> >>> + __s32 min, max;
> >>> +};
> >>> +
> >>> +enum lut_type {
> >>> + LUT_TYPE_DEGAMMA = 0,
> >>> + LUT_TYPE_GAMMA = 1,
> >>> +};
> >>> +
> >>> +/*
> >>> + * Creating 64 bit palette entries for better data
> >>> + * precision. This will be required for HDR and
> >>> + * similar color processing usecases.
> >>> + */
> >>> +struct drm_color_lut_ext {
> >>> + /*
> >>> +  * Data is U32.32 fixed point format.
> >>> +  */
> >>> + __u64 red;
> >>> + __u64 green;
> >>> + __u64 blue;
> >>> + __u64 reserved;
> >>> +};  
> >>
> >> I've been drawing out examples of drm_color_lut_range defined PWLs
> >> and understand a bit better what you and Ville are trying to accomplish
> >> with it. It actually makes a lot of sense and would allow for a generic
> >> way to populate different PWL definitions with a generic function.
> >>
> >> But I still have some key questions that either are not answered in these
> >> patch sets or that I somehow overlooked.
> >>
> >> Can you explain how the U32.32 fixed point format relates to the input_bpc
> >> and output_bpc in drm_color_lut_range, as we as to the pixel coming in from
> >> the framebuffer.
> >>
> >> E.g. if we have ARGB2101010 what happens to a 0x3ff red value (assuming 
> >> alpha
> >> is non-multiplied)?
> >>
> >> If the drm_color_lut_range segments are defined with input_bpc of 24 bpc 
> >> will
> >> 0x3ff be zero-expanded to 24-bit? Is the 24 bpc an integer? I.e. would our 
> >> 3xff
> >> be interpreted as 0x3ff << (24-10)? 
> >>
> >> Assuming the output_bpc is 16 bpc and the programmed LUT makes this 1-to-1 
> >> would
> >> the output value be 0x3ff << (16-10)?
> >>
> >> On AMD HW the pipe-internal format is a custom floating point format. We 
> >> could
> >> probably express that in terms of input/output_bpc and do the translation 
> >> in our
> >> driver between that and the internal floating point format, depending on 
> >> the
> >> framebuffer format, but there is the added complication of the magnitude 
> >> of the
> >> pixel data and correlating HDR with SDR planes.
> >>
> >> E.g. any SDR data would map from 0.0 to 1.0 floating point, while HDR 
> >> content would
> >> map from 0.0 to some value larger than 1.0. I don't (yet) have a clear 
> >> picture how
> >> to represent that with the drm_color_lut_range definition.
> > 
> > 
> > Hi Harry,
> > 
> > I think you just would not. Conceptually an SDR plane gets 

Re: [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-11-04 Thread Pekka Paalanen
On Wed, 3 Nov 2021 11:08:13 -0400
Harry Wentland  wrote:

> On 2021-09-06 17:38, Uma Shankar wrote:
> > Existing LUT precision structure is having only 16 bit
> > precision. This is not enough for upcoming enhanced hardwares
> > and advance usecases like HDR processing. Hence added a new
> > structure with 32 bit precision values.
> > 
> > This also defines a new structure to define color lut ranges,
> > along with related macro definitions and enums. This will help
> > describe multi segmented lut ranges in the hardware.
> > 
> > Signed-off-by: Uma Shankar 
> > ---
> >  include/uapi/drm/drm_mode.h | 58 +
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 90c55383f1ee..1079794c86c3 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -903,6 +903,64 @@ struct hdr_output_metadata {
> > };
> >  };
> >  
> > +/*
> > + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> > + * can be used for either purpose, but not simultaneously. To expose
> > + * modes that support gamma and degamma simultaneously the gamma mode
> > + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> > + * ranges.
> > + */
> > +/* LUT is for gamma (after CTM) */
> > +#define DRM_MODE_LUT_GAMMA BIT(0)
> > +/* LUT is for degamma (before CTM) */
> > +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> > +/* linearly interpolate between the points */
> > +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> > +/*
> > + * the last value of the previous range is the
> > + * first value of the current range.
> > + */
> > +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> > +/* the curve must be non-decreasing */
> > +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> > +/* the curve is reflected across origin for negative inputs */
> > +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> > +/* the same curve (red) is used for blue and green channels as well */
> > +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> > +
> > +struct drm_color_lut_range {
> > +   /* DRM_MODE_LUT_* */
> > +   __u32 flags;
> > +   /* number of points on the curve */
> > +   __u16 count;
> > +   /* input/output bits per component */
> > +   __u8 input_bpc, output_bpc;
> > +   /* input start/end values */
> > +   __s32 start, end;
> > +   /* output min/max values */
> > +   __s32 min, max;
> > +};
> > +
> > +enum lut_type {
> > +   LUT_TYPE_DEGAMMA = 0,
> > +   LUT_TYPE_GAMMA = 1,
> > +};
> > +
> > +/*
> > + * Creating 64 bit palette entries for better data
> > + * precision. This will be required for HDR and
> > + * similar color processing usecases.
> > + */
> > +struct drm_color_lut_ext {
> > +   /*
> > +* Data is U32.32 fixed point format.
> > +*/
> > +   __u64 red;
> > +   __u64 green;
> > +   __u64 blue;
> > +   __u64 reserved;
> > +};  
> 
> I've been drawing out examples of drm_color_lut_range defined PWLs
> and understand a bit better what you and Ville are trying to accomplish
> with it. It actually makes a lot of sense and would allow for a generic
> way to populate different PWL definitions with a generic function.
> 
> But I still have some key questions that either are not answered in these
> patch sets or that I somehow overlooked.
> 
> Can you explain how the U32.32 fixed point format relates to the input_bpc
> and output_bpc in drm_color_lut_range, as we as to the pixel coming in from
> the framebuffer.
> 
> E.g. if we have ARGB2101010 what happens to a 0x3ff red value (assuming alpha
> is non-multiplied)?
> 
> If the drm_color_lut_range segments are defined with input_bpc of 24 bpc will
> 0x3ff be zero-expanded to 24-bit? Is the 24 bpc an integer? I.e. would our 
> 3xff
> be interpreted as 0x3ff << (24-10)? 
> 
> Assuming the output_bpc is 16 bpc and the programmed LUT makes this 1-to-1 
> would
> the output value be 0x3ff << (16-10)?
> 
> On AMD HW the pipe-internal format is a custom floating point format. We could
> probably express that in terms of input/output_bpc and do the translation in 
> our
> driver between that and the internal floating point format, depending on the
> framebuffer format, but there is the added complication of the magnitude of 
> the
> pixel data and correlating HDR with SDR planes.
> 
> E.g. any SDR data would map from 0.0 to 1.0 floating point, while HDR content 
> would
> map from 0.0 to some value larger than 1.0. I don't (yet) have a clear 
> picture how
> to represent that with the drm_color_lut_range definition.


Hi Harry,

I think you just would not. Conceptually an SDR plane gets its very own
LUT that converts the FB [0.0, 1.0] range to any appropriate [a >= 0.0,
b <= 1.0] range in HDR. This is purely conceptual, in the terms of the
abstract KMS color pipeline, where [0.0, 1.0] is always the full
dynamic range at any point of the pipeline, meaning it is relative to
its placement in the pipeline. If you want to use values >1.0 in hw,
you can do so under the hood.

At 

[Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes

2021-09-06 Thread Uma Shankar
Existing LUT precision structure is having only 16 bit
precision. This is not enough for upcoming enhanced hardwares
and advance usecases like HDR processing. Hence added a new
structure with 32 bit precision values.

This also defines a new structure to define color lut ranges,
along with related macro definitions and enums. This will help
describe multi segmented lut ranges in the hardware.

Signed-off-by: Uma Shankar 
---
 include/uapi/drm/drm_mode.h | 58 +
 1 file changed, 58 insertions(+)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 90c55383f1ee..1079794c86c3 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -903,6 +903,64 @@ struct hdr_output_metadata {
};
 };
 
+/*
+ * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
+ * can be used for either purpose, but not simultaneously. To expose
+ * modes that support gamma and degamma simultaneously the gamma mode
+ * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
+ * ranges.
+ */
+/* LUT is for gamma (after CTM) */
+#define DRM_MODE_LUT_GAMMA BIT(0)
+/* LUT is for degamma (before CTM) */
+#define DRM_MODE_LUT_DEGAMMA BIT(1)
+/* linearly interpolate between the points */
+#define DRM_MODE_LUT_INTERPOLATE BIT(2)
+/*
+ * the last value of the previous range is the
+ * first value of the current range.
+ */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3)
+/* the curve must be non-decreasing */
+#define DRM_MODE_LUT_NON_DECREASING BIT(4)
+/* the curve is reflected across origin for negative inputs */
+#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
+/* the same curve (red) is used for blue and green channels as well */
+#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+
+struct drm_color_lut_range {
+   /* DRM_MODE_LUT_* */
+   __u32 flags;
+   /* number of points on the curve */
+   __u16 count;
+   /* input/output bits per component */
+   __u8 input_bpc, output_bpc;
+   /* input start/end values */
+   __s32 start, end;
+   /* output min/max values */
+   __s32 min, max;
+};
+
+enum lut_type {
+   LUT_TYPE_DEGAMMA = 0,
+   LUT_TYPE_GAMMA = 1,
+};
+
+/*
+ * Creating 64 bit palette entries for better data
+ * precision. This will be required for HDR and
+ * similar color processing usecases.
+ */
+struct drm_color_lut_ext {
+   /*
+* Data is U32.32 fixed point format.
+*/
+   __u64 red;
+   __u64 green;
+   __u64 blue;
+   __u64 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
2.26.2