On 09/06, Harry Wentland wrote:
> On 2023-08-10 12:02, Melissa Wen wrote:
> > On AMD HW, 3D LUT always assumes a preceding shaper 1D LUT used for
> > delinearizing and/or normalizing the color space before applying a 3D
> > LUT. Add pre-defined transfer function to enable delinearizing content
> > with or without shaper LUT, where AMD color module calculates the
> > resulted shaper curve. We apply an inverse EOTF to go from linear values
> > to encoded values. If we are already in a non-linear space and/or don't
> > need to normalize values, we can bypass shaper LUT with a linear
> > transfer function that is also the default TF value.
> > 
> 
> I think the color module will combine the TF and the custom 1D LUT
> into the LUT that's actually programmed. We should spell out this
> behavior in the comments below and in the patch description as it's
> important for a userspace application to know.
> 
> The same applies to all other TF+LUT blocks.

yeah, you're right, I'll describe it better.

Thanks,

Melissa

> 
> Harry
> 
> > v2:
> > - squash commits for shaper LUT and shaper TF
> > - define inverse EOTF as supported shaper TFs
> > 
> > Signed-off-by: Melissa Wen <m...@igalia.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      | 16 ++++++++++
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++++++
> >  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 29 +++++++++++++++++
> >  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 32 +++++++++++++++++++
> >  4 files changed, 88 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > index 730a88236501..4fb164204ee6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -363,6 +363,22 @@ struct amdgpu_mode_info {
> >      * @plane_hdr_mult_property:
> >      */
> >     struct drm_property *plane_hdr_mult_property;
> > +   /**
> > +    * @shaper_lut_property: Plane property to set pre-blending shaper LUT
> > +    * that converts color content before 3D LUT.
> > +    */
> > +   struct drm_property *plane_shaper_lut_property;
> > +   /**
> > +    * @shaper_lut_size_property: Plane property for the size of
> > +    * pre-blending shaper LUT as supported by the driver (read-only).
> > +    */
> > +   struct drm_property *plane_shaper_lut_size_property;
> > +   /**
> > +    * @plane_shaper_tf_property: Plane property to set a predefined
> > +    * transfer function for pre-blending shaper (before applying 3D LUT)
> > +    * with or without LUT.
> > +    */
> > +   struct drm_property *plane_shaper_tf_property;
> >     /**
> >      * @plane_lut3d_property: Plane property for gamma correction using a
> >      * 3D LUT (pre-blending).
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > index deea90212e31..6b6c2980f0af 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > @@ -769,6 +769,17 @@ struct dm_plane_state {
> >      * S31.32 sign-magnitude.
> >      */
> >     __u64 hdr_mult;
> > +   /**
> > +    * @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
> > +    * array of &struct drm_color_lut.
> > +    */
> > +   struct drm_property_blob *shaper_lut;
> > +   /**
> > +    * @shaper_tf:
> > +    *
> > +    * Predefined transfer function to delinearize color space.
> > +    */
> > +   enum amdgpu_transfer_function shaper_tf;
> >     /**
> >      * @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
> >      * &struct drm_color_lut.
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > index 7e6d4df99a0c..fbcee717bf0a 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > @@ -151,6 +151,14 @@ static const u32 amdgpu_eotf =
> >     BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
> >     BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
> >  
> > +static const u32 amdgpu_inv_eotf =
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF) |
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF) |
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF) |
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF) |
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF) |
> > +   BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF);
> > +
> >  static struct drm_property *
> >  amdgpu_create_tf_property(struct drm_device *dev,
> >                       const char *name,
> > @@ -209,6 +217,27 @@ amdgpu_dm_create_color_properties(struct amdgpu_device 
> > *adev)
> >             return -ENOMEM;
> >     adev->mode_info.plane_hdr_mult_property = prop;
> >  
> > +   prop = drm_property_create(adev_to_drm(adev),
> > +                              DRM_MODE_PROP_BLOB,
> > +                              "AMD_PLANE_SHAPER_LUT", 0);
> > +   if (!prop)
> > +           return -ENOMEM;
> > +   adev->mode_info.plane_shaper_lut_property = prop;
> > +
> > +   prop = drm_property_create_range(adev_to_drm(adev),
> > +                                    DRM_MODE_PROP_IMMUTABLE,
> > +                                    "AMD_PLANE_SHAPER_LUT_SIZE", 0, 
> > UINT_MAX);
> > +   if (!prop)
> > +           return -ENOMEM;
> > +   adev->mode_info.plane_shaper_lut_size_property = prop;
> > +
> > +   prop = amdgpu_create_tf_property(adev_to_drm(adev),
> > +                                    "AMD_PLANE_SHAPER_TF",
> > +                                    amdgpu_inv_eotf);
> > +   if (!prop)
> > +           return -ENOMEM;
> > +   adev->mode_info.plane_shaper_tf_property = prop;
> > +
> >     prop = drm_property_create(adev_to_drm(adev),
> >                                DRM_MODE_PROP_BLOB,
> >                                "AMD_PLANE_LUT3D", 0);
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> > index 882391f7add6..8d6ddf19bb87 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> > @@ -1332,6 +1332,7 @@ static void dm_drm_plane_reset(struct drm_plane 
> > *plane)
> >     __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base);
> >     amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
> >     amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT;
> > +   amdgpu_state->shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT;
> >  }
> >  
> >  static struct drm_plane_state *
> > @@ -1353,11 +1354,14 @@ dm_drm_plane_duplicate_state(struct drm_plane 
> > *plane)
> >  
> >     if (dm_plane_state->degamma_lut)
> >             drm_property_blob_get(dm_plane_state->degamma_lut);
> > +   if (dm_plane_state->shaper_lut)
> > +           drm_property_blob_get(dm_plane_state->shaper_lut);
> >     if (dm_plane_state->lut3d)
> >             drm_property_blob_get(dm_plane_state->lut3d);
> >  
> >     dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
> >     dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
> > +   dm_plane_state->shaper_tf = old_dm_plane_state->shaper_tf;
> >  
> >     return &dm_plane_state->base;
> >  }
> > @@ -1430,6 +1434,8 @@ static void dm_drm_plane_destroy_state(struct 
> > drm_plane *plane,
> >             drm_property_blob_put(dm_plane_state->degamma_lut);
> >     if (dm_plane_state->lut3d)
> >             drm_property_blob_put(dm_plane_state->lut3d);
> > +   if (dm_plane_state->shaper_lut)
> > +           drm_property_blob_put(dm_plane_state->shaper_lut);
> >  
> >     if (dm_plane_state->dc_state)
> >             dc_plane_state_release(dm_plane_state->dc_state);
> > @@ -1462,6 +1468,14 @@ dm_atomic_plane_attach_color_mgmt_properties(struct 
> > amdgpu_display_manager *dm,
> >                                AMDGPU_HDR_MULT_DEFAULT);
> >  
> >     if (dpp_color_caps.hw_3d_lut) {
> > +           drm_object_attach_property(&plane->base,
> > +                                      mode_info.plane_shaper_lut_property, 
> > 0);
> > +           drm_object_attach_property(&plane->base,
> > +                                      
> > mode_info.plane_shaper_lut_size_property,
> > +                                      MAX_COLOR_LUT_ENTRIES);
> > +           drm_object_attach_property(&plane->base,
> > +                                      mode_info.plane_shaper_tf_property,
> > +                                      AMDGPU_TRANSFER_FUNCTION_DEFAULT);
> >             drm_object_attach_property(&plane->base,
> >                                        mode_info.plane_lut3d_property, 0);
> >             drm_object_attach_property(&plane->base,
> > @@ -1499,6 +1513,19 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
> >                     dm_plane_state->hdr_mult = val;
> >                     dm_plane_state->base.color_mgmt_changed = 1;
> >             }
> > +   } else if (property == adev->mode_info.plane_shaper_lut_property) {
> > +           ret = drm_property_replace_blob_from_id(plane->dev,
> > +                                                   
> > &dm_plane_state->shaper_lut,
> > +                                                   val, -1,
> > +                                                   sizeof(struct 
> > drm_color_lut),
> > +                                                   &replaced);
> > +           dm_plane_state->base.color_mgmt_changed |= replaced;
> > +           return ret;
> > +   } else if (property == adev->mode_info.plane_shaper_tf_property) {
> > +           if (dm_plane_state->shaper_tf != val) {
> > +                   dm_plane_state->shaper_tf = val;
> > +                   dm_plane_state->base.color_mgmt_changed = 1;
> > +           }
> >     } else if (property == adev->mode_info.plane_lut3d_property) {
> >             ret = drm_property_replace_blob_from_id(plane->dev,
> >                                                     &dm_plane_state->lut3d,
> > @@ -1534,6 +1561,11 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
> >             *val = dm_plane_state->degamma_tf;
> >     } else if (property == adev->mode_info.plane_hdr_mult_property) {
> >             *val = dm_plane_state->hdr_mult;
> > +   } else  if (property == adev->mode_info.plane_shaper_lut_property) {
> > +           *val = (dm_plane_state->shaper_lut) ?
> > +                   dm_plane_state->shaper_lut->base.id : 0;
> > +   } else if (property == adev->mode_info.plane_shaper_tf_property) {
> > +           *val = dm_plane_state->shaper_tf;
> >     } else  if (property == adev->mode_info.plane_lut3d_property) {
> >             *val = (dm_plane_state->lut3d) ?
> >                     dm_plane_state->lut3d->base.id : 0;
> 

Reply via email to