[RFC PATCH 0/5] DRM CRTC 3D LUT interface for AMD DCN
Melissa Wen
mwen at igalia.com
Tue Jul 12 16:16:29 UTC 2022
On 06/28, Harry Wentland wrote:
>
>
> On 2022-06-19 18:30, Melissa Wen wrote:
> > Hi,
> >
> > I've been working on a proposal to add 3D LUT interface to DRM CRTC
> > color mgmt, that means new **after-blending** properties for color
> > correction. As I'm targeting AMD display drivers, I need to map these
> > new properties to AMD DC interface and I have some doubts about the 3D
> > LUT programming on DCN blocks.
> >
> > First of all, this patchset is a working in progress and further
> > discussions about the DRM interface should be done. I've examined
> > previous proposal to add 3D LUT[1][2] and I understand that the main
> > difference between them is regarding the property position in the DRM
> > color management pipeline (between CTM and Gamma 1D or after Gamma 1D).
> > On the other hand, AMD DC always considers a shaper (1D) LUT before a 3D
> > LUT, used to delinearize and shape the content. These LUTs are then
> > positioned between DRM CTM and Gamma (1D).
> >
> > By comparing the AMD design with the other proposals, I see that it's
> > possible to cover all of them by adding and combining shaper (1D) LUT
> > and 3D LUT as new color mgmt properties. Moreover, it'll not limit the
> > exposure of AMD color correction caps to the userspace. Therefore, my
> > proposal is to add these two new properties in the DRM color mgmt
> > pipeline as follows:
> >
> > +------------+
> > | |
> > | Degamma |
> > +-----+------+
> > |
> > +-----v------+
> > | |
> > | CTM |
> > +-----+------+
> > |
> > ++-----v------++
> > || ||
> > || Shaper LUT ||
> > ++-----+------++
> > |
> > ++-----v------++
> > || ||
> > || 3D LUT ||
> > ++-----+------++
> > |
> > +-----v------+
> > | |
> > | Gamma (1D) |
> > +------------+
> >
>
> As Ville already mentioned on patch 4, the increasing complexity of the
> color pipeline and the arguments about the placement of the 3D LUT means
> that we will probably need a definition of a particular HW's color
> pipeline. Something like this proposal from Sebastian:
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
>
> > However, many doubts arose when I was mapping these two new properties
> > to DC interface. This is why I decided to share an not-yet-completed
> > implementation to get some feedback and explanation.
> >
> > This RFC patchset is divided in three scopes of change. The first two
> > patches document the AMD DM color correction mapping. Some comments were
> > rewritten as kernel doc entries. I also summarize all information
> > provided in previous discussions[3] and also redid those diagrams to
> > svg. All doc should be reviewed and some struct members lack
> > explanation. I can add them to documentation if you can provide a
> > description. Some examples that lack description so far:
> > * in amdgpu_display_manager: dmub_outbox_params, dmub_aux_transfer_done, delayed_hpd_wq;
> > * in dpp_color_caps: dgam_ram, dgam_rom_for_yuv;
> > * in mpc_color_caps: ogam_ram.
> >
> > The next two patches expand DRM color mgmt interface to add shaper LUT
> > and 3D LUT. Finally, the last patch focuses on mapping DRM properties to
> > DC interface and these are my doubts so far:
> >
> > - To configure a shaper LUT, I related it to the current configuration
> > of gamma 1D. For dc_transfer_func, I should set tf according to the
> > input space, that means, LINEAR for shaper LUT after blending, right?
> > When 3D LUT is set, the input space for gamma 1D will no longer be
> > linear, so how to define the tf? should I set tf as sRGB, BT709 or
> > what?
> >
>
> We don't know the input space. It's nowhere defined in the KMS API. It
> entirely depends on how a compositor renders the framebuffer (or transforms
> it using some future KMS plane API).
>
> DC interfaces are designed for a system where the driver is aware of the input
> color space and linearity/non-linearity. This means that you'll often need
> to dig through the API down to the HW programming bits to understand what
> it actually does. A leaky abstraction, essentially.
>
> Because KMS drivers don't know the linearity/non-linearity at any point
> int the pipeline we need an API where the KMS client provides the
> appropriate shaper LUT. In the case of any current KMS client that
> will always be non-colormanaged and is assumed to be sRGB.
>
> If your framebuffer is non-linear (sRGB) and you're not linearizing it
> using the CRTC Degamma you'll already have non-linear values and won't
> need to program the shaper LUT (i.e. use it in bypass or linear).
>
> If your framebuffer is linear and you're not de-linearizing it with the
> CRTC Degamma LUT you'll have linear values and need to program the
> inverse EOTF for sRGB in your shaper (or degamma) LUT.
>
> > - I see the 3dlut values being mapped to struct tetrahedral_17 as four
> > arrays lut0-4. From that I am considering tetrahedral interpolation.
> > Is there any other interpolation option? Also, as the total size of the
> > four arrays is the same of the 3D LUT size, I'm mapping DRM color lut
> > values in ascending order, starting by filling lut0 to lut4. Is it right
> > or is there another way to distribute these values across lut0-4 arrays?
> >
>
> We seem to do this on other platforms (illustrating for red only)
>
> for (lut_i = 0, i = 0; i < lut_size-4; lut_i++, i += 4) {
> lut0[lut_i].red = rgb[i].red;
> lut1[lut_i].red = rgb[i + 1].red;
> lut2[lut_i].red = rgb[i + 2].red;
> lut3[lut_i].red = rgb[i + 3].red;
> }
> lut0[lut_i].red = rgb[i].red;
>
> And yes, it uses tetrahedral interpolation.
>
> > - I also see tetrahedral 9x9x9, that means DC supports 9x9x9 3D LUT too?
> > If so, does it depend on hw caps or it is defined by the user? Also, I
> > see 10 and 12 bits color channel precision, does it depend on hw caps or
> > it is also defined by the userspace? Any ideas on how to expose it?
> >
>
> This is user-configurable, HW supports both 9^3 and 17^3 and both 10 and
> 12-bit variants.
>
> > - Why func_shaper and lut3d_func are defined as constant on
> > dc_stream_state, while the other color properties are not? How should
> > I change them from the proposed DRM properties? should I set 3D LUT in a
> > different struct of the DC interface or a different DC pipeline stage?
> >
>
> It's a pointer to constant struct. If you want to change it you should
> allocate a new func_shaper or lut3d_func struct.
>
> See https://www.internalpointers.com/post/constant-pointers-vs-pointer-constants-c-and-c
>
> > - In mpc3_program_3dlut(), why we are setting is_12bits_color_channel in
> > get3dlut_config(), since right after that we are changing its values
> > with this line `is_12bits_color_channel = params->use_12bits`?
> >
>
> We're reading the HW default to be used unless someone sets use_12bits
> but then we're always setting it based on use_12bits anyways. We wouldn't
> need the former but it's code that's never hurt anyone. :)
>
> > - In mpc3_set3dlut_ram10(), there is a suspicious comment for a shift
> > operation: `should we shift red 22bit and green 12? ask Nvenko` So, is
> > this operation stable/working as expected?
> >
>
> You can safely ignore this, at least as long as your LUT programming works.
> If it doesn't this comment is an indication of what you can try.
Hi Harry,
So, I need to fix many points to program 3D LUT on AMD correctly...
Thanks for explaining all questions, from this, I'm working on a new
version.
Best Regards,
Melissa
>
> Harry
>
> > Thanks in advance for clarifications,
> >
> > Melissa
> >
> > [1] https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+renesas@ideasonboard.com/
> > [2] https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69
> > [3] https://lore.kernel.org/amd-gfx/20220505220744.3sex7ka2ha2vcguv@mail.igalia.com/
> >
> > Melissa Wen (5):
> > Documentation/amdgpu_dm: Add DM color correction documentation
> > Documentation/amdgpu/display: add DC color caps info
> > drm/drm_color_mgmt: add shaper LUT to color mgmt properties
> > drm/drm_color_mgmt: add 3D LUT to color mgmt properties
> > drm/amd/display: mapping new DRM 3D LUT properties to AMD hw blocks
> >
> > .../amdgpu/display/dcn2_cm_drm_current.svg | 1370 +++++++++++++++
> > .../amdgpu/display/dcn3_cm_drm_current.svg | 1528 +++++++++++++++++
> > .../gpu/amdgpu/display/display-manager.rst | 44 +
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 5 +-
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 265 ++-
> > drivers/gpu/drm/amd/display/dc/dc.h | 53 +-
> > drivers/gpu/drm/amd/display/dc/dc_stream.h | 4 +-
> > drivers/gpu/drm/drm_atomic_state_helper.c | 7 +
> > drivers/gpu/drm/drm_atomic_uapi.c | 20 +
> > drivers/gpu/drm/drm_color_mgmt.c | 89 +-
> > drivers/gpu/drm/drm_fb_helper.c | 5 +
> > drivers/gpu/drm/drm_mode_config.c | 28 +
> > include/drm/drm_color_mgmt.h | 4 +
> > include/drm/drm_crtc.h | 24 +-
> > include/drm/drm_mode_config.h | 25 +
> > 16 files changed, 3411 insertions(+), 62 deletions(-)
> > create mode 100644 Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg
> > create mode 100644 Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg
> >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220712/98e41434/attachment-0001.sig>
More information about the dri-devel
mailing list