Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 8 Sep 2023 13:19:02 -0100
From:   Melissa Wen <mwen@igalia.com>
To:     Pekka Paalanen <pekka.paalanen@collabora.com>
Cc:     Harry Wentland <harry.wentland@amd.com>,
        amd-gfx@lists.freedesktop.org,
        Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>,
        sunpeng.li@amd.com, Alex Deucher <alexander.deucher@amd.com>,
        dri-devel@lists.freedesktop.org, christian.koenig@amd.com,
        Xinhui.Pan@amd.com, airlied@gmail.com, daniel@ffwll.ch,
        Joshua Ashton <joshua@froggi.es>,
        Sebastian Wick <sebastian.wick@redhat.com>,
        Xaver Hugl <xaver.hugl@gmail.com>,
        Shashank Sharma <Shashank.Sharma@amd.com>,
        Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>,
        sungjoon.kim@amd.com, Alex Hung <alex.hung@amd.com>,
        Simon Ser <contact@emersion.fr>, kernel-dev@igalia.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 10/34] drm/amd/display: add plane 3D LUT
 driver-specific properties
Message-ID: <20230908141902.5lfgdjusfcabgluw@mail.igalia.com>
References: <20230810160314.48225-1-mwen@igalia.com>
 <20230810160314.48225-11-mwen@igalia.com>
 <758deee7-7530-4931-830e-d5a4acff337f@amd.com>
 <20230907105701.5b1c115f.pekka.paalanen@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230907105701.5b1c115f.pekka.paalanen@collabora.com>
Precedence: bulk

On 09/07, Pekka Paalanen wrote:
> On Wed, 6 Sep 2023 15:30:04 -0400
> Harry Wentland <harry.wentland@amd.com> wrote:
> 
> > On 2023-08-10 12:02, Melissa Wen wrote:
> > > Add 3D LUT property for plane gamma correction using a 3D lookup table.
> > > Since a 3D LUT has a limited number of entries in each dimension we want
> > > to use them in an optimal fashion. This means using the 3D LUT in a
> > > colorspace that is optimized for human vision, such as sRGB, PQ, or
> > > another non-linear space. Therefore, userpace may need one 1D LUT
> > > (shaper) before it to delinearize content and another 1D LUT after 3D
> > > LUT (blend) to linearize content again for blending. The next patches
> > > add these 1D LUTs to the plane color mgmt pipeline.
> > > 
> > > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      | 10 ++++++++
> > >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  9 ++++++++
> > >  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 14 +++++++++++
> > >  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 23 +++++++++++++++++++
> > >  4 files changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > index 66bae0eed80c..730a88236501 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > > @@ -363,6 +363,16 @@ struct amdgpu_mode_info {
> > >  	 * @plane_hdr_mult_property:
> > >  	 */
> > >  	struct drm_property *plane_hdr_mult_property;
> > > +	/**
> > > +	 * @plane_lut3d_property: Plane property for gamma correction using a
> > > +	 * 3D LUT (pre-blending).
> > > +	 */  
> > 
> > I think we'll want to describe how the 3DLUT entries are laid out.
> > Something that describes how userspace should fill it, like
> > gamescope does for example:
> > https://github.com/ValveSoftware/gamescope/blob/7108880ed80b68c21750369e2ac9b7315fecf264/src/color_helpers.cpp#L302
> > 
> > Something like: a three-dimensional array, with each dimension
> > having a size of the cubed root of lut3d_size, blue being the
> > outermost dimension, red the innermost.
> >
> 
> Here is an example of how we defined a 3D LUT layout in Weston:
> 
> https://gitlab.freedesktop.org/wayland/weston/-/blob/68e2a606c056c8453c770263f41f34cd68bdc9d0/libweston/color.h#L114-152
> 
> I think that is the most clear definition it can be, without needing to
> understand specific terminology.

Thanks for sharing it, Pekka! I'll check Weston's definition for 3D LUT
and also other elements for improving docs.

Melissa

> 
> 
> Thanks,
> pq
> 
> > 
> > > +	struct drm_property *plane_lut3d_property;
> > > +	/**
> > > +	 * @plane_degamma_lut_size_property: Plane property to define the max
> > > +	 * size of 3D LUT as supported by the driver (read-only).
> > > +	 */  
> > 
> > We should probably document that the size of the 3DLUT should
> > be the size of one dimension cubed, or that the cubed root of
> > the LUT size gives the size per dimension.
> > 
> > Harry