Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
Hi Pekka thanks for the reply On Thu, Jun 15, 2023 at 10:14:05AM +0300, Pekka Paalanen wrote: > On Tue, 13 Jun 2023 17:43:55 +0200 > Jacopo Mondi wrote: > > > Hello > > > >I'm completing the support for 3D LUT on R-Car DU peripheral > > and I have used this series as a base. > > > > I'm wondering, since quite some time has passed without any update if > > this series is still a thing and it makes any sense for me to try to > > bring it forward. > > > > I'm asking as I've noticed: > > "[PATCH 00/36] drm/amd/display: add AMD driver-specific properties for > > color mgmt" > > > > which seems to supersede this proposal with driver-specific > > properties. > > > > I asked Melissa privately but I wasn't able to get an hold of her, so > > if anyone has any clue feel free to reply :) > > Hi, > > since no-one else replied, I'll point you to the thread starting at > https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html Yes, Melissa pointed me to that series privately yesterday. However, and here I might be missing something, per-plane properties do not apply well to the HW pipeline I'm looking at. The R-Car DU has a 1D LUT and a 3D LUT at the CRTC level (I guess 'post blending' is the right term here) ? A per-plane property doesn't seem to match how the HW work, but please feel free to correct me as this is all rather new to me and I might be overlooking something. My plan at the moment would have been to base my work on Melissa's RFC and re-send to prop discussions, unless it is certainly a dead-end and I have missed how to properly use per-plane properties on our HW. Thank you! > and continuing to June. That is the plan of getting a common UAPI for > these things. > > > Thanks, > pq > > > > > > Thanks > > j > > > > On Mon, Jan 09, 2023 at 01:38:28PM -0100, Melissa Wen wrote: > > > Hi, > > > > > > After collecting comments in different places, here is a second version > > > of the work on adding DRM CRTC 3D LUT support to the current DRM color > > > mgmt interface. In comparison to previous proposals [1][2][3], here we > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, > > > that means the following DRM CRTC color correction pipeline: > > > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT > > > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > > LUT3D_SIZE, that allows userspace to use different supported settings of > > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > > adjusted the pre-blending proposal for post-blending usage. > > > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > > > extra/optional patch to define a default value for LUT3D_MODE, inspired > > > by what we do for the plane blend mode property (pre-multiplied). > > > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current > > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context > > > for DCN 301 (easily extendible to other DCN families). Finally, from > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > > driver, exposing modes supported by HW and programming user shaper and > > > 3D LUT accordingly. > > > > > > Our target userspace is Gamescope/SteamOS. > > > > > > Basic IGT tests were based on [5][6] and are available here (in-progress): > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > > > [1] > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > > [2] > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > > [3] > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > > [4] > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > > [5] https://patchwork.freedesktop.org/series/90165/ > > > [6] https://patchwork.freedesktop.org/series/109402/ > > > [VA_API] > > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > > > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > > > > > Let me know your thoughts. > > > > > > Thanks, > > > > > > Melissa > > > > > > Alex Hung (2): > > > drm: Add 3D LUT mode and its attributes > > > drm/amd/display: Define 3D LUT struct for HDR planes > > > > > > Melissa Wen (16): > > > drm/drm_color_mgmt: add shaper LUT to color mgmt properties > > > drm/drm_color_mgmt: add 3D LUT props to DRM color mgmt > > > drm/drm_color_mgmt: add function to create 3D LUT modes supported > > > drm/drm_color_mgmt: add function to attach 3D LUT props > > >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Thu, 15 Jun 2023 10:07:35 +0200 Jacopo Mondi wrote: > Hi Pekka > thanks for the reply > > On Thu, Jun 15, 2023 at 10:14:05AM +0300, Pekka Paalanen wrote: > > On Tue, 13 Jun 2023 17:43:55 +0200 > > Jacopo Mondi wrote: > > > > > Hello > > > > > >I'm completing the support for 3D LUT on R-Car DU peripheral > > > and I have used this series as a base. > > > > > > I'm wondering, since quite some time has passed without any update if > > > this series is still a thing and it makes any sense for me to try to > > > bring it forward. > > > > > > I'm asking as I've noticed: > > > "[PATCH 00/36] drm/amd/display: add AMD driver-specific properties for > > > color mgmt" > > > > > > which seems to supersede this proposal with driver-specific > > > properties. > > > > > > I asked Melissa privately but I wasn't able to get an hold of her, so > > > if anyone has any clue feel free to reply :) > > > > Hi, > > > > since no-one else replied, I'll point you to the thread starting at > > https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html > > Yes, Melissa pointed me to that series privately yesterday. > > However, and here I might be missing something, per-plane properties do > not apply well to the HW pipeline I'm looking at. > > The R-Car DU has a 1D LUT and a 3D LUT at the CRTC level (I guess > 'post blending' is the right term here) ? A per-plane property > doesn't seem to match how the HW work, but please feel free to correct > me as this is all rather new to me and I might be overlooking > something. Post-blending, correct. The long term plan is to replicate the same idea to post-blending as it is planned for pre-blending. > > My plan at the moment would have been to base my work on Melissa's RFC > and re-send to prop discussions, unless it is certainly a dead-end and > I have missed how to properly use per-plane properties on our HW. I'm not a kernel dev nor a maintainer, so I can't comment on what would be acceptable in the mean time before the new pipeline design is implemented. The long term plan is to supersede all existing color transformation related properties with pipelines. Thanks, pq > > Thank you! > > > and continuing to June. That is the plan of getting a common UAPI for > > these things. > > > > > > Thanks, > > pq > > > > > > > > > > Thanks > > > j > > > > > > On Mon, Jan 09, 2023 at 01:38:28PM -0100, Melissa Wen wrote: > > > > Hi, > > > > > > > > After collecting comments in different places, here is a second version > > > > of the work on adding DRM CRTC 3D LUT support to the current DRM color > > > > mgmt interface. In comparison to previous proposals [1][2][3], here we > > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, > > > > that means the following DRM CRTC color correction pipeline: > > > > > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D > > > > LUT > > > > > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > > > LUT3D_SIZE, that allows userspace to use different supported settings of > > > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > > > adjusted the pre-blending proposal for post-blending usage. > > > > > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to > > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > > > > extra/optional patch to define a default value for LUT3D_MODE, inspired > > > > by what we do for the plane blend mode property (pre-multiplied). > > > > > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage > > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current > > > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context > > > > for DCN 301 (easily extendible to other DCN families). Finally, from > > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > > > driver, exposing modes supported by HW and programming user shaper and > > > > 3D LUT accordingly. > > > > > > > > Our target userspace is Gamescope/SteamOS. > > > > > > > > Basic IGT tests were based on [5][6] and are available here > > > > (in-progress): > > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > > > > > [1] > > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > > > [2] > > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > > > [3] > > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > > > [4] > > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > > > [5] https://patchwork.freedesktop.org/series/90165/ > > > > [6] https://patchwork.freedesktop.org/series/109402/ > > >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Tue, 13 Jun 2023 17:43:55 +0200 Jacopo Mondi wrote: > Hello > >I'm completing the support for 3D LUT on R-Car DU peripheral > and I have used this series as a base. > > I'm wondering, since quite some time has passed without any update if > this series is still a thing and it makes any sense for me to try to > bring it forward. > > I'm asking as I've noticed: > "[PATCH 00/36] drm/amd/display: add AMD driver-specific properties for color > mgmt" > > which seems to supersede this proposal with driver-specific > properties. > > I asked Melissa privately but I wasn't able to get an hold of her, so > if anyone has any clue feel free to reply :) Hi, since no-one else replied, I'll point you to the thread starting at https://lists.freedesktop.org/archives/dri-devel/2023-May/403173.html and continuing to June. That is the plan of getting a common UAPI for these things. Thanks, pq > > Thanks > j > > On Mon, Jan 09, 2023 at 01:38:28PM -0100, Melissa Wen wrote: > > Hi, > > > > After collecting comments in different places, here is a second version > > of the work on adding DRM CRTC 3D LUT support to the current DRM color > > mgmt interface. In comparison to previous proposals [1][2][3], here we > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, > > that means the following DRM CRTC color correction pipeline: > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > LUT3D_SIZE, that allows userspace to use different supported settings of > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > adjusted the pre-blending proposal for post-blending usage. > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > > extra/optional patch to define a default value for LUT3D_MODE, inspired > > by what we do for the plane blend mode property (pre-multiplied). > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context > > for DCN 301 (easily extendible to other DCN families). Finally, from > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > driver, exposing modes supported by HW and programming user shaper and > > 3D LUT accordingly. > > > > Our target userspace is Gamescope/SteamOS. > > > > Basic IGT tests were based on [5][6] and are available here (in-progress): > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > [1] > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > [2] > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > [3] https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > [4] > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > [5] https://patchwork.freedesktop.org/series/90165/ > > [6] https://patchwork.freedesktop.org/series/109402/ > > [VA_API] > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > > > Let me know your thoughts. > > > > Thanks, > > > > Melissa > > > > Alex Hung (2): > > drm: Add 3D LUT mode and its attributes > > drm/amd/display: Define 3D LUT struct for HDR planes > > > > Melissa Wen (16): > > drm/drm_color_mgmt: add shaper LUT to color mgmt properties > > drm/drm_color_mgmt: add 3D LUT props to DRM color mgmt > > drm/drm_color_mgmt: add function to create 3D LUT modes supported > > drm/drm_color_mgmt: add function to attach 3D LUT props > > drm/drm_color_mgmt: set first lut3d mode as default > > drm/amd/display: remove unused regamma condition > > drm/amd/display: add comments to describe DM crtc color mgmt behavior > > drm/amd/display: encapsulate atomic regamma operation > > drm/amd/display: update lut3d and shaper lut to stream > > drm/amd/display: handle MPC 3D LUT resources for a given context > > drm/amd/display: acquire/release 3D LUT resources for ctx on DCN301 > > drm/amd/display: expand array of supported 3D LUT modes > > drm/amd/display: enable 3D-LUT DRM properties if supported > > drm/amd/display: add user 3D LUT support to the amdgpu_dm color > > pipeline > > drm/amd/display: decouple steps to reuse in shaper LUT support > > drm/amd/display: add user shaper LUT support to amdgpu_dm color > > pipeline > > > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 + > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 + > >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
Hello I'm completing the support for 3D LUT on R-Car DU peripheral and I have used this series as a base. I'm wondering, since quite some time has passed without any update if this series is still a thing and it makes any sense for me to try to bring it forward. I'm asking as I've noticed: "[PATCH 00/36] drm/amd/display: add AMD driver-specific properties for color mgmt" which seems to supersede this proposal with driver-specific properties. I asked Melissa privately but I wasn't able to get an hold of her, so if anyone has any clue feel free to reply :) Thanks j On Mon, Jan 09, 2023 at 01:38:28PM -0100, Melissa Wen wrote: > Hi, > > After collecting comments in different places, here is a second version > of the work on adding DRM CRTC 3D LUT support to the current DRM color > mgmt interface. In comparison to previous proposals [1][2][3], here we > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, > that means the following DRM CRTC color correction pipeline: > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > LUT3D_SIZE, that allows userspace to use different supported settings of > 3D LUT, fitting VA-API and new color API better. In this sense, I > adjusted the pre-blending proposal for post-blending usage. > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > extra/optional patch to define a default value for LUT3D_MODE, inspired > by what we do for the plane blend mode property (pre-multiplied). > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context > for DCN 301 (easily extendible to other DCN families). Finally, from > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > driver, exposing modes supported by HW and programming user shaper and > 3D LUT accordingly. > > Our target userspace is Gamescope/SteamOS. > > Basic IGT tests were based on [5][6] and are available here (in-progress): > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > [1] > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > [2] > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > [3] https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > [4] > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > [5] https://patchwork.freedesktop.org/series/90165/ > [6] https://patchwork.freedesktop.org/series/109402/ > [VA_API] > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > Let me know your thoughts. > > Thanks, > > Melissa > > Alex Hung (2): > drm: Add 3D LUT mode and its attributes > drm/amd/display: Define 3D LUT struct for HDR planes > > Melissa Wen (16): > drm/drm_color_mgmt: add shaper LUT to color mgmt properties > drm/drm_color_mgmt: add 3D LUT props to DRM color mgmt > drm/drm_color_mgmt: add function to create 3D LUT modes supported > drm/drm_color_mgmt: add function to attach 3D LUT props > drm/drm_color_mgmt: set first lut3d mode as default > drm/amd/display: remove unused regamma condition > drm/amd/display: add comments to describe DM crtc color mgmt behavior > drm/amd/display: encapsulate atomic regamma operation > drm/amd/display: update lut3d and shaper lut to stream > drm/amd/display: handle MPC 3D LUT resources for a given context > drm/amd/display: acquire/release 3D LUT resources for ctx on DCN301 > drm/amd/display: expand array of supported 3D LUT modes > drm/amd/display: enable 3D-LUT DRM properties if supported > drm/amd/display: add user 3D LUT support to the amdgpu_dm color > pipeline > drm/amd/display: decouple steps to reuse in shaper LUT support > drm/amd/display: add user shaper LUT support to amdgpu_dm color > pipeline > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 + > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 + > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 370 -- > .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 2 + > drivers/gpu/drm/amd/display/dc/core/dc.c | 49 ++- > drivers/gpu/drm/amd/display/dc/dc.h | 8 + > .../amd/display/dc/dcn301/dcn301_resource.c | 47 ++- > .../amd/display/modules/color/color_gamma.h | 43 ++ > drivers/gpu/drm/drm_atomic_state_helper.c | 7 + > drivers/gpu/drm/drm_atomic_uapi.c | 24 ++ > drivers/gpu/drm/drm_color_mgmt.c | 127 ++ >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Tue, 14 Feb 2023 11:19:47 +0200 Pekka Paalanen wrote: > On Mon, 13 Feb 2023 18:26:55 -0100 > Melissa Wen wrote: > > > On 02/10, Pekka Paalanen wrote: > > > On Thu, 9 Feb 2023 13:27:02 -0100 > > > Melissa Wen wrote: > > > > > > > On 01/31, Pekka Paalanen wrote: > > > > > On Mon, 9 Jan 2023 14:38:09 -0100 > > > > > Melissa Wen wrote: > > > > > > > > > > > On 01/09, Melissa Wen wrote: > > > > > > > Hi, > > > > > > > > > > > > > > After collecting comments in different places, here is a second > > > > > > > version > > > > > > > of the work on adding DRM CRTC 3D LUT support to the current DRM > > > > > > > color > > > > > > > mgmt interface. In comparison to previous proposals [1][2][3], > > > > > > > here we > > > > > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before > > > > > > > 3D LUT, > > > > > > > that means the following DRM CRTC color correction pipeline: > > > > > > > > > > > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> > > > > > > > Gamma 1D LUT > > > > > > > > > > Hi Melissa, > > > > > > > > > > that makes sense to me, for CRTCs. It would be really good to have > > > > > that > > > > > as a diagram in the KMS UAPI documentation. > > > > > > > > > > > > > Hi Pekka, > > > > > > > > Thanks for your feedbacks and your time reviewing this proposal. > > > > > > No problem, and sorry it took so long! > > > > > > I'm just finishing the catch-up with everything that happened during > > > winter holidays. > > > > > > > > If someone wants to add a 3D LUT to KMS planes as well, then I'm not > > > > > sure if it should be this order or swapped. I will probably have an > > > > > opinion about that once Weston is fully HDR capable and has been tried > > > > > in the wild for a while with the HDR color operations fine-tuned based > > > > > on community feedback. IOW, not for a long time. The YUV to RGB > > > > > conversion factors in there as well. > > > > > > > > > I see, this is also the reason I reuse here Alex Hung's proposal for > > > > pre-blending API. I'll work on better documentation. > > > > > > > > > > > > > > > > > > > > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > > > > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > > > > > > LUT3D_SIZE, that allows userspace to use different supported > > > > > > > settings of > > > > > > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > > > > > > adjusted the pre-blending proposal for post-blending usage. > > > > > > > > > > > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT > > > > > > > properties to > > > > > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be > > > > > > > considered an > > > > > > > extra/optional patch to define a default value for LUT3D_MODE, > > > > > > > inspired > > > > > > > by what we do for the plane blend mode property (pre-multiplied). > > > > > > > > > > > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT > > > > > > > usage > > > > > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on > > > > > > > current > > > > > > > AMD DM colors code, patch 10 updates AMD stream in case of user > > > > > > > 3D LUT > > > > > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by > > > > > > > context > > > > > > > for DCN 301 (easily extendible to other DCN families). Finally, > > > > > > > from > > > > > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > > > > > > driver, exposing modes supported by HW and programming user > > > > > > > shaper and > > > > > > > 3D LUT accordingly. > > > > > > > > > > > > > > Our target userspace is Gamescope/SteamOS. > > > > > > > > > > > > > > Basic IGT tests were based on [5][6] and are available here > > > > > > > (in-progress): > > > > > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > > > > > > > > > > > [1] > > > > > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > > > > > > [2] > > > > > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > > > > > > [3] > > > > > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > > > > > > [4] > > > > > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > > > > > > [5] https://patchwork.freedesktop.org/series/90165/ > > > > > > > [6] https://patchwork.freedesktop.org/series/109402/ > > > > > > > [VA_API] > > > > > > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > > > > > > > [KMS_pipe_API] > > > > > > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > > > > > > > > > > > > > Let me know your thoughts. > > > > > > > > > > > > +Simon Ser, +Pekka Paalanen who might also be interested in this > > > > > >
RE: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
[AMD Official Use Only - General]
+ Uday, for awareness.
Regards
Shashank
-Original Message-
From: Pekka Paalanen
Sent: 14 February 2023 10:28
To: Melissa Wen
Cc: Ville Syrjälä ;
dri-devel@lists.freedesktop.org; airl...@gmail.com;
laurent.pinchart+rene...@ideasonboard.com; Sharma, Shashank
; Siqueira, Rodrigo ;
amd-...@lists.freedesktop.org; Hung, Alex ; Wentland, Harry
; tzimmerm...@suse.de; Li, Sun peng (Leo)
; maarten.lankho...@linux.intel.com; mrip...@kernel.org;
seanp...@chromium.org; dan...@ffwll.ch; Lakha, Bhawanpreet
; Kim, Sung joon ;
cont...@emersion.fr; Pan, Xinhui ; Koenig, Christian
; kernel-...@igalia.com; Deucher, Alexander
; Kazlauskas, Nicholas
; Joshua Ashton
Subject: Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Mon, 13 Feb 2023 18:45:40 -0100
Melissa Wen wrote:
> On 02/13, Ville Syrjälä wrote:
> > On Mon, Feb 13, 2023 at 11:01:31AM +0200, Pekka Paalanen wrote:
> > > On Fri, 10 Feb 2023 14:47:50 -0500 Harry Wentland
> > > wrote:
> > >
> > > > On 2/10/23 04:28, Pekka Paalanen wrote:
> > > > > On Thu, 9 Feb 2023 13:27:02 -0100 Melissa Wen
> > > > > wrote:
> > > > >
> > > > >> On 01/31, Pekka Paalanen wrote:
> > > > >>> On Mon, 9 Jan 2023 14:38:09 -0100 Melissa Wen
> > > > >>> wrote:
> > > > >>>
> > > > >>>> On 01/09, Melissa Wen wrote:
> > > > >>>>> Hi,
> > > > >>>>>
> > > > >>>>> After collecting comments in different places, here is a
> > > > >>>>> second version of the work on adding DRM CRTC 3D LUT
> > > > >>>>> support to the current DRM color mgmt interface. In
> > > > >>>>> comparison to previous proposals [1][2][3], here we add 3D
> > > > >>>>> LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> > > > >>>>> that means the following DRM CRTC color correction pipeline:
> > > > >>>>>
> > > > >>>>> Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT ->
> > > > >>>>> Gamma 1D LUT
> > >
> > > ...
> > >
> > > > >>> +/*
> > > > >>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> > > > >>> + * @lut_size: number of valid points on every dimension of 3D LUT.
> > > > >>> + * @lut_stride: number of points on every dimension of 3D LUT.
> > > > >>> + * @bit_depth: number of bits of RGB. If color_mode defines
> > > > >>> entries with higher
> > > > >>> + * bit_depth the least significant bits will be
> > > > >>> truncated.
> > > > >>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> > > > >>> DRM_FORMAT_XBGR16161616.
> > > > >>> + * @flags: flags for hardware-sepcific features */ struct
> > > > >>> +drm_mode_lut3d_mode {
> > > > >>> + __u16 lut_size;
> > > > >>> + __u16 lut_stride[3];
> > > > >>> + __u16 bit_depth;
> > > > >>> + __u32 color_format;
> > > > >>> + __u32 flags;
> > > > >>> +};
> > >
> > > ...
> > >
> > > > >>> What is "number of bits of RGB"? Input precision? Output precision?
> > > > >>> Integer or floating point?
> > > > >>
> > > > >> It's the bit depth of the 3D LUT values, the same for every
> > > > >> channels. In
> > > > >> the AMD case, it's supports 10-bit and 12-bit, for example.
> > > > >
> > > > > Ok. So e.g. r5g6b5 is not a possible 3D LUT element type on
> > > > > any hardware ever?
> > > > >
> > > >
> > > > I haven't had a chance to go through all patches yet but if this
> > > > is modeled after Alex Hung's work this should be covered by
> > > > color_format.
> > > > The idea is that color_format takes a FOURCC value and defines
> > > > the format of the entries in the 3DLUT blob.
> > > >
> > > > The bit_depth describes the actual bit depth that the HW supports.
> > > > E.g., color_format could be DRM_FORMAT_XRGB16161616 but HW might
> > >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Mon, 13 Feb 2023 18:45:40 -0100
Melissa Wen wrote:
> On 02/13, Ville Syrjälä wrote:
> > On Mon, Feb 13, 2023 at 11:01:31AM +0200, Pekka Paalanen wrote:
> > > On Fri, 10 Feb 2023 14:47:50 -0500
> > > Harry Wentland wrote:
> > >
> > > > On 2/10/23 04:28, Pekka Paalanen wrote:
> > > > > On Thu, 9 Feb 2023 13:27:02 -0100
> > > > > Melissa Wen wrote:
> > > > >
> > > > >> On 01/31, Pekka Paalanen wrote:
> > > > >>> On Mon, 9 Jan 2023 14:38:09 -0100
> > > > >>> Melissa Wen wrote:
> > > > >>>
> > > > On 01/09, Melissa Wen wrote:
> > > > > Hi,
> > > > >
> > > > > After collecting comments in different places, here is a second
> > > > > version
> > > > > of the work on adding DRM CRTC 3D LUT support to the current DRM
> > > > > color
> > > > > mgmt interface. In comparison to previous proposals [1][2][3],
> > > > > here we
> > > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before
> > > > > 3D LUT,
> > > > > that means the following DRM CRTC color correction pipeline:
> > > > >
> > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT ->
> > > > > Gamma 1D LUT
> > >
> > > ...
> > >
> > > > >>> +/*
> > > > >>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> > > > >>> + * @lut_size: number of valid points on every dimension of 3D LUT.
> > > > >>> + * @lut_stride: number of points on every dimension of 3D LUT.
> > > > >>> + * @bit_depth: number of bits of RGB. If color_mode defines
> > > > >>> entries with higher
> > > > >>> + * bit_depth the least significant bits will be
> > > > >>> truncated.
> > > > >>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> > > > >>> DRM_FORMAT_XBGR16161616.
> > > > >>> + * @flags: flags for hardware-sepcific features
> > > > >>> + */
> > > > >>> +struct drm_mode_lut3d_mode {
> > > > >>> + __u16 lut_size;
> > > > >>> + __u16 lut_stride[3];
> > > > >>> + __u16 bit_depth;
> > > > >>> + __u32 color_format;
> > > > >>> + __u32 flags;
> > > > >>> +};
> > >
> > > ...
> > >
> > > > >>> What is "number of bits of RGB"? Input precision? Output precision?
> > > > >>> Integer or floating point?
> > > > >>
> > > > >> It's the bit depth of the 3D LUT values, the same for every
> > > > >> channels. In
> > > > >> the AMD case, it's supports 10-bit and 12-bit, for example.
> > > > >
> > > > > Ok. So e.g. r5g6b5 is not a possible 3D LUT element type on any
> > > > > hardware ever?
> > > > >
> > > >
> > > > I haven't had a chance to go through all patches yet but if this is
> > > > modeled after Alex Hung's work this should be covered by color_format.
> > > > The idea is that color_format takes a FOURCC value and defines the
> > > > format of the entries in the 3DLUT blob.
> > > >
> > > > The bit_depth describes the actual bit depth that the HW supports.
> > > > E.g., color_format could be DRM_FORMAT_XRGB16161616 but HW might only
> > > > support 12-bit precision. In that case the least significant bits get
> > > > truncated.
> > > >
> > > > One could define the bit_depth per color, but I'm not sure that'll be
> > > > necessary.
> > >
> > > Exactly. I just have no idea how sure we should be about that.
> > >
> > > > > What exactly is the truncation the comment refers to?
> > > > >
> > > > > It sounds like if input has higher precision than the LUT elements,
> > > > > then "truncation" occurs. I can kind of see that, but I also think it
> > > > > is a false characterisation. The LUT input precision affects the
> > > > > precision of LUT indexing and the precision of interpolation between
> > > > > the LUT elements. I would not expect those two precisions to be
> > > > > truncated to the LUT element precision (but they could be truncated to
> > > > > something else hardware specific). Instead, I do expect the
> > > > > interpolation result to be truncated to the LUT output precision,
> > > > > which
> > > > > probably is the same as the LUT element precision, but not
> > > > > necessarily.
> > > > >
> > > > > Maybe the comment about truncation should simply be removed? The
> > > > > result
> > > > > is obvious if we know the LUT input, element, and output precision,
> > > > > and
> > > > > what exactly happens with the indexing and interpolation is probably
> > > > > good enough to be left hardware-specific if it is difficult to
> > > > > describe
> > > > > in generic terms across different hardware.
> > > > >
> > > >
> > > > Maybe it makes sense to just drop the bit_depth field.
> > >
> > > Well, it's really interesting information for userspace, but maybe it
> > > should have a more holistic design. Precision is a factor, when
> > > userspace considers whether it can use KMS hardware for a conversion or
> > > not. Unfortunately, none of the existing KMS color pipeline elements
> > > have any information on precision IIRC, so there is more to be fixed.
> > >
> > > The
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Mon, 13 Feb 2023 18:26:55 -0100 Melissa Wen wrote: > On 02/10, Pekka Paalanen wrote: > > On Thu, 9 Feb 2023 13:27:02 -0100 > > Melissa Wen wrote: > > > > > On 01/31, Pekka Paalanen wrote: > > > > On Mon, 9 Jan 2023 14:38:09 -0100 > > > > Melissa Wen wrote: > > > > > > > > > On 01/09, Melissa Wen wrote: > > > > > > Hi, > > > > > > > > > > > > After collecting comments in different places, here is a second > > > > > > version > > > > > > of the work on adding DRM CRTC 3D LUT support to the current DRM > > > > > > color > > > > > > mgmt interface. In comparison to previous proposals [1][2][3], here > > > > > > we > > > > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D > > > > > > LUT, > > > > > > that means the following DRM CRTC color correction pipeline: > > > > > > > > > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma > > > > > > 1D LUT > > > > > > > > Hi Melissa, > > > > > > > > that makes sense to me, for CRTCs. It would be really good to have that > > > > as a diagram in the KMS UAPI documentation. > > > > > > > > > > Hi Pekka, > > > > > > Thanks for your feedbacks and your time reviewing this proposal. > > > > No problem, and sorry it took so long! > > > > I'm just finishing the catch-up with everything that happened during > > winter holidays. > > > > > > If someone wants to add a 3D LUT to KMS planes as well, then I'm not > > > > sure if it should be this order or swapped. I will probably have an > > > > opinion about that once Weston is fully HDR capable and has been tried > > > > in the wild for a while with the HDR color operations fine-tuned based > > > > on community feedback. IOW, not for a long time. The YUV to RGB > > > > conversion factors in there as well. > > > > > > > I see, this is also the reason I reuse here Alex Hung's proposal for > > > pre-blending API. I'll work on better documentation. > > > > > > > > > > > > > > > > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > > > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > > > > > LUT3D_SIZE, that allows userspace to use different supported > > > > > > settings of > > > > > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > > > > > adjusted the pre-blending proposal for post-blending usage. > > > > > > > > > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT > > > > > > properties to > > > > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered > > > > > > an > > > > > > extra/optional patch to define a default value for LUT3D_MODE, > > > > > > inspired > > > > > > by what we do for the plane blend mode property (pre-multiplied). > > > > > > > > > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT > > > > > > usage > > > > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on > > > > > > current > > > > > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D > > > > > > LUT > > > > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by > > > > > > context > > > > > > for DCN 301 (easily extendible to other DCN families). Finally, from > > > > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > > > > > driver, exposing modes supported by HW and programming user shaper > > > > > > and > > > > > > 3D LUT accordingly. > > > > > > > > > > > > Our target userspace is Gamescope/SteamOS. > > > > > > > > > > > > Basic IGT tests were based on [5][6] and are available here > > > > > > (in-progress): > > > > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > > > > > > > > > [1] > > > > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > > > > > [2] > > > > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > > > > > [3] > > > > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > > > > > [4] > > > > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > > > > > [5] https://patchwork.freedesktop.org/series/90165/ > > > > > > [6] https://patchwork.freedesktop.org/series/109402/ > > > > > > [VA_API] > > > > > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > > > > > > [KMS_pipe_API] > > > > > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > > > > > > > > > > > Let me know your thoughts. > > > > > > > > > > +Simon Ser, +Pekka Paalanen who might also be interested in this > > > > > series. > > > > > > > > Unfortunately I don't have the patch emails to reply to, so here's a > > > > messy bunch of comments. I'll concentrate on the UAPI design as always. > > > > > > > > > > Sorry, the patchset is here: > > > https://lore.kernel.org/dri-devel/20230109143846.1966301-1-m...@igalia.com/ > > > In the
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On 02/13, Ville Syrjälä wrote:
> On Mon, Feb 13, 2023 at 11:01:31AM +0200, Pekka Paalanen wrote:
> > On Fri, 10 Feb 2023 14:47:50 -0500
> > Harry Wentland wrote:
> >
> > > On 2/10/23 04:28, Pekka Paalanen wrote:
> > > > On Thu, 9 Feb 2023 13:27:02 -0100
> > > > Melissa Wen wrote:
> > > >
> > > >> On 01/31, Pekka Paalanen wrote:
> > > >>> On Mon, 9 Jan 2023 14:38:09 -0100
> > > >>> Melissa Wen wrote:
> > > >>>
> > > On 01/09, Melissa Wen wrote:
> > > > Hi,
> > > >
> > > > After collecting comments in different places, here is a second
> > > > version
> > > > of the work on adding DRM CRTC 3D LUT support to the current DRM
> > > > color
> > > > mgmt interface. In comparison to previous proposals [1][2][3], here
> > > > we
> > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D
> > > > LUT,
> > > > that means the following DRM CRTC color correction pipeline:
> > > >
> > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma
> > > > 1D LUT
> >
> > ...
> >
> > > >>> +/*
> > > >>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> > > >>> + * @lut_size: number of valid points on every dimension of 3D LUT.
> > > >>> + * @lut_stride: number of points on every dimension of 3D LUT.
> > > >>> + * @bit_depth: number of bits of RGB. If color_mode defines entries
> > > >>> with higher
> > > >>> + * bit_depth the least significant bits will be
> > > >>> truncated.
> > > >>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> > > >>> DRM_FORMAT_XBGR16161616.
> > > >>> + * @flags: flags for hardware-sepcific features
> > > >>> + */
> > > >>> +struct drm_mode_lut3d_mode {
> > > >>> + __u16 lut_size;
> > > >>> + __u16 lut_stride[3];
> > > >>> + __u16 bit_depth;
> > > >>> + __u32 color_format;
> > > >>> + __u32 flags;
> > > >>> +};
> >
> > ...
> >
> > > >>> What is "number of bits of RGB"? Input precision? Output precision?
> > > >>> Integer or floating point?
> > > >>
> > > >> It's the bit depth of the 3D LUT values, the same for every channels.
> > > >> In
> > > >> the AMD case, it's supports 10-bit and 12-bit, for example.
> > > >
> > > > Ok. So e.g. r5g6b5 is not a possible 3D LUT element type on any
> > > > hardware ever?
> > > >
> > >
> > > I haven't had a chance to go through all patches yet but if this is
> > > modeled after Alex Hung's work this should be covered by color_format.
> > > The idea is that color_format takes a FOURCC value and defines the
> > > format of the entries in the 3DLUT blob.
> > >
> > > The bit_depth describes the actual bit depth that the HW supports.
> > > E.g., color_format could be DRM_FORMAT_XRGB16161616 but HW might only
> > > support 12-bit precision. In that case the least significant bits get
> > > truncated.
> > >
> > > One could define the bit_depth per color, but I'm not sure that'll be
> > > necessary.
> >
> > Exactly. I just have no idea how sure we should be about that.
> >
> > > > What exactly is the truncation the comment refers to?
> > > >
> > > > It sounds like if input has higher precision than the LUT elements,
> > > > then "truncation" occurs. I can kind of see that, but I also think it
> > > > is a false characterisation. The LUT input precision affects the
> > > > precision of LUT indexing and the precision of interpolation between
> > > > the LUT elements. I would not expect those two precisions to be
> > > > truncated to the LUT element precision (but they could be truncated to
> > > > something else hardware specific). Instead, I do expect the
> > > > interpolation result to be truncated to the LUT output precision, which
> > > > probably is the same as the LUT element precision, but not necessarily.
> > > >
> > > > Maybe the comment about truncation should simply be removed? The result
> > > > is obvious if we know the LUT input, element, and output precision, and
> > > > what exactly happens with the indexing and interpolation is probably
> > > > good enough to be left hardware-specific if it is difficult to describe
> > > > in generic terms across different hardware.
> > > >
> > >
> > > Maybe it makes sense to just drop the bit_depth field.
> >
> > Well, it's really interesting information for userspace, but maybe it
> > should have a more holistic design. Precision is a factor, when
> > userspace considers whether it can use KMS hardware for a conversion or
> > not. Unfortunately, none of the existing KMS color pipeline elements
> > have any information on precision IIRC, so there is more to be fixed.
> >
> > The interesting thing is the minimum guaranteed precision of each
> > element and the connections between them. It might be different for
> > pass-through vs. not. Another interesting thing is the usable value
> > range.
> >
> > This is probably a complex problem, so there should be no need to solve
> > it before a 3D LUT interface can land, given old
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On 02/10, Pekka Paalanen wrote: > On Thu, 9 Feb 2023 13:27:02 -0100 > Melissa Wen wrote: > > > On 01/31, Pekka Paalanen wrote: > > > On Mon, 9 Jan 2023 14:38:09 -0100 > > > Melissa Wen wrote: > > > > > > > On 01/09, Melissa Wen wrote: > > > > > Hi, > > > > > > > > > > After collecting comments in different places, here is a second > > > > > version > > > > > of the work on adding DRM CRTC 3D LUT support to the current DRM color > > > > > mgmt interface. In comparison to previous proposals [1][2][3], here we > > > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D > > > > > LUT, > > > > > that means the following DRM CRTC color correction pipeline: > > > > > > > > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D > > > > > LUT > > > > > > Hi Melissa, > > > > > > that makes sense to me, for CRTCs. It would be really good to have that > > > as a diagram in the KMS UAPI documentation. > > > > > > > Hi Pekka, > > > > Thanks for your feedbacks and your time reviewing this proposal. > > No problem, and sorry it took so long! > > I'm just finishing the catch-up with everything that happened during > winter holidays. > > > > If someone wants to add a 3D LUT to KMS planes as well, then I'm not > > > sure if it should be this order or swapped. I will probably have an > > > opinion about that once Weston is fully HDR capable and has been tried > > > in the wild for a while with the HDR color operations fine-tuned based > > > on community feedback. IOW, not for a long time. The YUV to RGB > > > conversion factors in there as well. > > > > > I see, this is also the reason I reuse here Alex Hung's proposal for > > pre-blending API. I'll work on better documentation. > > > > > > > > > > > > > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > > > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > > > > > LUT3D_SIZE, that allows userspace to use different supported settings > > > > > of > > > > > 3D LUT, fitting VA-API and new color API better. In this sense, I > > > > > adjusted the pre-blending proposal for post-blending usage. > > > > > > > > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties > > > > > to > > > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > > > > > extra/optional patch to define a default value for LUT3D_MODE, > > > > > inspired > > > > > by what we do for the plane blend mode property (pre-multiplied). > > > > > > > > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT > > > > > usage > > > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on > > > > > current > > > > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > > > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by > > > > > context > > > > > for DCN 301 (easily extendible to other DCN families). Finally, from > > > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > > > > > driver, exposing modes supported by HW and programming user shaper and > > > > > 3D LUT accordingly. > > > > > > > > > > Our target userspace is Gamescope/SteamOS. > > > > > > > > > > Basic IGT tests were based on [5][6] and are available here > > > > > (in-progress): > > > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > > > > > > > > > [1] > > > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > > > > > [2] > > > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > > > > > [3] > > > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > > > > > [4] > > > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > > > > > [5] https://patchwork.freedesktop.org/series/90165/ > > > > > [6] https://patchwork.freedesktop.org/series/109402/ > > > > > [VA_API] > > > > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > > > > > [KMS_pipe_API] > > > > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > > > > > > > > > Let me know your thoughts. > > > > > > > > +Simon Ser, +Pekka Paalanen who might also be interested in this > > > > series. > > > > > > Unfortunately I don't have the patch emails to reply to, so here's a > > > messy bunch of comments. I'll concentrate on the UAPI design as always. > > > > Sorry, the patchset is here: > > https://lore.kernel.org/dri-devel/20230109143846.1966301-1-m...@igalia.com/ > > In the next version, I won't forget cc'ing you at first. > > > > > > +/* > > > + * struct drm_mode_lut3d_mode - 3D LUT mode information. > > > + * @lut_size: number of valid points on every dimension of 3D LUT. > > > + * @lut_stride: number of points on every dimension of 3D LUT. > > > + * @bit_depth: number of bits of RGB. If color_mode defines entries with > > > higher > > >
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Mon, Feb 13, 2023 at 11:01:31AM +0200, Pekka Paalanen wrote:
> On Fri, 10 Feb 2023 14:47:50 -0500
> Harry Wentland wrote:
>
> > On 2/10/23 04:28, Pekka Paalanen wrote:
> > > On Thu, 9 Feb 2023 13:27:02 -0100
> > > Melissa Wen wrote:
> > >
> > >> On 01/31, Pekka Paalanen wrote:
> > >>> On Mon, 9 Jan 2023 14:38:09 -0100
> > >>> Melissa Wen wrote:
> > >>>
> > On 01/09, Melissa Wen wrote:
> > > Hi,
> > >
> > > After collecting comments in different places, here is a second
> > > version
> > > of the work on adding DRM CRTC 3D LUT support to the current DRM color
> > > mgmt interface. In comparison to previous proposals [1][2][3], here we
> > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D
> > > LUT,
> > > that means the following DRM CRTC color correction pipeline:
> > >
> > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D
> > > LUT
>
> ...
>
> > >>> +/*
> > >>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> > >>> + * @lut_size: number of valid points on every dimension of 3D LUT.
> > >>> + * @lut_stride: number of points on every dimension of 3D LUT.
> > >>> + * @bit_depth: number of bits of RGB. If color_mode defines entries
> > >>> with higher
> > >>> + * bit_depth the least significant bits will be truncated.
> > >>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> > >>> DRM_FORMAT_XBGR16161616.
> > >>> + * @flags: flags for hardware-sepcific features
> > >>> + */
> > >>> +struct drm_mode_lut3d_mode {
> > >>> + __u16 lut_size;
> > >>> + __u16 lut_stride[3];
> > >>> + __u16 bit_depth;
> > >>> + __u32 color_format;
> > >>> + __u32 flags;
> > >>> +};
>
> ...
>
> > >>> What is "number of bits of RGB"? Input precision? Output precision?
> > >>> Integer or floating point?
> > >>
> > >> It's the bit depth of the 3D LUT values, the same for every channels. In
> > >> the AMD case, it's supports 10-bit and 12-bit, for example.
> > >
> > > Ok. So e.g. r5g6b5 is not a possible 3D LUT element type on any
> > > hardware ever?
> > >
> >
> > I haven't had a chance to go through all patches yet but if this is
> > modeled after Alex Hung's work this should be covered by color_format.
> > The idea is that color_format takes a FOURCC value and defines the
> > format of the entries in the 3DLUT blob.
> >
> > The bit_depth describes the actual bit depth that the HW supports.
> > E.g., color_format could be DRM_FORMAT_XRGB16161616 but HW might only
> > support 12-bit precision. In that case the least significant bits get
> > truncated.
> >
> > One could define the bit_depth per color, but I'm not sure that'll be
> > necessary.
>
> Exactly. I just have no idea how sure we should be about that.
>
> > > What exactly is the truncation the comment refers to?
> > >
> > > It sounds like if input has higher precision than the LUT elements,
> > > then "truncation" occurs. I can kind of see that, but I also think it
> > > is a false characterisation. The LUT input precision affects the
> > > precision of LUT indexing and the precision of interpolation between
> > > the LUT elements. I would not expect those two precisions to be
> > > truncated to the LUT element precision (but they could be truncated to
> > > something else hardware specific). Instead, I do expect the
> > > interpolation result to be truncated to the LUT output precision, which
> > > probably is the same as the LUT element precision, but not necessarily.
> > >
> > > Maybe the comment about truncation should simply be removed? The result
> > > is obvious if we know the LUT input, element, and output precision, and
> > > what exactly happens with the indexing and interpolation is probably
> > > good enough to be left hardware-specific if it is difficult to describe
> > > in generic terms across different hardware.
> > >
> >
> > Maybe it makes sense to just drop the bit_depth field.
>
> Well, it's really interesting information for userspace, but maybe it
> should have a more holistic design. Precision is a factor, when
> userspace considers whether it can use KMS hardware for a conversion or
> not. Unfortunately, none of the existing KMS color pipeline elements
> have any information on precision IIRC, so there is more to be fixed.
>
> The interesting thing is the minimum guaranteed precision of each
> element and the connections between them. It might be different for
> pass-through vs. not. Another interesting thing is the usable value
> range.
>
> This is probably a complex problem, so there should be no need to solve
> it before a 3D LUT interface can land, given old elements already have
> the issue.
Yeah, I think all the precision stuff is all better handled by
eg. the proposed GAMMA_MODE property or something similar.
It's going to be needed for 1D LUTs as well. 1D LUTs would
also need it to expose diffrent LUT sizes with different
precision
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Fri, 10 Feb 2023 14:47:50 -0500
Harry Wentland wrote:
> On 2/10/23 04:28, Pekka Paalanen wrote:
> > On Thu, 9 Feb 2023 13:27:02 -0100
> > Melissa Wen wrote:
> >
> >> On 01/31, Pekka Paalanen wrote:
> >>> On Mon, 9 Jan 2023 14:38:09 -0100
> >>> Melissa Wen wrote:
> >>>
> On 01/09, Melissa Wen wrote:
> > Hi,
> >
> > After collecting comments in different places, here is a second version
> > of the work on adding DRM CRTC 3D LUT support to the current DRM color
> > mgmt interface. In comparison to previous proposals [1][2][3], here we
> > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> > that means the following DRM CRTC color correction pipeline:
> >
> > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D
> > LUT
...
> >>> +/*
> >>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> >>> + * @lut_size: number of valid points on every dimension of 3D LUT.
> >>> + * @lut_stride: number of points on every dimension of 3D LUT.
> >>> + * @bit_depth: number of bits of RGB. If color_mode defines entries with
> >>> higher
> >>> + * bit_depth the least significant bits will be truncated.
> >>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> >>> DRM_FORMAT_XBGR16161616.
> >>> + * @flags: flags for hardware-sepcific features
> >>> + */
> >>> +struct drm_mode_lut3d_mode {
> >>> + __u16 lut_size;
> >>> + __u16 lut_stride[3];
> >>> + __u16 bit_depth;
> >>> + __u32 color_format;
> >>> + __u32 flags;
> >>> +};
...
> >>> What is "number of bits of RGB"? Input precision? Output precision?
> >>> Integer or floating point?
> >>
> >> It's the bit depth of the 3D LUT values, the same for every channels. In
> >> the AMD case, it's supports 10-bit and 12-bit, for example.
> >
> > Ok. So e.g. r5g6b5 is not a possible 3D LUT element type on any
> > hardware ever?
> >
>
> I haven't had a chance to go through all patches yet but if this is
> modeled after Alex Hung's work this should be covered by color_format.
> The idea is that color_format takes a FOURCC value and defines the
> format of the entries in the 3DLUT blob.
>
> The bit_depth describes the actual bit depth that the HW supports.
> E.g., color_format could be DRM_FORMAT_XRGB16161616 but HW might only
> support 12-bit precision. In that case the least significant bits get
> truncated.
>
> One could define the bit_depth per color, but I'm not sure that'll be
> necessary.
Exactly. I just have no idea how sure we should be about that.
> > What exactly is the truncation the comment refers to?
> >
> > It sounds like if input has higher precision than the LUT elements,
> > then "truncation" occurs. I can kind of see that, but I also think it
> > is a false characterisation. The LUT input precision affects the
> > precision of LUT indexing and the precision of interpolation between
> > the LUT elements. I would not expect those two precisions to be
> > truncated to the LUT element precision (but they could be truncated to
> > something else hardware specific). Instead, I do expect the
> > interpolation result to be truncated to the LUT output precision, which
> > probably is the same as the LUT element precision, but not necessarily.
> >
> > Maybe the comment about truncation should simply be removed? The result
> > is obvious if we know the LUT input, element, and output precision, and
> > what exactly happens with the indexing and interpolation is probably
> > good enough to be left hardware-specific if it is difficult to describe
> > in generic terms across different hardware.
> >
>
> Maybe it makes sense to just drop the bit_depth field.
Well, it's really interesting information for userspace, but maybe it
should have a more holistic design. Precision is a factor, when
userspace considers whether it can use KMS hardware for a conversion or
not. Unfortunately, none of the existing KMS color pipeline elements
have any information on precision IIRC, so there is more to be fixed.
The interesting thing is the minimum guaranteed precision of each
element and the connections between them. It might be different for
pass-through vs. not. Another interesting thing is the usable value
range.
This is probably a complex problem, so there should be no need to solve
it before a 3D LUT interface can land, given old elements already have
the issue.
Thanks,
pq
pgp3v1OaQ61Lx.pgp
Description: OpenPGP digital signature
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On 2/10/23 04:28, Pekka Paalanen wrote:
> On Thu, 9 Feb 2023 13:27:02 -0100
> Melissa Wen wrote:
>
>> On 01/31, Pekka Paalanen wrote:
>>> On Mon, 9 Jan 2023 14:38:09 -0100
>>> Melissa Wen wrote:
>>>
On 01/09, Melissa Wen wrote:
> Hi,
>
> After collecting comments in different places, here is a second version
> of the work on adding DRM CRTC 3D LUT support to the current DRM color
> mgmt interface. In comparison to previous proposals [1][2][3], here we
> add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> that means the following DRM CRTC color correction pipeline:
>
> Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT
>
>>>
>>> Hi Melissa,
>>>
>>> that makes sense to me, for CRTCs. It would be really good to have that
>>> as a diagram in the KMS UAPI documentation.
>>>
>>
>> Hi Pekka,
>>
>> Thanks for your feedbacks and your time reviewing this proposal.
>
> No problem, and sorry it took so long!
>
> I'm just finishing the catch-up with everything that happened during
> winter holidays.
>
>>> If someone wants to add a 3D LUT to KMS planes as well, then I'm not
>>> sure if it should be this order or swapped. I will probably have an
>>> opinion about that once Weston is fully HDR capable and has been tried
>>> in the wild for a while with the HDR color operations fine-tuned based
>>> on community feedback. IOW, not for a long time. The YUV to RGB
>>> conversion factors in there as well.
>>>
>> I see, this is also the reason I reuse here Alex Hung's proposal for
>> pre-blending API. I'll work on better documentation.
>>
>>>
>
> and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung
> proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a
> LUT3D_SIZE, that allows userspace to use different supported settings of
> 3D LUT, fitting VA-API and new color API better. In this sense, I
> adjusted the pre-blending proposal for post-blending usage.
>
> Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to
> the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an
> extra/optional patch to define a default value for LUT3D_MODE, inspired
> by what we do for the plane blend mode property (pre-multiplied).
>
> Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage
> on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current
> AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT
> changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context
> for DCN 301 (easily extendible to other DCN families). Finally, from
> 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display
> driver, exposing modes supported by HW and programming user shaper and
> 3D LUT accordingly.
>
> Our target userspace is Gamescope/SteamOS.
>
> Basic IGT tests were based on [5][6] and are available here (in-progress):
> https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api
>
> [1]
> https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/
> [2]
> https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69
> [3]
> https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/
> [4]
> https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/
> [5] https://patchwork.freedesktop.org/series/90165/
> [6] https://patchwork.freedesktop.org/series/109402/
> [VA_API]
> http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html
> [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
>
> Let me know your thoughts.
+Simon Ser, +Pekka Paalanen who might also be interested in this series.
>>>
>>> Unfortunately I don't have the patch emails to reply to, so here's a
>>> messy bunch of comments. I'll concentrate on the UAPI design as always.
>>
>> Sorry, the patchset is here:
>> https://lore.kernel.org/dri-devel/20230109143846.1966301-1-m...@igalia.com/
>> In the next version, I won't forget cc'ing you at first.
>>>
>>> +/*
>>> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
>>> + * @lut_size: number of valid points on every dimension of 3D LUT.
>>> + * @lut_stride: number of points on every dimension of 3D LUT.
>>> + * @bit_depth: number of bits of RGB. If color_mode defines entries with
>>> higher
>>> + * bit_depth the least significant bits will be truncated.
>>> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
>>> DRM_FORMAT_XBGR16161616.
>>> + * @flags: flags for hardware-sepcific features
>>> + */
>>> +struct drm_mode_lut3d_mode {
>>> + __u16 lut_size;
>>> + __u16 lut_stride[3];
>>> + __u16 bit_depth;
>>> + __u32 color_format;
>>> + __u32 flags;
>>> +};
>>>
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Thu, 9 Feb 2023 13:27:02 -0100
Melissa Wen wrote:
> On 01/31, Pekka Paalanen wrote:
> > On Mon, 9 Jan 2023 14:38:09 -0100
> > Melissa Wen wrote:
> >
> > > On 01/09, Melissa Wen wrote:
> > > > Hi,
> > > >
> > > > After collecting comments in different places, here is a second version
> > > > of the work on adding DRM CRTC 3D LUT support to the current DRM color
> > > > mgmt interface. In comparison to previous proposals [1][2][3], here we
> > > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> > > > that means the following DRM CRTC color correction pipeline:
> > > >
> > > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D
> > > > LUT
> >
> > Hi Melissa,
> >
> > that makes sense to me, for CRTCs. It would be really good to have that
> > as a diagram in the KMS UAPI documentation.
> >
>
> Hi Pekka,
>
> Thanks for your feedbacks and your time reviewing this proposal.
No problem, and sorry it took so long!
I'm just finishing the catch-up with everything that happened during
winter holidays.
> > If someone wants to add a 3D LUT to KMS planes as well, then I'm not
> > sure if it should be this order or swapped. I will probably have an
> > opinion about that once Weston is fully HDR capable and has been tried
> > in the wild for a while with the HDR color operations fine-tuned based
> > on community feedback. IOW, not for a long time. The YUV to RGB
> > conversion factors in there as well.
> >
> I see, this is also the reason I reuse here Alex Hung's proposal for
> pre-blending API. I'll work on better documentation.
>
> >
> > > >
> > > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung
> > > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a
> > > > LUT3D_SIZE, that allows userspace to use different supported settings of
> > > > 3D LUT, fitting VA-API and new color API better. In this sense, I
> > > > adjusted the pre-blending proposal for post-blending usage.
> > > >
> > > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to
> > > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an
> > > > extra/optional patch to define a default value for LUT3D_MODE, inspired
> > > > by what we do for the plane blend mode property (pre-multiplied).
> > > >
> > > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage
> > > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current
> > > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT
> > > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context
> > > > for DCN 301 (easily extendible to other DCN families). Finally, from
> > > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display
> > > > driver, exposing modes supported by HW and programming user shaper and
> > > > 3D LUT accordingly.
> > > >
> > > > Our target userspace is Gamescope/SteamOS.
> > > >
> > > > Basic IGT tests were based on [5][6] and are available here
> > > > (in-progress):
> > > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api
> > > >
> > > > [1]
> > > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/
> > > > [2]
> > > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69
> > > > [3]
> > > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/
> > > > [4]
> > > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/
> > > > [5] https://patchwork.freedesktop.org/series/90165/
> > > > [6] https://patchwork.freedesktop.org/series/109402/
> > > > [VA_API]
> > > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html
> > > > [KMS_pipe_API]
> > > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
> > > >
> > > > Let me know your thoughts.
> > >
> > > +Simon Ser, +Pekka Paalanen who might also be interested in this series.
> >
> > Unfortunately I don't have the patch emails to reply to, so here's a
> > messy bunch of comments. I'll concentrate on the UAPI design as always.
>
> Sorry, the patchset is here:
> https://lore.kernel.org/dri-devel/20230109143846.1966301-1-m...@igalia.com/
> In the next version, I won't forget cc'ing you at first.
> >
> > +/*
> > + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> > + * @lut_size: number of valid points on every dimension of 3D LUT.
> > + * @lut_stride: number of points on every dimension of 3D LUT.
> > + * @bit_depth: number of bits of RGB. If color_mode defines entries with
> > higher
> > + * bit_depth the least significant bits will be truncated.
> > + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> > DRM_FORMAT_XBGR16161616.
> > + * @flags: flags for hardware-sepcific features
> > + */
> > +struct drm_mode_lut3d_mode {
> > + __u16 lut_size;
> > + __u16 lut_stride[3];
> > + __u16
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On 01/31, Pekka Paalanen wrote:
> On Mon, 9 Jan 2023 14:38:09 -0100
> Melissa Wen wrote:
>
> > On 01/09, Melissa Wen wrote:
> > > Hi,
> > >
> > > After collecting comments in different places, here is a second version
> > > of the work on adding DRM CRTC 3D LUT support to the current DRM color
> > > mgmt interface. In comparison to previous proposals [1][2][3], here we
> > > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> > > that means the following DRM CRTC color correction pipeline:
> > >
> > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT
>
> Hi Melissa,
>
> that makes sense to me, for CRTCs. It would be really good to have that
> as a diagram in the KMS UAPI documentation.
>
Hi Pekka,
Thanks for your feedbacks and your time reviewing this proposal.
> If someone wants to add a 3D LUT to KMS planes as well, then I'm not
> sure if it should be this order or swapped. I will probably have an
> opinion about that once Weston is fully HDR capable and has been tried
> in the wild for a while with the HDR color operations fine-tuned based
> on community feedback. IOW, not for a long time. The YUV to RGB
> conversion factors in there as well.
>
I see, this is also the reason I reuse here Alex Hung's proposal for
pre-blending API. I'll work on better documentation.
>
> > >
> > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung
> > > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a
> > > LUT3D_SIZE, that allows userspace to use different supported settings of
> > > 3D LUT, fitting VA-API and new color API better. In this sense, I
> > > adjusted the pre-blending proposal for post-blending usage.
> > >
> > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to
> > > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an
> > > extra/optional patch to define a default value for LUT3D_MODE, inspired
> > > by what we do for the plane blend mode property (pre-multiplied).
> > >
> > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage
> > > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current
> > > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT
> > > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context
> > > for DCN 301 (easily extendible to other DCN families). Finally, from
> > > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display
> > > driver, exposing modes supported by HW and programming user shaper and
> > > 3D LUT accordingly.
> > >
> > > Our target userspace is Gamescope/SteamOS.
> > >
> > > Basic IGT tests were based on [5][6] and are available here (in-progress):
> > > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api
> > >
> > > [1]
> > > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/
> > > [2]
> > > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69
> > > [3]
> > > https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/
> > > [4]
> > > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/
> > > [5] https://patchwork.freedesktop.org/series/90165/
> > > [6] https://patchwork.freedesktop.org/series/109402/
> > > [VA_API]
> > > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html
> > > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
> > >
> > > Let me know your thoughts.
> >
> > +Simon Ser, +Pekka Paalanen who might also be interested in this series.
>
> Unfortunately I don't have the patch emails to reply to, so here's a
> messy bunch of comments. I'll concentrate on the UAPI design as always.
Sorry, the patchset is here:
https://lore.kernel.org/dri-devel/20230109143846.1966301-1-m...@igalia.com/
In the next version, I won't forget cc'ing you at first.
>
> +/*
> + * struct drm_mode_lut3d_mode - 3D LUT mode information.
> + * @lut_size: number of valid points on every dimension of 3D LUT.
> + * @lut_stride: number of points on every dimension of 3D LUT.
> + * @bit_depth: number of bits of RGB. If color_mode defines entries with
> higher
> + * bit_depth the least significant bits will be truncated.
> + * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
> DRM_FORMAT_XBGR16161616.
> + * @flags: flags for hardware-sepcific features
> + */
> +struct drm_mode_lut3d_mode {
> + __u16 lut_size;
> + __u16 lut_stride[3];
> + __u16 bit_depth;
> + __u32 color_format;
> + __u32 flags;
> +};
>
> Why is lut_stride an array of 3, but lut_size is not?
It cames from VA-API:
https://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html#a682756be15d09327ba725b74a863cbcc
In short, the reason is that lut_size is the valid points and is
the same for every dimensions, but lut_stride may vary.
>
> What is the color_mode the comment is
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On Mon, 9 Jan 2023 14:38:09 -0100
Melissa Wen wrote:
> On 01/09, Melissa Wen wrote:
> > Hi,
> >
> > After collecting comments in different places, here is a second version
> > of the work on adding DRM CRTC 3D LUT support to the current DRM color
> > mgmt interface. In comparison to previous proposals [1][2][3], here we
> > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT,
> > that means the following DRM CRTC color correction pipeline:
> >
> > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT
Hi Melissa,
that makes sense to me, for CRTCs. It would be really good to have that
as a diagram in the KMS UAPI documentation.
If someone wants to add a 3D LUT to KMS planes as well, then I'm not
sure if it should be this order or swapped. I will probably have an
opinion about that once Weston is fully HDR capable and has been tried
in the wild for a while with the HDR color operations fine-tuned based
on community feedback. IOW, not for a long time. The YUV to RGB
conversion factors in there as well.
> >
> > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung
> > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a
> > LUT3D_SIZE, that allows userspace to use different supported settings of
> > 3D LUT, fitting VA-API and new color API better. In this sense, I
> > adjusted the pre-blending proposal for post-blending usage.
> >
> > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to
> > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an
> > extra/optional patch to define a default value for LUT3D_MODE, inspired
> > by what we do for the plane blend mode property (pre-multiplied).
> >
> > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage
> > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current
> > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT
> > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context
> > for DCN 301 (easily extendible to other DCN families). Finally, from
> > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display
> > driver, exposing modes supported by HW and programming user shaper and
> > 3D LUT accordingly.
> >
> > Our target userspace is Gamescope/SteamOS.
> >
> > Basic IGT tests were based on [5][6] and are available here (in-progress):
> > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api
> >
> > [1]
> > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/
> > [2]
> > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69
> > [3] https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/
> > [4]
> > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/
> > [5] https://patchwork.freedesktop.org/series/90165/
> > [6] https://patchwork.freedesktop.org/series/109402/
> > [VA_API]
> > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html
> > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
> >
> > Let me know your thoughts.
>
> +Simon Ser, +Pekka Paalanen who might also be interested in this series.
Unfortunately I don't have the patch emails to reply to, so here's a
messy bunch of comments. I'll concentrate on the UAPI design as always.
+/*
+ * struct drm_mode_lut3d_mode - 3D LUT mode information.
+ * @lut_size: number of valid points on every dimension of 3D LUT.
+ * @lut_stride: number of points on every dimension of 3D LUT.
+ * @bit_depth: number of bits of RGB. If color_mode defines entries with higher
+ * bit_depth the least significant bits will be truncated.
+ * @color_format: fourcc values, ex. DRM_FORMAT_XRGB16161616 or
DRM_FORMAT_XBGR16161616.
+ * @flags: flags for hardware-sepcific features
+ */
+struct drm_mode_lut3d_mode {
+ __u16 lut_size;
+ __u16 lut_stride[3];
+ __u16 bit_depth;
+ __u32 color_format;
+ __u32 flags;
+};
Why is lut_stride an array of 3, but lut_size is not?
What is the color_mode the comment is referring to?
What is "number of bits of RGB"? Input precision? Output precision?
Integer or floating point?
Flags cannot be hardware specific, because it makes the whole KMS UAPI
hardware specific. That won't work. You have to have driver-agnostic
definitions for all possible flags.
Why is this the whole first patch? There is no documentation for the
UAPI on how this struct works, so I cannot review this. Explaining just
the individual fields is not enough to understand it. Is this something
the kernel fills in and is read-only to userspace? Is userspace filling
this in?
+ * “LUT3D”:
+ * Blob property to set the 3D LUT mapping pixel data after the color
+ * transformation matrix and before gamma 1D lut correction. The
+ * data is interpreted as an array of drm_color_lut elements.
+ * Hardware might choose not
Re: [RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
On 01/09, Melissa Wen wrote: > Hi, > > After collecting comments in different places, here is a second version > of the work on adding DRM CRTC 3D LUT support to the current DRM color > mgmt interface. In comparison to previous proposals [1][2][3], here we > add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, > that means the following DRM CRTC color correction pipeline: > > Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT > > and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung > proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a > LUT3D_SIZE, that allows userspace to use different supported settings of > 3D LUT, fitting VA-API and new color API better. In this sense, I > adjusted the pre-blending proposal for post-blending usage. > > Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to > the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an > extra/optional patch to define a default value for LUT3D_MODE, inspired > by what we do for the plane blend mode property (pre-multiplied). > > Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage > on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current > AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT > changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context > for DCN 301 (easily extendible to other DCN families). Finally, from > 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display > driver, exposing modes supported by HW and programming user shaper and > 3D LUT accordingly. > > Our target userspace is Gamescope/SteamOS. > > Basic IGT tests were based on [5][6] and are available here (in-progress): > https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api > > [1] > https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ > [2] > https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 > [3] https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ > [4] > https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ > [5] https://patchwork.freedesktop.org/series/90165/ > [6] https://patchwork.freedesktop.org/series/109402/ > [VA_API] > http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html > [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 > > Let me know your thoughts. +Simon Ser, +Pekka Paalanen who might also be interested in this series. Also please let me know if I forgot to address any comments. Melissa > > Thanks, > > Melissa > > Alex Hung (2): > drm: Add 3D LUT mode and its attributes > drm/amd/display: Define 3D LUT struct for HDR planes > > Melissa Wen (16): > drm/drm_color_mgmt: add shaper LUT to color mgmt properties > drm/drm_color_mgmt: add 3D LUT props to DRM color mgmt > drm/drm_color_mgmt: add function to create 3D LUT modes supported > drm/drm_color_mgmt: add function to attach 3D LUT props > drm/drm_color_mgmt: set first lut3d mode as default > drm/amd/display: remove unused regamma condition > drm/amd/display: add comments to describe DM crtc color mgmt behavior > drm/amd/display: encapsulate atomic regamma operation > drm/amd/display: update lut3d and shaper lut to stream > drm/amd/display: handle MPC 3D LUT resources for a given context > drm/amd/display: acquire/release 3D LUT resources for ctx on DCN301 > drm/amd/display: expand array of supported 3D LUT modes > drm/amd/display: enable 3D-LUT DRM properties if supported > drm/amd/display: add user 3D LUT support to the amdgpu_dm color > pipeline > drm/amd/display: decouple steps to reuse in shaper LUT support > drm/amd/display: add user shaper LUT support to amdgpu_dm color > pipeline > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 + > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 + > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 370 -- > .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 2 + > drivers/gpu/drm/amd/display/dc/core/dc.c | 49 ++- > drivers/gpu/drm/amd/display/dc/dc.h | 8 + > .../amd/display/dc/dcn301/dcn301_resource.c | 47 ++- > .../amd/display/modules/color/color_gamma.h | 43 ++ > drivers/gpu/drm/drm_atomic_state_helper.c | 7 + > drivers/gpu/drm/drm_atomic_uapi.c | 24 ++ > drivers/gpu/drm/drm_color_mgmt.c | 127 ++ > drivers/gpu/drm/drm_fb_helper.c | 5 + > drivers/gpu/drm/drm_mode_config.c | 21 + > include/drm/drm_color_mgmt.h | 8 + > include/drm/drm_crtc.h| 32 +- > include/drm/drm_mode_config.h | 25 ++ > include/drm/drm_mode_object.h | 2 +- > include/uapi/drm/drm_mode.h | 17 + > 18 files changed, 757 insertions(+), 39 deletions(-)
[RFC PATCH v2 00/18] Add DRM CRTC 3D LUT interface
Hi, After collecting comments in different places, here is a second version of the work on adding DRM CRTC 3D LUT support to the current DRM color mgmt interface. In comparison to previous proposals [1][2][3], here we add 3D LUT before gamma 1D LUT, but also a shaper 1D LUT before 3D LUT, that means the following DRM CRTC color correction pipeline: Blend -> Degamma 1D LUT -> CTM -> Shaper 1D LUT -> 3D LUT -> Gamma 1D LUT and we also add a DRM CRTC LUT3D_MODE property, based on Alex Hung proposal for pre-blending 3D LUT [4] (Thanks!), instead of just a LUT3D_SIZE, that allows userspace to use different supported settings of 3D LUT, fitting VA-API and new color API better. In this sense, I adjusted the pre-blending proposal for post-blending usage. Patches 1-6 targets the addition of shaper LUT and 3D LUT properties to the current DRM CRTC color mgmt pipeline. Patch 6 can be considered an extra/optional patch to define a default value for LUT3D_MODE, inspired by what we do for the plane blend mode property (pre-multiplied). Patches 7-18 targets AMD display code to enable shaper and 3D LUT usage on DCN 301 (our HW case). Patches 7-9 performs code cleanups on current AMD DM colors code, patch 10 updates AMD stream in case of user 3D LUT changes, patch 11/12 rework AMD MPC 3D LUT resource handling by context for DCN 301 (easily extendible to other DCN families). Finally, from 13-18, we wire up SHAPER LUT, LUT3D and LUT3D MODE to AMD display driver, exposing modes supported by HW and programming user shaper and 3D LUT accordingly. Our target userspace is Gamescope/SteamOS. Basic IGT tests were based on [5][6] and are available here (in-progress): https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/crtc-lut3d-api [1] https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+rene...@ideasonboard.com/ [2] https://github.com/vsyrjala/linux/commit/4d28e8ddf2a076f30f9e5bdc17cbb4656fe23e69 [3] https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/ [4] https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.h...@amd.com/ [5] https://patchwork.freedesktop.org/series/90165/ [6] https://patchwork.freedesktop.org/series/109402/ [VA_API] http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html [KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 Let me know your thoughts. Thanks, Melissa Alex Hung (2): drm: Add 3D LUT mode and its attributes drm/amd/display: Define 3D LUT struct for HDR planes Melissa Wen (16): drm/drm_color_mgmt: add shaper LUT to color mgmt properties drm/drm_color_mgmt: add 3D LUT props to DRM color mgmt drm/drm_color_mgmt: add function to create 3D LUT modes supported drm/drm_color_mgmt: add function to attach 3D LUT props drm/drm_color_mgmt: set first lut3d mode as default drm/amd/display: remove unused regamma condition drm/amd/display: add comments to describe DM crtc color mgmt behavior drm/amd/display: encapsulate atomic regamma operation drm/amd/display: update lut3d and shaper lut to stream drm/amd/display: handle MPC 3D LUT resources for a given context drm/amd/display: acquire/release 3D LUT resources for ctx on DCN301 drm/amd/display: expand array of supported 3D LUT modes drm/amd/display: enable 3D-LUT DRM properties if supported drm/amd/display: add user 3D LUT support to the amdgpu_dm color pipeline drm/amd/display: decouple steps to reuse in shaper LUT support drm/amd/display: add user shaper LUT support to amdgpu_dm color pipeline .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 + .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 370 -- .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 2 + drivers/gpu/drm/amd/display/dc/core/dc.c | 49 ++- drivers/gpu/drm/amd/display/dc/dc.h | 8 + .../amd/display/dc/dcn301/dcn301_resource.c | 47 ++- .../amd/display/modules/color/color_gamma.h | 43 ++ drivers/gpu/drm/drm_atomic_state_helper.c | 7 + drivers/gpu/drm/drm_atomic_uapi.c | 24 ++ drivers/gpu/drm/drm_color_mgmt.c | 127 ++ drivers/gpu/drm/drm_fb_helper.c | 5 + drivers/gpu/drm/drm_mode_config.c | 21 + include/drm/drm_color_mgmt.h | 8 + include/drm/drm_crtc.h| 32 +- include/drm/drm_mode_config.h | 25 ++ include/drm/drm_mode_object.h | 2 +- include/uapi/drm/drm_mode.h | 17 + 18 files changed, 757 insertions(+), 39 deletions(-) -- 2.35.1
