From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Harry Wentland <harry.wentland@amd.com>
Cc: intel-gfx@lists.freedesktop.org, ppaalanen@gmail.com,
dri-devel@lists.freedesktop.org, sebastian@sebastianwick.net
Subject: Re: [Intel-gfx] [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes
Date: Tue, 9 Nov 2021 23:45:54 +0200 [thread overview]
Message-ID: <YYrsErnyQ6oOFDps@intel.com> (raw)
In-Reply-To: <5c7b65b6-69ed-2259-0edd-cf04cffa9231@amd.com>
On Tue, Nov 09, 2021 at 03:19:47PM -0500, Harry Wentland wrote:
> On 2021-11-05 08:59, Ville Syrjälä wrote:
> > On Wed, Nov 03, 2021 at 11:10:37AM -0400, Harry Wentland wrote:
> >>
> >>
> >> On 2021-09-06 17:38, Uma Shankar wrote:
> >>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> >>> planes have different capabilities, implemented respective
> >>> structure for the HDR planes.
> >>>
> >>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++
> >>> 1 file changed, 52 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> >>> index afcb4bf3826c..6403bd74324b 100644
> >>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
> >>> }
> >>> }
> >>>
> >>> + /* FIXME input bpc? */
> >>> +__maybe_unused
> >>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> >>> + /* segment 1 */
> >>> + {
> >>> + .flags = (DRM_MODE_LUT_GAMMA |
> >>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>> + DRM_MODE_LUT_INTERPOLATE |
> >>> + DRM_MODE_LUT_NON_DECREASING),
> >>> + .count = 128,
> >>
> >> Is the distribution of the 128 entries uniform?
> >
> > I guess this is the plane gamma thing despite being in intel_color.c,
> > so yeah I think that's correct.
> >
> >> If so, is a
> >> uniform distribution of 128 points across most of the LUT
> >> good enough for HDR with 128 entries?
> >
> > No idea how good this actually is. It is .24 so at least
> > it does have a fair bit of precision.
> >
>
> Precision is good but you also need enough samples. Though that's
> probably less my concern and more your concern and should become
> apparent once its used.
Yeah, for pipe gamma we have a few different variants with
non-uniform spacing of the samples. But not here AFAICS for
whatever reason.
>
> >>
> >>> + .input_bpc = 24, .output_bpc = 16,
> >>> + .start = 0, .end = (1 << 24) - 1,
> >>> + .min = 0, .max = (1 << 24) - 1,
> >>> + },
> >>> + /* segment 2 */
> >>> + {
> >>> + .flags = (DRM_MODE_LUT_GAMMA |
> >>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>> + DRM_MODE_LUT_INTERPOLATE |
> >>> + DRM_MODE_LUT_REUSE_LAST |
> >>> + DRM_MODE_LUT_NON_DECREASING),
> >>> + .count = 1,
> >>> + .input_bpc = 24, .output_bpc = 16,
> >>> + .start = (1 << 24) - 1, .end = 1 << 24,
> >>
> >> .start and .end are only a single entry apart. Is this correct?
> >
> > One think I wanted to do is simplify this stuff by getting rid of
> > .end entirely. So I think this should just be '.start=1<<24' (or
> > whatever way we decide to specify the input precision, which is
> > I think another slightly open question).
> >
> > So for this thing we could just have:
> > { .count = 128, .min = 0, .max = (1 << 24) - 1, .start = 0 },
> > { .count = 1, .min = 0, .max = (7 << 24) - 1, .start = 1 << 24 },
> > { .count = 1, .min = 0, .max = (7 << 24) - 1, .start = 3 << 24 },
> > { .count = 1, .min = 0, .max = (7 << 24) - 1, .start = 7 << 24 },
> >
> > + flags/etc. which I left out for brevity.
> >
>
> Makes sense. I like this.
>
> > So that is trying to indicate that the first 129 entries are equally
> > spaced, and would be used to interpolate for input values [0.0,1.0).
> > Input values [1.0,3.0) would interpolate between entry 128 and 129,
> > and [3.0,7.0) would interpolate between entry 129 and 130.
> >
>
> What in the segment definition defines the 1.0 mark? In your example
> it seems to be at (1 << 24) but then we would have values that go
> beyond the input_bpc for the last three segments.
Yes, input_bpc would define the precision of the input values (.start).
so 1.0 would be at 1<<input_bpc. Tne range of input values is allowed to
extend outside the 0.0-1.0 range.
>
> How about output_bpc? Would output_bpc somehow limit the U32.32 (or
> S31.32) entries, and if so, how?
output_bpc would define the actual precision of the output values,
so again 1.0 would be 1<<output_bpc, and .min and .max define the
min/max values (which can extend outside the 0.0-1.0 range). The
alternative I guess would be to not have .output_bpc at all and
just have .min/.max be s32.32 values. Though then you can't tell
what the actual precision is. Same could be done for .input_bpc
I suppose.
>
> Or should we treat input_/output_bpc only as capability reporting, so
> userspace can calculate the possible error when programming the LUT?
> Again, this leaves us with the question what the input_/output_bpc
> means for our PWL entries.
Yeah, I mostly thought they might be interesting if userspace wants
to know the exact precision. But not strictly necessary if you want
just to go generate a "close enough" curve.
What's PWL?
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2021-11-09 21:46 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-06 21:38 [Intel-gfx] [RFC v2 00/22] Add Support for Plane Color Lut and CSC features Uma Shankar
2021-09-06 21:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add Support for Plane Color Lut and CSC features (rev2) Patchwork
2021-09-06 21:30 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 01/22] drm: RFC for Plane Color Hardware Pipeline Uma Shankar
2021-10-12 10:30 ` Pekka Paalanen
2021-10-12 10:35 ` Simon Ser
2021-10-12 12:00 ` Pekka Paalanen
2021-10-12 19:11 ` Shankar, Uma
2021-10-13 7:25 ` Pekka Paalanen
2021-10-14 19:46 ` Shankar, Uma
2021-10-12 20:58 ` Shankar, Uma
2021-10-13 8:30 ` Pekka Paalanen
2021-10-14 19:44 ` Shankar, Uma
2021-10-15 7:42 ` Pekka Paalanen
2021-10-26 15:11 ` Harry Wentland
2021-10-26 15:36 ` Harry Wentland
2021-10-27 8:00 ` Pekka Paalanen
2021-10-27 12:48 ` Harry Wentland
2021-10-26 15:40 ` Harry Wentland
2021-11-23 15:05 ` Harry Wentland
2021-11-25 20:43 ` Shankar, Uma
2021-11-26 8:21 ` Pekka Paalanen
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 02/22] drm: Add Enhanced Gamma and color lut range attributes Uma Shankar
2021-11-03 15:08 ` Harry Wentland
2021-11-04 8:38 ` Pekka Paalanen
2021-11-04 16:27 ` Harry Wentland
2021-11-05 11:49 ` Ville Syrjälä
2021-11-09 20:22 ` Harry Wentland
2021-11-08 9:54 ` Pekka Paalanen
2021-11-09 20:47 ` Harry Wentland
2021-11-09 22:02 ` Ville Syrjälä
2021-11-10 8:49 ` Pekka Paalanen
2021-11-10 11:55 ` Ville Syrjälä
2021-11-10 15:17 ` Harry Wentland
2021-11-11 8:22 ` Pekka Paalanen
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 03/22] drm: Add Plane Degamma Mode property Uma Shankar
2021-10-12 11:50 ` Pekka Paalanen
2021-10-12 21:02 ` Shankar, Uma
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 04/22] drm: Add Plane Degamma Lut property Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes Uma Shankar
2021-11-03 15:10 ` Harry Wentland
2021-11-05 12:59 ` Ville Syrjälä
2021-11-09 20:19 ` Harry Wentland
2021-11-09 21:45 ` Ville Syrjälä [this message]
2021-11-09 21:56 ` Harry Wentland
2021-11-11 15:17 ` Harry Wentland
2021-11-11 16:42 ` Ville Syrjälä
2021-11-11 20:42 ` Shankar, Uma
2021-11-11 21:10 ` Harry Wentland
2021-11-11 21:58 ` Shankar, Uma
2021-11-12 8:37 ` Pekka Paalanen
2021-11-23 14:40 ` Harry Wentland
2021-11-12 14:54 ` Ville Syrjälä
2021-11-16 8:15 ` Pekka Paalanen
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 06/22] drm/i915/xelpd: Add register definitions for Plane Degamma Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 07/22] drm/i915/xelpd: Enable plane color features Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 08/22] drm/i915/xelpd: Add color capabilities of SDR planes Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 09/22] drm/i915/xelpd: Program Plane Degamma Registers Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 10/22] drm/i915/xelpd: Add plane color check to glk_plane_color_ctl Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 11/22] drm/i915/xelpd: Initialize plane color features Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 12/22] drm/i915/xelpd: Load plane color luts from atomic flip Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 13/22] drm: Add Plane CTM property Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 14/22] drm: Add helper to attach Plane ctm property Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 15/22] drm/i915/xelpd: Define Plane CSC Registers Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 16/22] drm/i915/xelpd: Enable Plane CSC Uma Shankar
2021-09-06 21:38 ` [Intel-gfx] [RFC v2 17/22] drm: Add Plane Gamma Mode property Uma Shankar
2021-09-06 21:39 ` [Intel-gfx] [RFC v2 18/22] drm: Add Plane Gamma Lut property Uma Shankar
2021-09-06 21:39 ` [Intel-gfx] [RFC v2 19/22] drm/i915/xelpd: Define and Initialize Plane Gamma Lut range Uma Shankar
2021-09-06 21:39 ` [Intel-gfx] [RFC v2 20/22] drm/i915/xelpd: Add register definitions for Plane Gamma Uma Shankar
2021-09-06 21:39 ` [Intel-gfx] [RFC v2 21/22] drm/i915/xelpd: Program Plane Gamma Registers Uma Shankar
2021-09-06 21:39 ` [Intel-gfx] [RFC v2 22/22] drm/i915/xelpd: Enable plane gamma Uma Shankar
2021-09-06 21:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Add Support for Plane Color Lut and CSC features (rev2) Patchwork
2021-09-06 23:12 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-10-12 11:55 ` [Intel-gfx] [RFC v2 00/22] Add Support for Plane Color Lut and CSC features Pekka Paalanen
2021-10-12 21:01 ` Shankar, Uma
2021-10-26 15:02 ` Harry Wentland
2021-10-27 8:18 ` Pekka Paalanen
2022-02-02 16:11 ` Harry Wentland
2022-02-03 17:22 ` Shankar, Uma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YYrsErnyQ6oOFDps@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=harry.wentland@amd.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ppaalanen@gmail.com \
--cc=sebastian@sebastianwick.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).