[v2] ipa: rpi: Fix gamma lookup table generation for PiSP platform
diff mbox series

Message ID 20260327090323.2250-1-david.plowman@raspberrypi.com
State New
Headers show
Series
  • [v2] ipa: rpi: Fix gamma lookup table generation for PiSP platform
Related show

Commit Message

David Plowman March 27, 2026, 9:02 a.m. UTC
generateLut was failing to fill in the final slope value, meaning that
fully saturated pixels would full slightly short (the slope of the
final piecewise linear segment would default to zero).

The loop is slightly reorganised to fix the problem.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/pisp/pisp.cpp | 42 +++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

Comments

Nick Hollinghurst March 27, 2026, 10:12 a.m. UTC | #1
Hi David,

LGTM

Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>


On Fri, 27 Mar 2026 at 09:03, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> generateLut was failing to fill in the final slope value, meaning that
> fully saturated pixels would full slightly short (the slope of the
> final piecewise linear segment would default to zero).
>
> The loop is slightly reorganised to fix the problem.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/rpi/pisp/pisp.cpp | 42 +++++++++++++++++----------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
> index ec7593ff..2abb59fa 100644
> --- a/src/ipa/rpi/pisp/pisp.cpp
> +++ b/src/ipa/rpi/pisp/pisp.cpp
> @@ -80,36 +80,30 @@ int generateLut(const ipa::Pwl &pwl, uint32_t *lut, std::size_t lutSize,
>         if (pwl.empty())
>                 return -EINVAL;
>
> -       int lastY = 0;
> +       int nextY = pwl.eval(0);
>         for (unsigned int i = 0; i < lutSize; i++) {
> -               int x, y;
> -               if (i < 32)
> -                       x = i * 512;
> -               else if (i < 48)
> -                       x = (i - 32) * 1024 + 16384;
> +               unsigned int nextI = i + 1;
> +
> +               int nextX;
> +               if (nextI < 32)
> +                       nextX = nextI * 512;
> +               else if (nextI < 48)
> +                       nextX = (nextI - 32) * 1024 + 16384;
>                 else
> -                       x = std::min(65535u, (i - 48) * 2048 + 32768);
> +                       nextX = (nextI - 48) * 2048 + 32768;
>
> -               y = pwl.eval(x);
> -               if (y < 0 || (i && y < lastY)) {
> -                       LOG(IPARPI, Error)
> -                               << "Malformed PWL for Gamma, disabling!";
> -                       return -1;
> -               }
> +               int y = nextY;
> +               nextY = pwl.eval(nextX);
>
> -               if (i) {
> -                       unsigned int slope = y - lastY;
> -                       if (slope >= (1u << SlopeBits)) {
> -                               slope = (1u << SlopeBits) - 1;
> -                               LOG(IPARPI, Info)
> -                                       << ("Maximum Gamma slope exceeded, adjusting!");
> -                               y = lastY + slope;
> -                       }
> -                       lut[i - 1] |= slope << PosBits;
> +               unsigned int slope = nextY - y;
> +               if (slope >= (1u << SlopeBits)) {
> +                       slope = (1u << SlopeBits) - 1;
> +                       LOG(IPARPI, Info)
> +                               << "Maximum Gamma slope exceeded, adjusting!";
> +                       nextY = y + slope;
>                 }
>
> -               lut[i] = y;
> -               lastY = y;
> +               lut[i] = y | (slope << PosBits);
>         }
>
>         return 0;
> --
> 2.47.3
>

Patch
diff mbox series

diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
index ec7593ff..2abb59fa 100644
--- a/src/ipa/rpi/pisp/pisp.cpp
+++ b/src/ipa/rpi/pisp/pisp.cpp
@@ -80,36 +80,30 @@  int generateLut(const ipa::Pwl &pwl, uint32_t *lut, std::size_t lutSize,
 	if (pwl.empty())
 		return -EINVAL;
 
-	int lastY = 0;
+	int nextY = pwl.eval(0);
 	for (unsigned int i = 0; i < lutSize; i++) {
-		int x, y;
-		if (i < 32)
-			x = i * 512;
-		else if (i < 48)
-			x = (i - 32) * 1024 + 16384;
+		unsigned int nextI = i + 1;
+
+		int nextX;
+		if (nextI < 32)
+			nextX = nextI * 512;
+		else if (nextI < 48)
+			nextX = (nextI - 32) * 1024 + 16384;
 		else
-			x = std::min(65535u, (i - 48) * 2048 + 32768);
+			nextX = (nextI - 48) * 2048 + 32768;
 
-		y = pwl.eval(x);
-		if (y < 0 || (i && y < lastY)) {
-			LOG(IPARPI, Error)
-				<< "Malformed PWL for Gamma, disabling!";
-			return -1;
-		}
+		int y = nextY;
+		nextY = pwl.eval(nextX);
 
-		if (i) {
-			unsigned int slope = y - lastY;
-			if (slope >= (1u << SlopeBits)) {
-				slope = (1u << SlopeBits) - 1;
-				LOG(IPARPI, Info)
-					<< ("Maximum Gamma slope exceeded, adjusting!");
-				y = lastY + slope;
-			}
-			lut[i - 1] |= slope << PosBits;
+		unsigned int slope = nextY - y;
+		if (slope >= (1u << SlopeBits)) {
+			slope = (1u << SlopeBits) - 1;
+			LOG(IPARPI, Info)
+				<< "Maximum Gamma slope exceeded, adjusting!";
+			nextY = y + slope;
 		}
 
-		lut[i] = y;
-		lastY = y;
+		lut[i] = y | (slope << PosBits);
 	}
 
 	return 0;