u16 a, u16 b, s64 t)
>
> s64 delta = drm_fixp_mul(b_fp - a_fp, t);
>
> - return drm_fixp2int(a_fp + delta);
> + return drm_fixp2int_round(a_fp + delta);
> }
>
> static s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value)
> --
> 2.46.0
>
Reviewed-by: Louis Chauvet
a_blend(plane[i]->frame_info, stage_buffer,
> output_buffer);
> }
[...]
> diff --git a/drivers/gpu/drm/vkms/vkms_luts.c
> b/drivers/gpu/drm/vkms/vkms_luts.c
> new file mode 100644
> index ..6553d6d442b4
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_luts.c
> @@ -0,0 +1,802 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include
> +
> +#include "vkms_drv.h"
> +#include "vkms_luts.h"
> +
> +static struct drm_color_lut linear_array[LUT_SIZE] = {
> + { 0x0, 0x0, 0x0, 0 },
> + { 0x101, 0x101, 0x101, 0 },
> + { 0x202, 0x202, 0x202, 0 },
> + { 0x303, 0x303, 0x303, 0 },
> + { 0x404, 0x404, 0x404, 0 },
For this LUT and the other, can you add a comment to explain how the
values were generated/found?
[...]
Thanks,
Louis Chauvet
Le 19/08/24 - 16:56, Harry Wentland a écrit :
[...]
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index 3d6785d081f2..3ecda70c2b55 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -435,3 +435,7
drm_colorop_state *state;
> +
> + /* colorop properties */
Maybe the same kind of comment here?
> + /** @properties: property tracking for this colorop */
> + struct drm_object_properties properties;
> +
> +};
> +
[...]
Louis Chauvet
r identity, and sRGB, and replace
> them with fixed-point math instead.
Hi!
I reviewed your VKMS patches and added a few comments in your series. This
series looks very good.
Thanks for this work,
Louis Chauvet
> There are plenty of things that I would like to see, but they could
>
Le 19/08/24 - 16:56, Harry Wentland a écrit :
> Certain operations require us to preserve values below 0.0 and
> above 1.0 (0x0 and 0x respectively in 16 bpc unorm). One
> such operation is a BT709 encoding operation followed by its
> decoding operation, or the reverse.
>
> We'll use s32 value
Le 19/08/24 - 16:57, Harry Wentland a écrit :
> Not all HW will be able to do bypass on all color
> operations. Introduce an 'allow_bypass' boolean for
> all colorop init functions and only create the BYPASS
> property when it's true.
You did not change the documentation of struct drm_colorop_stat
Le 19/08/24 - 16:56, Harry Wentland a écrit :
> We add two 3x4 matrices into the VKMS color pipeline. The reason
> we're adding matrices is so that we can test that application
> of a matrix and its inverse yields an output equal to the input
> image.
>
> One complication with the matrix implement
Signed-off-by: Harry Wentland
> ---
> v6:
> - use clamp_val instead of manual clamping (Louis Chauvet)
Perfect!
> v4:
> - Clarify that we're packing 16-bit UNORM into s32, not
>converting values to a different representation (Pekka)
>
> v3:
> - Use new colorop-&g
;
> Signed-off-by: Harry Wentland
Reviewed-by: Louis Chauvet
> ---
>
> v6:
> - update reference values since we're now rounding
>
> v5:
> - Make apply_3x4_matrix static
>
> v4:
> - Comment on origin of bt709_enc matrix (Pekka)
> - Use full opaque alpha (P
On 03/10/24 - 16:01, Harry Wentland wrote:
> Not all HW will be able to do bypass on all color
> operations. Introduce an 'allow_bypass' boolean for
> all colorop init functions and only create the BYPASS
> property when it's true.
>
> Signed-off-by: Harry Wentland
> ---
> .../amd/display/amdgpu
minate need to include test as .c file (Louis Chauvet)
Thanks for the modification!
With the checkpatch warning fixed:
Reviewed-by: Louis Chauvet
> v5:
> - Bring back static for lerp_u16 and get_lut_index (Arthur)
>
> v4:
> - Test the critical points of the lerp function (Pe
Few checkpatch warnings:
- line length of 103 exceeds 100 columns
- Blank lines aren't necessary before a close brace '}'
- adding a line without newline at end of file
With those fixed:
Reviewd-by: Louis Chauvet
On 03/10/24 - 16:01, Harry Wentland wrote:
> While
mplement.
>
> Signed-off-by: Harry Wentland
> ---
>
> v6:
> - pre-compute colors (Louis Chauvet)
> - round matrix output (Louis Chauvet)
>
> drivers/gpu/drm/vkms/vkms_colorop.c | 34 +++-
> drivers/gpu/drm/vkms/vkms_composer.c | 32 +
kms/vkms_composer.h
> +++ b/drivers/gpu/drm/vkms/vkms_composer.h
> @@ -5,9 +5,22 @@
>
> #include
>
> +/*
> + * This enum is related to the positions of the variables inside
> + * `struct drm_color_lut`, so the order of both needs to be the same.
> + */
> +enum lut
ely need higher precision
> but I'm reluctant to make that change without clear indication
> that we need it. We'll revisit and, if necessary, regenerate
> the LUTs when we have IGT tests for higher precision buffers.
>
> Signed-off-by: Harry Wentland
> Signed-off-b
Le 28/02/2025 à 02:07, Alex Hung a écrit :
On 2/25/25 03:13, Louis Chauvet wrote:
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
Add a new drm_colorop with DRM_COLOROP_1D_CURVE with two subtypes:
DRM_COLOROP_1D_CURVE_SRGB_EOTF and DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
fixp2int always rounds down, fixp2int_ceil rounds up. We need
the new fixp2int_round.
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
Reviewed-by: Louis Chauvet
Hi,
Can I extract this patch from the series and
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
Debugging LUT math is much easier when we can unit test
it. Add kunit functionality to VKMS and add tests for
- get_lut_index
- lerp_u16
Reviewed-by: Louis Chauvet
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
Le 24/02/2025 à 19:50, Alex Hung a écrit :
On 2/24/25 09:07, Louis Chauvet wrote:
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
CTM values are defined as signed-magnitude values. Add
a helper that converts from CTM signed-magnitude fixed
point value to the twos
v6:
- Comment that properties validity depends on type (Louis Chauvet)
v5:
- Add comment to drm_atomic_state.colorops
- Replace a misplaced 'plane' with 'colorop' in comment
- Fix colorop_list kernel doc
- Add kernel doc for color_pipeline
- drop unused drm_c
d-off-by: Alex Hung
Signed-off-by: Harry Wentland
Reviewed-by: Louis Chauvet
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
&val);
+ colorop_state->curve_1d_type = val;
+ }
}
/**
@@ -114,3 +240,11 @@ const char *drm_get_colorop_type_name(enum
drm_colorop_type type
return colorop_type_name[type];
Probably a dumb question: can't we use drm_colorop_type_enum_list
instead of colorop_type_name in the drm_get_colorop_type_name function?
So we will avoid duplicating the string "1D Curve".
}
Thanks,
Louis Chauvet
[...]
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
We want to be able to bypass each colorop at all times.
Introduce a new BYPASS boolean property for this.
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
Reviewed-by: Louis Chauvet
--
Louis Chauvet, Bootlin
drm_property *next_property;
+
};
#define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
+
+
+
Strange useless lines
Thanks,
Louis Chauvet
/**
* drm_colorop_find - look up a Colorop object from its ID
* @dev: DRM device
@@ -268,4 +286,6
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
CTM values are defined as signed-magnitude values. Add
a helper that converts from CTM signed-magnitude fixed
point value to the twos-complement value used by
drm_fixed.
Signed-off-by: Harry Wentland
Reviewed-by: Louis
Le 25/02/2025 à 12:28, Simon Ser a écrit :
On Tuesday, February 25th, 2025 at 10:37, Louis Chauvet
wrote:
Can I extract this patch from the series and apply it on drm-misc-next?
That sounds completely fine by me, and TBH it sounds like it could even
be drm-misc-fixes material
Le 20/12/2024 à 05:33, Alex Hung a écrit :
From: Harry Wentland
Two tests are added to VKMS LUT handling:
- linear
- inv_srgb
Reviewed-by: Louis Chauvet
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
---
v7:
- Fix checkpatch warnings (Louis Chauvet)
- Adde a commit messages
t I'm reluctant to make that change without clear indication
that we need it. We'll revisit and, if necessary, regenerate
the LUTs when we have IGT tests for higher precision buffers.
Signed-off-by: Harry Wentland
Signed-off-by: Alex Hung
---
v7:
- Fix checkpatch warnings (Louis Chauv
with drm/amd code but we can check that before
merging.
That sounds like the simplest solution to me :)
So let's go this way!
I just applied PATCH 2.
I don't feel comfortable merging the whole colorop and AMD parts without
broader reviews, but I can definitely apply the VKMS part.
=%d\n", state->data ?
state->data->base.id : 0);
+ break;
+ default:
+ break;
+ }
+
As suggested by Simon, you could add this switch in a previous patch, so
you could avoid editing the same line twice.
With or without this change:
Reviewed
ff-by: Alex Hung
Signed-off-by: Harry Wentland
---
v7:
- Fix checkpatch warnings
- Change kzalloc(sizeof(struct drm_colorop) ...) to kzalloc(sizeof(*ops[i])
...)
- Change i-1to i - 1
- Add a new line at EOF
v6:
- pre-compute colors (Louis Chauvet)
- round matrix output (Louis Ch
39 matches
Mail list logo
