Re: [PATCH 02/21] drm: Add Plane Degamma Mode property

2021-06-08 Thread Pekka Paalanen
On Mon, 7 Jun 2021 17:34:06 +
"Shankar, Uma"  wrote:

> > -Original Message-
> > From: Harry Wentland 
> > Sent: Friday, June 4, 2021 11:54 PM
> > To: Shankar, Uma ; intel-...@lists.freedesktop.org; 
> > dri-
> > de...@lists.freedesktop.org
> > Cc: Modem, Bhanuprakash ; Cyr, Aric
> > 
> > Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> > 
> > On 2021-06-01 6:51 a.m., Uma Shankar wrote:  
> > > Add Plane Degamma Mode as an enum property. Create a helper function
> > > for all plane color management features.
> > >
> > > This is an enum property with values as blob_id's and exposes the
> > > various gamma modes supported and the lut ranges. Getting the blob id
> > > in userspace, user can get the mode supported and also the range of
> > > gamma mode supported with number of lut coefficients. It can then set
> > > one of the modes using this enum property.
> > >
> > > Lut values will be sent through separate GAMMA_LUT blob property.
> > >
> > > Signed-off-by: Uma Shankar 
> > > ---
> > >  Documentation/gpu/drm-kms.rst | 90 ++
> > >  drivers/gpu/drm/drm_atomic.c  |  1 +
> > >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  4 +
> > >  drivers/gpu/drm/drm_color_mgmt.c  | 93 ++-
> > >  include/drm/drm_mode_object.h |  2 +-
> > >  include/drm/drm_plane.h   | 23 ++
> > >  7 files changed, 212 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/drm-kms.rst
> > > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > > 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -514,9 +514,99 @@ Damage Tracking Properties  Color Management
> > > Properties
> > >  ---
> > >
> > > +Below is how a typical hardware pipeline for color will look like:
> > > +
> > > +.. kernel-render:: DOT
> > > +   :alt: Display Color Pipeline
> > > +   :caption: Display Color Pipeline Overview
> > > +
> > > +   digraph "KMS" {
> > > +  node [shape=box]
> > > +
> > > +  subgraph cluster_static {
> > > +  style=dashed
> > > +  label="Display Color Hardware Blocks"
> > > +
> > > +  node [bgcolor=grey style=filled]
> > > +  "Plane Degamma A" -> "Plane CSC/CTM A"
> > > +  "Plane CSC/CTM A" -> "Plane Gamma A"
> > > +  "Pipe Blender" [color=lightblue,style=filled, width=5.25, 
> > > height=0.75];
> > > +  "Plane Gamma A" -> "Pipe Blender"
> > > +   "Pipe Blender" -> "Pipe DeGamma"
> > > +  "Pipe DeGamma" -> "Pipe CSC/CTM"
> > > +  "Pipe CSC/CTM" -> "Pipe Gamma"
> > > +  "Pipe Gamma" -> "Pipe Output"
> > > +  }
> > > +  
> > 
> > It might be worthwhile to also highlight the YCbCr coefficient matrix in 
> > the pipeline,
> > between the FB and Plane degamma, i.e.
> >   YCbCr coefficients > plane degamma > csc > ...
> > 
> > One problem with this view is that not all HW will support all (or any) of 
> > these CM
> > blocks on all planes. For example, on AMD HW cursors are very different 
> > from other
> > planes and don't really have full CM support.
> >   
> > > +  subgraph cluster_static {
> > > +  style=dashed
> > > +
> > > +  node [shape=box]
> > > +  "Plane Degamma B" -> "Plane CSC/CTM B"
> > > +  "Plane CSC/CTM B" -> "Plane Gamma B"
> > > +  "Plane Gamma B" -> "Pipe Blender"
> > > +  }
> > > +
> > > +  subgraph cluster_static {
> > > +  style=dashed
> > > +
> > > +  node [shape=box]
> > > +  "Plane Degamma C" -> "Plane CSC/CTM C"
> > > +  "Plane CSC/CTM C" -> "Plane Gamma C"
> > > +  "Plane Gamma C" -> "Pipe Blender"
> > > +  }
> > > +
>

RE: [PATCH 02/21] drm: Add Plane Degamma Mode property

2021-06-07 Thread Shankar, Uma


> -Original Message-
> From: Harry Wentland 
> Sent: Friday, June 4, 2021 11:54 PM
> To: Shankar, Uma ; intel-...@lists.freedesktop.org; 
> dri-
> de...@lists.freedesktop.org
> Cc: Modem, Bhanuprakash ; Cyr, Aric
> 
> Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> 
> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper function
> > for all plane color management features.
> >
> > This is an enum property with values as blob_id's and exposes the
> > various gamma modes supported and the lut ranges. Getting the blob id
> > in userspace, user can get the mode supported and also the range of
> > gamma mode supported with number of lut coefficients. It can then set
> > one of the modes using this enum property.
> >
> > Lut values will be sent through separate GAMMA_LUT blob property.
> >
> > Signed-off-by: Uma Shankar 
> > ---
> >  Documentation/gpu/drm-kms.rst | 90 ++
> >  drivers/gpu/drm/drm_atomic.c  |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c  | 93 ++-
> >  include/drm/drm_mode_object.h |  2 +-
> >  include/drm/drm_plane.h   | 23 ++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-kms.rst
> > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking Properties  Color Management
> > Properties
> >  ---
> >
> > +Below is how a typical hardware pipeline for color will look like:
> > +
> > +.. kernel-render:: DOT
> > +   :alt: Display Color Pipeline
> > +   :caption: Display Color Pipeline Overview
> > +
> > +   digraph "KMS" {
> > +  node [shape=box]
> > +
> > +  subgraph cluster_static {
> > +  style=dashed
> > +  label="Display Color Hardware Blocks"
> > +
> > +  node [bgcolor=grey style=filled]
> > +  "Plane Degamma A" -> "Plane CSC/CTM A"
> > +  "Plane CSC/CTM A" -> "Plane Gamma A"
> > +  "Pipe Blender" [color=lightblue,style=filled, width=5.25, 
> > height=0.75];
> > +  "Plane Gamma A" -> "Pipe Blender"
> > + "Pipe Blender" -> "Pipe DeGamma"
> > +  "Pipe DeGamma" -> "Pipe CSC/CTM"
> > +  "Pipe CSC/CTM" -> "Pipe Gamma"
> > +  "Pipe Gamma" -> "Pipe Output"
> > +  }
> > +
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the 
> pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of 
> these CM
> blocks on all planes. For example, on AMD HW cursors are very different from 
> other
> planes and don't really have full CM support.
> 
> > +  subgraph cluster_static {
> > +  style=dashed
> > +
> > +  node [shape=box]
> > +  "Plane Degamma B" -> "Plane CSC/CTM B"
> > +  "Plane CSC/CTM B" -> "Plane Gamma B"
> > +  "Plane Gamma B" -> "Pipe Blender"
> > +  }
> > +
> > +  subgraph cluster_static {
> > +  style=dashed
> > +
> > +  node [shape=box]
> > +  "Plane Degamma C" -> "Plane CSC/CTM C"
> > +  "Plane CSC/CTM C" -> "Plane Gamma C"
> > +  "Plane Gamma C" -> "Pipe Blender"
> > +  }
> > +
> > +  subgraph cluster_fb {
> > +  style=dashed
> > +  label="RAM"
> > +
> > +  node [shape=box width=1.7 height=0.2]
> > +
> > +  "FB 1" -> "Plane Degamma A"
> > +  "FB 2" -> "Plane Degamma B"
> > +  "FB 3" -> "Plane Degamma C"
> > +  }
> > +   }
> > +
> > +In real world usecases,
> > +
> > +1. Plane Degamma can be used to linearize 

Re: [PATCH 02/21] drm: Add Plane Degamma Mode property

2021-06-07 Thread Pekka Paalanen
On Fri, 4 Jun 2021 14:24:28 -0400
Harry Wentland  wrote:

> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper
> > function for all plane color management features.
> > 
> > This is an enum property with values as blob_id's and exposes
> > the various gamma modes supported and the lut ranges. Getting
> > the blob id in userspace, user can get the mode supported and
> > also the range of gamma mode supported with number of lut
> > coefficients. It can then set one of the modes using this
> > enum property.
> > 
> > Lut values will be sent through separate GAMMA_LUT blob property.
> > 
> > Signed-off-by: Uma Shankar 
> > ---
> >  Documentation/gpu/drm-kms.rst | 90 ++
> >  drivers/gpu/drm/drm_atomic.c  |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c  | 93 ++-
> >  include/drm/drm_mode_object.h |  2 +-
> >  include/drm/drm_plane.h   | 23 ++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 87e5023e3f55..752be545e7d7 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking Properties
> >  Color Management Properties
> >  ---
> >  
> > +Below is how a typical hardware pipeline for color
> > +will look like:
> > +
> > +.. kernel-render:: DOT
> > +   :alt: Display Color Pipeline
> > +   :caption: Display Color Pipeline Overview
> > +
> > +   digraph "KMS" {
> > +  node [shape=box]
> > +
> > +  subgraph cluster_static {
> > +  style=dashed
> > +  label="Display Color Hardware Blocks"
> > +
> > +  node [bgcolor=grey style=filled]
> > +  "Plane Degamma A" -> "Plane CSC/CTM A"
> > +  "Plane CSC/CTM A" -> "Plane Gamma A"
> > +  "Pipe Blender" [color=lightblue,style=filled, width=5.25, 
> > height=0.75];
> > +  "Plane Gamma A" -> "Pipe Blender"
> > + "Pipe Blender" -> "Pipe DeGamma"
> > +  "Pipe DeGamma" -> "Pipe CSC/CTM"
> > +  "Pipe CSC/CTM" -> "Pipe Gamma"
> > +  "Pipe Gamma" -> "Pipe Output"
> > +  }
> > +  
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the 
> pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of 
> these
> CM blocks on all planes. For example, on AMD HW cursors are very different 
> from
> other planes and don't really have full CM support.
> 
> > +  subgraph cluster_static {
> > +  style=dashed
> > +
> > +  node [shape=box]
> > +  "Plane Degamma B" -> "Plane CSC/CTM B"
> > +  "Plane CSC/CTM B" -> "Plane Gamma B"
> > +  "Plane Gamma B" -> "Pipe Blender"
> > +  }
> > +
> > +  subgraph cluster_static {
> > +  style=dashed
> > +
> > +  node [shape=box]
> > +  "Plane Degamma C" -> "Plane CSC/CTM C"
> > +  "Plane CSC/CTM C" -> "Plane Gamma C"
> > +  "Plane Gamma C" -> "Pipe Blender"
> > +  }
> > +
> > +  subgraph cluster_fb {
> > +  style=dashed
> > +  label="RAM"
> > +
> > +  node [shape=box width=1.7 height=0.2]
> > +
> > +  "FB 1" -> "Plane Degamma A"
> > +  "FB 2" -> "Plane Degamma B"
> > +  "FB 3" -> "Plane Degamma C"
> > +  }
> > +   }
> > +
> > +In real world usecases,
> > +
> > +1. Plane Degamma can be used to linearize a non linear gamma
> > +encoded framebuffer. This is needed to do any linear math like
> > +color space conversion. For ex, linearize frames encoded in SRGB
> > +or by HDR curve.
> > +
> > +2. Later Plane CTM block can convert the content to some different
> > +colorspace. For ex, SRGB to BT2020 etc.
> > +
> > +3. Plane Gamma block can be used later to re-apply the non-linear
> > +curve. This can also be used to apply Tone Mapping for HDR usecases.
> > +  
> 
> This would mean you're blending in gamma space which is likely not what
> most compositors expect. There are numerous articles that describe why
> blending in gamma space is problematic, such as [1]
> 
> [1] 
> https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html
> 
> To blend in linear space this should be configured to do
> 
>   Plane Degamma > Plane CTM > CRTC Gamma
> 
> I think it would also be good if we moved away from calling this gamma. It's
> really only gamma for legacy SDR scenarios. For HDR cases I would never expect
> these to use gamma and even though the sRGB transfer function is based on 
> gamma
> functions it's complicated [2].
> 
> [2] https://en.wikipedia.org/wiki/SRGB
> 
> A better way to describe these would be as "transfer 

Re: [PATCH 02/21] drm: Add Plane Degamma Mode property

2021-06-04 Thread Harry Wentland
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> Add Plane Degamma Mode as an enum property. Create a helper
> function for all plane color management features.
> 
> This is an enum property with values as blob_id's and exposes
> the various gamma modes supported and the lut ranges. Getting
> the blob id in userspace, user can get the mode supported and
> also the range of gamma mode supported with number of lut
> coefficients. It can then set one of the modes using this
> enum property.
> 
> Lut values will be sent through separate GAMMA_LUT blob property.
> 
> Signed-off-by: Uma Shankar 
> ---
>  Documentation/gpu/drm-kms.rst | 90 ++
>  drivers/gpu/drm/drm_atomic.c  |  1 +
>  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 +
>  drivers/gpu/drm/drm_color_mgmt.c  | 93 ++-
>  include/drm/drm_mode_object.h |  2 +-
>  include/drm/drm_plane.h   | 23 ++
>  7 files changed, 212 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 87e5023e3f55..752be545e7d7 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -514,9 +514,99 @@ Damage Tracking Properties
>  Color Management Properties
>  ---
>  
> +Below is how a typical hardware pipeline for color
> +will look like:
> +
> +.. kernel-render:: DOT
> +   :alt: Display Color Pipeline
> +   :caption: Display Color Pipeline Overview
> +
> +   digraph "KMS" {
> +  node [shape=box]
> +
> +  subgraph cluster_static {
> +  style=dashed
> +  label="Display Color Hardware Blocks"
> +
> +  node [bgcolor=grey style=filled]
> +  "Plane Degamma A" -> "Plane CSC/CTM A"
> +  "Plane CSC/CTM A" -> "Plane Gamma A"
> +  "Pipe Blender" [color=lightblue,style=filled, width=5.25, 
> height=0.75];
> +  "Plane Gamma A" -> "Pipe Blender"
> +   "Pipe Blender" -> "Pipe DeGamma"
> +  "Pipe DeGamma" -> "Pipe CSC/CTM"
> +  "Pipe CSC/CTM" -> "Pipe Gamma"
> +  "Pipe Gamma" -> "Pipe Output"
> +  }
> +

It might be worthwhile to also highlight the YCbCr coefficient matrix in the 
pipeline,
between the FB and Plane degamma, i.e.
  YCbCr coefficients > plane degamma > csc > ...

One problem with this view is that not all HW will support all (or any) of these
CM blocks on all planes. For example, on AMD HW cursors are very different from
other planes and don't really have full CM support.

> +  subgraph cluster_static {
> +  style=dashed
> +
> +  node [shape=box]
> +  "Plane Degamma B" -> "Plane CSC/CTM B"
> +  "Plane CSC/CTM B" -> "Plane Gamma B"
> +  "Plane Gamma B" -> "Pipe Blender"
> +  }
> +
> +  subgraph cluster_static {
> +  style=dashed
> +
> +  node [shape=box]
> +  "Plane Degamma C" -> "Plane CSC/CTM C"
> +  "Plane CSC/CTM C" -> "Plane Gamma C"
> +  "Plane Gamma C" -> "Pipe Blender"
> +  }
> +
> +  subgraph cluster_fb {
> +  style=dashed
> +  label="RAM"
> +
> +  node [shape=box width=1.7 height=0.2]
> +
> +  "FB 1" -> "Plane Degamma A"
> +  "FB 2" -> "Plane Degamma B"
> +  "FB 3" -> "Plane Degamma C"
> +  }
> +   }
> +
> +In real world usecases,
> +
> +1. Plane Degamma can be used to linearize a non linear gamma
> +encoded framebuffer. This is needed to do any linear math like
> +color space conversion. For ex, linearize frames encoded in SRGB
> +or by HDR curve.
> +
> +2. Later Plane CTM block can convert the content to some different
> +colorspace. For ex, SRGB to BT2020 etc.
> +
> +3. Plane Gamma block can be used later to re-apply the non-linear
> +curve. This can also be used to apply Tone Mapping for HDR usecases.
> +

This would mean you're blending in gamma space which is likely not what
most compositors expect. There are numerous articles that describe why
blending in gamma space is problematic, such as [1]

[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html

To blend in linear space this should be configured to do

  Plane Degamma > Plane CTM > CRTC Gamma

I think it would also be good if we moved away from calling this gamma. It's
really only gamma for legacy SDR scenarios. For HDR cases I would never expect
these to use gamma and even though the sRGB transfer function is based on gamma
functions it's complicated [2].

[2] https://en.wikipedia.org/wiki/SRGB

A better way to describe these would be as "transfer function" and "inverse
transfer function." The space at various stages could then be described as 
linear
or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer
function.

Harry

> +All the layers or framebuffers need to be converted to same color
> +space and format before blending. The plane color hardware