[{"id":37246,"web_url":"https://patchwork.libcamera.org/comment/37246/","msgid":"<85h5ty5t7f.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-12-10T14:04:20","subject":"Re: [PATCH v7 01/26] libcamera: software_isp: gbm: Add a GBM helper\n\tclass for GPU surface access","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Bryan,\n\nlooks OK to me, just some stylistic stuff, see the comments below.\n\nBryan O'Donoghue writes:\n\n> A helper class to interact with GBM. This will allow us to specify the\n> internal storage format of the GPU when making a texture for the Debayer\n> vertex/fragment shaders and thus ensure we receive an uncompressed and\n> untiled output buffer.\n>\n> Acked-by: Kieran Bingham \n> Signed-off-by: Bryan O'Donoghue \n> ---\n> include/libcamera/internal/gbm.h | 39 ++++++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/gbm.cpp | 130 +++++++++++++++++++++++++\n> src/libcamera/meson.build | 11 +++\n> 4 files changed, 181 insertions(+)\n> create mode 100644 include/libcamera/internal/gbm.h\n> create mode 100644 src/libcamera/gbm.cpp\n>\n> diff --git a/include/libcamera/internal/gbm.h b/include/libcamera/internal/gbm.h\n> new file mode 100644\n> index 000000000..09811d1ef\n> --- /dev/null\n> +++ b/include/libcamera/internal/gbm.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Linaro Ltd.\n> + *\n> + * Authors:\n> + * Bryan O'Donoghue \n> + *\n> + * Helper class for managing GBM interactions\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(GBM)\n> +\n> +class GBM\n> +{\n> +public:\n> +\tGBM();\n> +\t~GBM();\n> +\n> +\tint createDevice();\n> +\tstruct gbm_device *getDevice();\n> +\tPixelFormat getPixelFormat();\n> +\n> +private:\n> +\tint fd_;\n> +\tstruct gbm_device *gbmDevice_;\n> +\tPixelFormat format_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index e9540a2f7..b8324996b 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -23,6 +23,7 @@ libcamera_internal_headers = files([\n> 'dma_buf_allocator.h',\n> 'formats.h',\n> 'framebuffer.h',\n> + 'gbm.h',\n> 'global_configuration.h',\n> 'ipa_data_serializer.h',\n> 'ipa_manager.h',\n> diff --git a/src/libcamera/gbm.cpp b/src/libcamera/gbm.cpp\n> new file mode 100644\n> index 000000000..fcd8dd53c\n> --- /dev/null\n> +++ b/src/libcamera/gbm.cpp\n> @@ -0,0 +1,130 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Linaro Ltd.\n> + *\n> + * Authors:\n> + * Bryan O'Donoghue \n> + *\n> + * Helper class for managing GBM interactions\n> + */\n> +\n> +#include \"libcamera/internal/gbm.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(GBM)\n> +\n> +/**\n> + * \\class GBM\n> + * \\brief Helper class for managing GBM interactions\n> + *\n> + * The GBM class provides a simplified interface for creating and managing\n> + * GBM devices. It handles the initialization and teardown of GBM devices\n> + * used for buffer allocation in graphics and camera pipelines.\n> + *\n> + * This class is responsible for opening a DRI render node, creating a GBM\n> + * device, and providing access to the device and its associated pixel format.\n> + */\n> +\n> +/**\n> + *\\var GBM::fd_\n> + *\\brief file descriptor to DRI device\n> + */\n> +\n> +/**\n> + *\\var GBM::gbmDevice_\n> + *\\brief Pointer to GBM device structure derived from fd_\n> + */\n> +\n> +/**\n> + *\\var GBM::format_\n> + *\\brief Pixel format the GBM surface was created in\n> + */\n> +\n> +/**\n> + *\\brief GBM constructor.\n> + *\n> + * Creates a GBM instance with unitialised state.\n> + */\n> +GBM::GBM()\n> +{\n> +\tfd_ = 0;\n> +\tgbmDevice_ = NULL;\n\nnullptr\n\n> +}\n> +\n> +/**\n> + *\\brief GBM destructor\n> + *\n> + * Cleans up the GBM device if it was successfully created, and closes\n> + * the associated file descriptor.\n> + */\n> +GBM::~GBM()\n> +{\n> +\tif (gbmDevice_)\n> +\t\tgbm_device_destroy(gbmDevice_);\n> +}\n> +\n> +/**\n> + * \\brief Create and initialize a GBM device\n> + *\n> + * Opens the DRI render node (/dev/dri/renderD128) and creates a GBM\n> + * device using the libgbm library. Sets the default pixel format to\n> + * ARGB8888.\n> + *\n> + * \\return 0 on success, or a negative error code on failure\n> + */\n> +int GBM::createDevice()\n> +{\n> +\tconst char *dri_node = \"/dev/dri/renderD128\"; //TODO: get from an env or config setting\n\nDoxygen \\todo can be used for the purpose.\n\n> +\n> +\tfd_ = open(dri_node, O_RDWR | O_CLOEXEC);\n> +\tif (fd_ < 0) {\n> +\t\tLOG(GBM, Error) << \"Open \" << dri_node << \" fail \" << fd_;\n\ns/fail/failed/ ?\n\n> +\t\treturn fd_;\n> +\t}\n> +\n> +\tgbmDevice_ = gbm_create_device(fd_);\n> +\tif (!gbmDevice_) {\n> +\t\tLOG(GBM, Error) << \"gbm_create_device fail\";\n\nfailed?\n\n> +\t\tgoto fail;\n\nNo need to use goto here, can close and return directly.\n\n> +\t}\n> +\n> +\tformat_ = libcamera::formats::ARGB8888;\n> +\n> +\treturn 0;\n> +fail:\n> +\tclose(fd_);\n> +\treturn -errno;\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the GBM device handle\n> + *\n> + * \\return Pointer to the gbm_device structure, or nullptr if the device\n> + * has not been created\n> + */\n> +struct gbm_device * GBM::getDevice()\n> +{\n> +\treturn gbmDevice_;\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the pixel format\n> + *\n> + * \\return The PixelFormat used by this GBM instance (ARGB8888)\n> + */\n> +\n> +PixelFormat GBM::getPixelFormat()\n> +{\n> +\treturn format_;\n> +}\n> +} //namespace libcamera\n\n} /* namespace libcamera */\n\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 90d434a5a..4eaa42062 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -70,6 +70,16 @@ libcamera_deps = []\n> libatomic = cc.find_library('atomic', required : false)\n> libthreads = dependency('threads')\n> \n> +libgbm = cc.find_library('gbm', required: false)\n> +gbm_works = cc.check_header('gbm.h', required: false)\n> +\n> +if libgbm.found() and gbm_works\n> + config_h.set('HAVE_GBM', 1)\n\nHAVE_GBM doesn't seem to be used anywhere.\n\n> + libcamera_internal_sources += files([\n> + 'gbm.cpp',\n> + ])\n> +endif\n> +\n> subdir('base')\n> subdir('converter')\n> subdir('ipa')\n> @@ -178,6 +188,7 @@ libcamera_deps += [\n> libcamera_base_private,\n> libcrypto,\n> libdl,\n> + libgbm,\n> liblttng,\n> libudev,\n> libyaml,","headers":{"Return-Path":"","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 D0066BD1F1\n\tfor ;\n\tWed, 10 Dec 2025 14:04:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 17C076147B;\n\tWed, 10 Dec 2025 15:04:27 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 30A11613CB\n\tfor ;\n\tWed, 10 Dec 2025 15:04:26 +0100 (CET)","from mail-wr1-f70.google.com (mail-wr1-f70.google.com\n\t[209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-534-YfBiFPkAPCOWnbpwFElWaA-1; Wed, 10 Dec 2025 09:04:23 -0500","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-42b2c8fb84fso3397011f8f.2\n\tfor ;\n\tWed, 10 Dec 2025 06:04:23 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb ([213.175.37.14])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-42f7cbe8a7bsm37860382f8f.4.2025.12.10.06.04.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 10 Dec 2025 06:04:21 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"VBTRt4S0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1765375465;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=l5Myg6Gjc1Zv/Jsd/N1QmzHgD67HsEAjKtqJeHuDBBE=;\n\tb=VBTRt4S0qtGePaKCXvgGFFk3pUapvE4MbJwcSmXqa1AjWZKijAwiVmhMG5XmrY0PHUfXCg\n\tEQEvFysL7ZyPRQr6Ja9wyNZn0BzhBji3PuzXOimSM0i/KpzSCT0py32sYVn4xuoiQW4PrT\n\tqbutvvXOVI/uEKPfNPv4DXOW7nzywfU=","X-MC-Unique":"YfBiFPkAPCOWnbpwFElWaA-1","X-Mimecast-MFC-AGG-ID":"YfBiFPkAPCOWnbpwFElWaA_1765375462","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1765375462; x=1765980262;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=l5Myg6Gjc1Zv/Jsd/N1QmzHgD67HsEAjKtqJeHuDBBE=;\n\tb=hpZBcFEc21p4a22AN9HLioIxR6e+cRopABvfOg1uf027LgAoLq5/XlpgjIfs2BDV5w\n\tT2oYymLIQn2YZCzzYGKVdUDvpGw3PT2Qlp8Ecs5GcMpvAd+rJR2tq0u02xx6609LDStn\n\tu56nHE55jGzWHxVMdu6vP88KXw7k29ElQoN6MK6Z3yAEeK7w7KIlk5a58XMY1Y7HHTm2\n\t5ZMKapHckmJbOKEkuel/xsdNlcPddd7WQQEhYcTlXSoG8JNrzB4p3jTEW20g4G5kIy9r\n\t40hRcolIwXGCBqzd7xxugXnU0W1Y5pq6x/Vk4xjcu+If6VrOicfVCOt+oEd1poxmMJcM\n\t4bzg==","X-Gm-Message-State":"AOJu0Yx9n2bpKw4hqaaBmUk9UW/n9pPd+lo3xKPYGrIhlUH6YcX9GbF3\n\t2/PtqlP3N2Uj3tEYlI7hdToEXr2vVM77UPAGiMLKW1fMPBxGl+5mAWqBvHh0gCDv6Qb9Qir1GQd\n\t51RGwGJIY7V6S/bqwEGSi3Y+Y7OE4WRp1lIUt9sUbHRsd51+ZON4vooElF5kK5KXQIzPewhMwTK\n\tI=","X-Gm-Gg":"AY/fxX7jK29NnriFudPC3uCZ1h1IfAZdj9VP0nqlxahcEwc2ootihX9+YhOovoF72Fv\n\tq/LGV5CSsImJtQWhwI3x5l3XColltqc0GtDaPcRlxauohJHd+IryrV+ik5yj/um/jLniB/n3IQ+\n\trnVS38AJsqKyYqIznm1C+ZY7SvM1W+rRC6dxNqLzCBBFrBPCQiK5Magdx3b41uQ6Ci1+PnvFOW9\n\tHTM5ED/cxbRV9cuNkHbad6bOFy2OWlFPHdiMXzuY4Nv6DH+Hz1mQgdn9Cvj7Mde/DtulPFDM+Cc\n\t7ugYW0g1o5VQS1dEfGbl+aTghdNgVED7BGiuQvhOG5ScfDHKF898setTtU8IOTnuxcrvUdJyLVm\n\tKayA8anO/jpTIXAOwZAcIwJ8tog==","X-Received":["by 2002:a05:6000:1acd:b0:42b:47da:c31c with SMTP id\n\tffacd0b85a97d-42fa3afd40dmr2571008f8f.37.1765375462317; \n\tWed, 10 Dec 2025 06:04:22 -0800 (PST)","by 2002:a05:6000:1acd:b0:42b:47da:c31c with SMTP id\n\tffacd0b85a97d-42fa3afd40dmr2570948f8f.37.1765375461722; \n\tWed, 10 Dec 2025 06:04:21 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEkTZ6Pig86djdtduznO+1cOscXdJKg+1r8xMv1SNr3BpW9HiPPf+cyGqNxDqGCYCNvGbQ9Lg==","From":"Milan Zamazal ","To":"Bryan O'Donoghue ","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz, Kieran Bingham\n\t","Subject":"Re: [PATCH v7 01/26] libcamera: software_isp: gbm: Add a GBM helper\n\tclass for GPU surface access","In-Reply-To":"<20251210005354.44726-2-bryan.odonoghue@linaro.org> (Bryan\n\tO'Donoghue's message of \"Wed, 10 Dec 2025 00:53:29 +0000\")","References":"<20251210005354.44726-1-bryan.odonoghue@linaro.org>\n\t<20251210005354.44726-2-bryan.odonoghue@linaro.org>","Date":"Wed, 10 Dec 2025 15:04:20 +0100","Message-ID":"<85h5ty5t7f.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"P4vmGFTd3blfMGp5lKWjeME204kLRa1rB82ePnRqZm0_1765375462","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]