[PATCH] drm/colorop: get DATA blob ref at duplicate_state

2024-09-10 Thread Harry Wentland 
Signed-off-by: Harry Wentland 
---

It was a stupid mistake on my part. The duplicate_state
function needs to take a reference to the blob.

This should fix it. Please give it a try if you can.
I'll roll it into the patch that introduces the bug in
my v6.

Harry

 drivers/gpu/drm/drm_colorop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 86f7f084a73f..fd1cd934df48 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -401,6 +401,9 @@ static void 
__drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colo
 {
memcpy(state, colorop->state, sizeof(*state));
 
+   if (state->data)
+   drm_property_blob_get(state->data);
+
state->bypass = true;
 }
 
-- 
2.46.0

Re: [PATCH v5 00/44] Color Pipeline API w/ VKMS

2024-09-10 Thread Alex Goins 
Hi Harry,

Thanks for this. I just want to remind about a few things that would be required
for NVIDIA hardware, as discussed at the Display Next Hackfest -- fully
understand that they aren't currently included in this series because they
aren't required on AMD hardware. Allowing color ops to be non-bypassable is a
big improvement in this direction.

In each of these cases, it should be possible to represent NVIDIA's color
pipeline using the existing color ops, but it would rely on rejecting invalid
configurations in atomic check, with no way to communicate the limitations to
clients.

- Tag IN_FORMATS for a color pipeline (as you have mentioned).
+ Required to communicate that the first NVIDIA pre-blending LUT is
  incompatible with FP16 inputs.

- Introduce a single-channel 1DLUT color op, or add the ability to flag
  individual channels of an ordinary 1DLUT as read-only.
+ Required to handle NVIDIA's 1D tone-mapping LUT designed to operate only
  on the intensity component of ICtCp.
  
- Allow marking color ops as read-only and non-bypassable (halfway there)
  and/or allow marking a color op as requiring a modeset to update.
+ Required to handle specific LUTs that can't have their entries changed
  while in use without visible glitches/tearing, yet must be enabled to
  support tone-mapping and pre-blend scaling.
+ In lieu of the first option, it may be suitable to just use the existing
  1D curve colorop with only a single curve supported, marked as
  non-bypassable. Allowing modesets as in the second option would allow for
  the possibility of selecting different curves, however.

Thanks,
Alex Goins
NVIDIA

On Mon, 9 Sep 2024, Harry Wentland wrote:

> External email: Use caution opening links or attachments
> 
> 
> On 2024-08-29 10:55, Xaver Hugl wrote:
> > Hi,
> >
> > I have a WIP implementation of this for KWin at
> > https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop.
> > It maps KWin's color pipeline to the drm one to get (primary plane
> > only, for now) direct scanout of HDR content on SDR displays while
> > doing tone mapping (or SDR on HDR, but that's been possible before).
> > It currently uses the two 1D custom luts and the 3D lut for this; once
> > I add support for named 1D curves, it should also make use of that and
> > the matrix. I haven't tested the Intel version yet, but if it can
> > match the pipeline, it should work the same there.
> >
> > In testing your amd-color-pipeline-v5 branch I get two crashes in the
> > kernel though: https://invent.kde.org/-/snippets/3217
> > The null pointer dereference only happens sometimes, seemingly
> > randomly, but if I make KWin generate new blobs each frame, I can 100%
> > reliably reproduce the list_add corruption problem by just opening
> > glxgears in fullscreen while the color profile of the display is set
> > to "built in" (which makes it use the 3D LUT for gamut mapping).
> > Here's a branch that generates the color pipeline each frame and
> > triggers this: 
> > https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop-list-add-corruption
> >
> 
> Thanks for the test branch. Got a repro and will investigate.
> 
> Harry
> 
> >
> >
> > Am Mo., 19. Aug. 2024 um 22:57 Uhr schrieb Harry Wentland
> > :
> >>
> >> This is an RFC set for a color pipeline API, along with implementations
> >> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
> >> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
> >> allowable delta variation as the goal for these transformations is
> >> perceptual correctness, not complete pixel accuracy.
> >>
> >> v5 of this patchset fleshed out documentation for colorops and the
> >> various defines that are being introduced.
> >>
> >> VKMS supports two named transfer function colorops and two matrix
> >> colorops.
> >>
> >> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
> >>
> >> 1. 1D Curve EOTF
> >> 2. 3x4 CTM
> >> 3. Multiplier
> >> 4. 1D Curve Inverse EOTF
> >> 5. 1D LUT
> >> 6. 3D LUT
> >> 7. 1D Curve EOTF
> >> 8. 1D LUT
> >>
> >> The supported curves for the 1D Curve type are:
> >> - sRGB EOTF and its inverse
> >> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
> >> - BT.2020/BT.709 OETF and its inverse
> >>
> >> Note that the 1st and 5th colorops take the EOTF or Inverse
> >> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
> >>
> >> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
> >> exists for other drivers to describe their own 3D LUT capability.
> >>
> >> This mirrors the color pipeline used by gamescope and presented by
> >> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
> >> used. See [1]
> >> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
> >>
> >> At this point we're hoping to see gamescope and kwin implementations
> >> take shape. The existing