public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs
@ 2026-03-11 21:18 Alex Hung
  2026-03-12 22:35 ` Melissa Wen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Alex Hung @ 2026-03-11 21:18 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona, alex.hung, contact, daniels, mwen, amd-gfx,
	dri-devel
  Cc: Xaver Hugl

Use GAMMA22 for degamma/blend and GAMMA22_INV for shaper so
curves match the color pipeline.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5016
Tested-by: Xaver Hugl <xaver.hugl@kde.org>
Signed-off-by: Alex Hung <alex.hung@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index d59ba82d3d7c..aa4658867e55 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -37,19 +37,19 @@ const u64 amdgpu_dm_supported_degam_tfs =
 	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF) |
-	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
+	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
 
 const u64 amdgpu_dm_supported_shaper_tfs =
 	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_BT2020_OETF) |
-	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
+	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
 
 const u64 amdgpu_dm_supported_blnd_tfs =
 	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
 	BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF) |
-	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
+	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
 
 #define MAX_COLOR_PIPELINE_OPS 10
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs
  2026-03-11 21:18 [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs Alex Hung
@ 2026-03-12 22:35 ` Melissa Wen
  2026-03-13  4:43 ` Claude review: " Claude Code Review Bot
  2026-03-13  4:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Melissa Wen @ 2026-03-12 22:35 UTC (permalink / raw)
  To: Alex Hung, harry.wentland, sunpeng.li, alexander.deucher,
	christian.koenig, airlied, simona, contact, daniels, amd-gfx,
	dri-devel
  Cc: Xaver Hugl



On 11/03/2026 18:18, Alex Hung wrote:
> Use GAMMA22 for degamma/blend and GAMMA22_INV for shaper so
> curves match the color pipeline.
Oh, thanks for this fix!

I had a hard time figuring out that the problem I was facing in the
userspace was that I was trying setting an incorrect value, because
I hadn't paid attention to how it was exposed in the kernel. Unf!

Reviewed-by: Melissa Wen <mwen@igalia.com>

>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5016
> Tested-by: Xaver Hugl <xaver.hugl@kde.org>
> Signed-off-by: Alex Hung <alex.hung@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index d59ba82d3d7c..aa4658867e55 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -37,19 +37,19 @@ const u64 amdgpu_dm_supported_degam_tfs =
>   	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF) |
> -	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
> +	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
>   
>   const u64 amdgpu_dm_supported_shaper_tfs =
>   	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_BT2020_OETF) |
> -	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
> +	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
>   
>   const u64 amdgpu_dm_supported_blnd_tfs =
>   	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_PQ_125_EOTF) |
>   	BIT(DRM_COLOROP_1D_CURVE_BT2020_INV_OETF) |
> -	BIT(DRM_COLOROP_1D_CURVE_GAMMA22_INV);
> +	BIT(DRM_COLOROP_1D_CURVE_GAMMA22);
>   
>   #define MAX_COLOR_PIPELINE_OPS 10
>   


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: drm/amd/display: Fix gamma 2.2 colorop TFs
  2026-03-11 21:18 [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs Alex Hung
  2026-03-12 22:35 ` Melissa Wen
  2026-03-13  4:43 ` Claude review: " Claude Code Review Bot
@ 2026-03-13  4:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  4:43 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/amd/display: Fix gamma 2.2 colorop TFs
Author: Alex Hung <alex.hung@amd.com>
Patches: 2
Reviewed: 2026-03-13T14:43:47.091244

---

This is a single-patch fix that corrects the gamma 2.2 transfer function assignments in the AMD display colorop code. The fix is straightforward, well-motivated by a bug report, and already has a `Tested-by` tag from the KDE compositor developer who reported the issue.

The change is **correct**. The existing code had `GAMMA22` and `GAMMA22_INV` swapped relative to what degamma/shaper/blend operations require. Looking at the pattern of the other transfer functions in the same lists:

- **degamma** and **blend** use linearizing (EOTF) curves: `SRGB_EOTF`, `PQ_125_EOTF`, `BT2020_INV_OETF`
- **shaper** uses de-linearizing (inverse EOTF / OETF) curves: `SRGB_INV_EOTF`, `PQ_125_INV_EOTF`, `BT2020_OETF`

Per the header documentation (`drm_colorop.h:116-117`), `GAMMA22` "applies a power curve with gamma value of 2.2" — i.e., it linearizes gamma-encoded values (an EOTF-like operation). Its inverse `GAMMA22_INV` de-linearizes. So `GAMMA22` belongs with degamma/blend, and `GAMMA22_INV` belongs with shaper — exactly what this patch does.

No concerns with the patch. It should be safe to apply.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: drm/amd/display: Fix gamma 2.2 colorop TFs
  2026-03-11 21:18 [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs Alex Hung
  2026-03-12 22:35 ` Melissa Wen
@ 2026-03-13  4:43 ` Claude Code Review Bot
  2026-03-13  4:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-13  4:43 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The three swaps are all consistent with the established pattern of the other TFs in each list:

1. `amdgpu_dm_supported_degam_tfs`: `GAMMA22_INV` → `GAMMA22` — degamma linearizes, so it needs the forward (EOTF-like) curve, matching `SRGB_EOTF` and `PQ_125_EOTF` already in the list.

2. `amdgpu_dm_supported_shaper_tfs`: `GAMMA22` → `GAMMA22_INV` — shaper de-linearizes, so it needs the inverse curve, matching `SRGB_INV_EOTF` and `PQ_125_INV_EOTF` already in the list.

3. `amdgpu_dm_supported_blnd_tfs`: `GAMMA22_INV` → `GAMMA22` — blend linearizes (same direction as degamma), consistent with the identical set of other TFs shared with degamma.

**Minor nit on the commit message:** The body says "Use GAMMA22 for degamma/blend and GAMMA22_INV for shaper so curves match the color pipeline" — this is accurate and concise but could benefit from a brief note about *why* the previous assignment was wrong (the forward/inverse were swapped). This is purely a style suggestion and not blocking.

**Reviewed-by worthy.** The patch has a clear bug report link, a `Tested-by` from the reporter, and the fix is mechanically obvious once you understand the EOTF/OETF naming convention. No further issues found.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-13  4:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 21:18 [PATCH] drm/amd/display: Fix gamma 2.2 colorop TFs Alex Hung
2026-03-12 22:35 ` Melissa Wen
2026-03-13  4:43 ` Claude review: " Claude Code Review Bot
2026-03-13  4:43 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox