[{"id":38421,"web_url":"https://patchwork.libcamera.org/comment/38421/","msgid":"<CAPhyPA5_KMJeFBxswKg00wFz3UysWsy5CipGasvcP02qXix5wg@mail.gmail.com>","date":"2026-03-26T17:28:27","subject":"Re: [PATCH v1] ipa: rpi: Fix gamma lookup table generation for PiSP\n\tplatform","submitter":{"id":130,"url":"https://patchwork.libcamera.org/api/people/130/","name":"Nick Hollinghurst","email":"nick.hollinghurst@raspberrypi.com"},"content":"Hi David,\n\nThanks for finding this bug!\n\nOn Thu, 26 Mar 2026 at 16:42, David Plowman\n<david.plowman@raspberrypi.com> wrote:\n>\n> generateLut was failing to fill in the final slope value, meaning that\n> fully saturated pixels would full slightly short (the slope of the\n> final piecewise linear segment would default to zero).\n>\n> The loop is slightly reorganised to fix the problem.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/ipa/rpi/pisp/pisp.cpp | 42 +++++++++++++++++----------------------\n>  1 file changed, 18 insertions(+), 24 deletions(-)\n>\n> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp\n> index ec7593ff..851f9b30 100644\n> --- a/src/ipa/rpi/pisp/pisp.cpp\n> +++ b/src/ipa/rpi/pisp/pisp.cpp\n> @@ -80,36 +80,30 @@ int generateLut(const ipa::Pwl &pwl, uint32_t *lut, std::size_t lutSize,\n>         if (pwl.empty())\n>                 return -EINVAL;\n>\n> -       int lastY = 0;\n\nStrictly, lastY could be initialsed to pwl.eval(0);  although it's\nhard to imagine why anyone would configure a value other than 0\n\n\n> +       int nextY = 0;\n>         for (unsigned int i = 0; i < lutSize; i++) {\n> -               int x, y;\n> -               if (i < 32)\n> -                       x = i * 512;\n> -               else if (i < 48)\n> -                       x = (i - 32) * 1024 + 16384;\n> +               unsigned int nextI = i + 1;\n> +\n> +               int nextX;\n> +               if (nextI < 32)\n> +                       nextX = nextI * 512;\n> +               else if (nextI < 48)\n> +                       nextX = (nextI - 32) * 1024 + 16384;\n>                 else\n> -                       x = std::min(65535u, (i - 48) * 2048 + 32768);\n> +                       nextX = std::min(65535u, (nextI - 48) * 2048 + 32768);\n\nI think we can omit the std::min -- the final nextX value would then\nbe 65536. Though depending on how Pwl's extrapolation works\nnumerically, it might then be necessary to clip nextY to 65536.0 to\nprevent overflow.\n\nI note that the hardware's interpolation will always round down.\n\n>\n> -               y = pwl.eval(x);\n> -               if (y < 0 || (i && y < lastY)) {\n> -                       LOG(IPARPI, Error)\n> -                               << \"Malformed PWL for Gamma, disabling!\";\n> -                       return -1;\n> -               }\n> +               int y = nextY;\n> +               nextY = pwl.eval(nextX);\n>\n> -               if (i) {\n> -                       unsigned int slope = y - lastY;\n> -                       if (slope >= (1u << SlopeBits)) {\n> -                               slope = (1u << SlopeBits) - 1;\n> -                               LOG(IPARPI, Info)\n> -                                       << (\"Maximum Gamma slope exceeded, adjusting!\");\n> -                               y = lastY + slope;\n> -                       }\n> -                       lut[i - 1] |= slope << PosBits;\n> +               unsigned int slope = nextY - y;\n> +               if (slope >= (1u << SlopeBits)) {\n> +                       slope = (1u << SlopeBits) - 1;\n> +                       LOG(IPARPI, Info)\n> +                               << (\"Maximum Gamma slope exceeded, adjusting!\");\n> +                       nextY = y + slope;\n>                 }\n>\n> -               lut[i] = y;\n> -               lastY = y;\n> +               lut[i] = y | (slope << PosBits);\n>         }\n>\n>         return 0;\n> --\n> 2.47.3\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E90C4BE086\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Mar 2026 17:28:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3507F62858;\n\tThu, 26 Mar 2026 18:28:40 +0100 (CET)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFEAD62781\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Mar 2026 18:28:38 +0100 (CET)","by mail-ed1-x52a.google.com with SMTP id\n\t4fb4d7f45d1cf-6644a3029b3so2025133a12.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Mar 2026 10:28:38 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"re3RNJAY\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1774546118; cv=none;\n\td=google.com; s=arc-20240605;\n\tb=OG0WA9ramiDjH2LpCUgJCr2P7r0acaNoV/jRyVRRlvQ7OsD9tHLjz2KrhigYkfgelr\n\tRs36LoYqPeXcEBrs+wuUJ6Idg43fYwEKMJdTbsWLdG0MqA4Wo1/nIUwJCuO/xDyjrssq\n\tq66nm3NjPErIUgLtE4TN/hvfcpqCSkmRcJlbiq9yZIxvegF9K133khprpVa5ynTF88oH\n\tJj5+wtUdOg3PXWI1IINrqHhZUtAzyhgAhQC0UPJ4kSyQX6kQy+ZRYvVW/VvsVnr2V1t5\n\tEE7AI49msEFIQgqkF0Qak4q5lguhQb6T6c/+hd266s0rliKjqkO0XPYEDUn51MH1F8Z6\n\tnHFA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=arc-20240605; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:dkim-signature;\n\tbh=fnwFXWtY/QD52zzDwQ9tIcCVGVTheNJzmlPM8b4Tg6c=;\n\tfh=raf7gMhxwhTsDETG3li6wpOfdJf6YyDg/c7aYJ8coN8=;\n\tb=I6zRhf3p9bs1qKWxMeJX0QdunkWvoDM8nPzBl2nmAP4hWyEFLy5TSOP8gJaG5QONbY\n\tEP2N86K5kgeIiSO8r9xDB8cxEcyw5bqxVnCyhRKxBo3Mg2cQaJLQM7nMxI6C+vzgbCEo\n\tVdEbk0RWy3TvjN1F7tOXSwZul5r2Vfww1sPvjMsxHF62ZbsLbkah1bcwKJV+4+vehIS2\n\t+vRHuU/1VlMKvneWuFw6w45OS1wVTJDJ6We+iOTeXGRi05q5H9XF4UZqx/yshL2yQ9Y7\n\tX4xga7tqYCJkOqnU7VXCMcDK9AR4VuWU6K0dF2PNKa6/rTl36WZ0ODqsxw/8s21FhZ7T\n\tFAfw==; darn=lists.libcamera.org","ARC-Authentication-Results":"i=1; mx.google.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1774546118; x=1775150918;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=fnwFXWtY/QD52zzDwQ9tIcCVGVTheNJzmlPM8b4Tg6c=;\n\tb=re3RNJAY0CswRTFt6s4UPi79+yrpsBENM0yo+KXb60vR+2gWQSiTIYWPsg7XArJKI6\n\tDgYqLQ+lITZ6d1H9iqicaNfd4OoAMP35qrn2DU+tTBQx5daVI1iWdrr332ZMdEvLfFsj\n\tGtjGJPvrRhEU1KjubQmyhk8Zabv2vK8dmzl9oUyiynjLkhFbLMyo1dWpLHvELivH0J3a\n\tk4CzLkG7Z+w913nuKcwILQ/nVtbCgv/mzMfSRLKa4yb7brhRRtBZFkc18WxcpORIWmpC\n\tfaDG6JOhIpP0fVawRFo+pgSqSaKyQ/9QMz2SxZnCBoqohN19WLcJUWlRh1Ly92iUhGMk\n\tcj6A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1774546118; x=1775150918;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=fnwFXWtY/QD52zzDwQ9tIcCVGVTheNJzmlPM8b4Tg6c=;\n\tb=ObqXyM9XMJ0kjJbHsSYS62fMCljFAQtMoY7c2VQzF//I4Z5I+D1CWrCArB+r5SwRXm\n\tnV81FrUQt6HESpyIwjMN1oa2KnGVFz0OI46SXszmNn1esOvh4/geMu+fLekZTGxCM+/v\n\tgB4myvFAeMNI8e0xBIYORsloA2WXJkFPNyLsSeJkIY/fnEs5ECq2tdcGaQtjx2MVv+tQ\n\tYfUrndZCgfuKc1OK51AopRRuBCEgO039fIoocjivBRL/yrn/3W3et/EpXhlrW1JyXu1E\n\t9nYF78uIPw3Zr1Dtl0EADA/sjkQzKuijhHE8qoLsEWEJWJYP4XL8LDl8vlDh0w1Byrw9\n\tEA8A==","X-Gm-Message-State":"AOJu0YysBvMRGkPDBJBSm5ZKtFnxOsdPfm7YgmQx3wb1I0DY5WeDtzST\n\tpwRI90b6Y/oFNBckSadKr/pAbfOiwAofXwH8pC9QWpb8x0Xlkzne2xv9ltMcosFlR1oszgG3KXk\n\tNuG0gPBVkD2mDRwC2dKQU1UJxdzUT9hkdeonVGzaBKF+NRB5GApGGQsKnOQ==","X-Gm-Gg":"ATEYQzxzeoodE45Yt+RA/SMoJFcTeMZFDxOFxcAF4Es28+8LjbRkdfZbASJZc5X8xBx\n\tvKOc/nzhI+Fn9Pkc1lu5CcJIWL+lViY/I1pku82slKKhne9xim56e1eSPgEs+szCtL/5GQ08nEl\n\tssfzBOFT3CbOejrYXCCH2yABRUehHZTh2UTsT6EQ6Nu+0VgH73bu9QiigPQHQrLOYsozN6VKKBs\n\ttIDJFYkyzRgPrSVR7oNJH3NMAq2n6s2oQdQ2I3+iwB6Es0mDUfJUtqSp1yL/loMl9WxmFfY1KLH\n\tB6iZY3m3ENhMfoffxR1U5DdffnXIbHZo2wOKZP0=","X-Received":"by 2002:a05:6402:51c6:b0:668:599e:103a with SMTP id\n\t4fb4d7f45d1cf-66a826227c9mr6358565a12.8.1774546118215;\n\tThu, 26 Mar 2026 10:28:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20260326164241.4212-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20260326164241.4212-1-david.plowman@raspberrypi.com>","From":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>","Date":"Thu, 26 Mar 2026 17:28:27 +0000","X-Gm-Features":"AQROBzANIBk1WV509vQUaPsKdLg1otBnj_l4X-nel9w8SFNtDHyEuye0rKfzARs","Message-ID":"<CAPhyPA5_KMJeFBxswKg00wFz3UysWsy5CipGasvcP02qXix5wg@mail.gmail.com>","Subject":"Re: [PATCH v1] ipa: rpi: Fix gamma lookup table generation for PiSP\n\tplatform","To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]